Skip to content

fix: marshal async beforeunload JS dialog callbacks onto CEF UI thread - #5

Merged
trethore merged 5 commits into
masterfrom
dev
Mar 26, 2026
Merged

trethore merged 5 commits into
masterfrom
dev

Conversation

@trethore

Copy link
Copy Markdown
Owner

#4

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #4 by ensuring asynchronous CefJSDialogCallback::Continue(...) calls (notably for onBeforeUnloadDialog(...) in windowless mode) are marshaled onto the CEF UI thread, matching CEF’s expected threading model and avoiding fallback/default JS dialog handling.

Changes:

  • Marshal CefJSDialogCallback::Continue(...) onto TID_UI using CefPostTask when invoked off the UI thread.
  • Move the CEF clone target from ./cef to ./references/cef.
  • Update ignore configuration related to the new references/ location (and add a new .ignore file).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
native/CefJSDialogCallback_N.cpp Posts Continue(...) to the CEF UI thread when needed.
clone_cef.sh Clones CEF into references/cef instead of cef.
.ignore Adds gitignore-style negation rules for references/ (effectiveness depends on tooling).
.gitignore Updates ignored path from /cef to references/.
Comments suppressed due to low confidence (1)

clone_cef.sh:15

  • TARGET_DIR now points to ${SCRIPT_DIR}/references/cef, but the script doesn't create the parent ${SCRIPT_DIR}/references directory. git clone ... "${TARGET_DIR}" will fail if references/ doesn't already exist. Add a mkdir -p "$(dirname "${TARGET_DIR}")" (or similar) before the clone.
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
TARGET_DIR="${SCRIPT_DIR}/references/cef"
CEF_REPO_URL="https://github.com/chromiumembedded/cef"

if [ -e "${TARGET_DIR}" ]; then
  echo "${TARGET_DIR} already exists; skipping clone."
  exit 0
fi

git clone "${CEF_REPO_URL}" "${TARGET_DIR}"


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .ignore
@trethore
trethore merged commit c5c0137 into master Mar 26, 2026
4 checks passed
@trethore
trethore deleted the dev branch August 11, 2026 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants