diff --git a/.github/workflows/apply-patches.yml b/.github/workflows/apply-patches.yml index 32a6431564..4a88477802 100644 --- a/.github/workflows/apply-patches.yml +++ b/.github/workflows/apply-patches.yml @@ -73,7 +73,7 @@ jobs: target-platform: linux - name: Upload Patch Conflict Fix if: ${{ failure() }} - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: update-patches path: patches/update-patches.patch diff --git a/.github/workflows/archaeologist-dig.yml b/.github/workflows/archaeologist-dig.yml index d284e8b98b..45c35c2f38 100644 --- a/.github/workflows/archaeologist-dig.yml +++ b/.github/workflows/archaeologist-dig.yml @@ -49,7 +49,7 @@ jobs: sha-file: .dig-old filename: electron.old.d.ts - name: Upload artifacts - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f #v7.0.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a #v7.0.1 with: name: artifacts path: electron/artifacts diff --git a/.github/workflows/audit-branch-ci.yml b/.github/workflows/audit-branch-ci.yml index 602cc1b895..c23abce161 100644 --- a/.github/workflows/audit-branch-ci.yml +++ b/.github/workflows/audit-branch-ci.yml @@ -28,7 +28,7 @@ jobs: .github .yarn - run: yarn workspaces focus @electron/gha-workflows - - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 id: audit-errors with: github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/branch-created.yml b/.github/workflows/branch-created.yml index d89ce6ade0..b83b02744d 100644 --- a/.github/workflows/branch-created.yml +++ b/.github/workflows/branch-created.yml @@ -105,7 +105,7 @@ jobs: org: electron - name: Generate Release Project Board Metadata if: ${{ steps.check-major-version.outputs.MAJOR }} - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 id: generate-project-metadata env: MAJOR: ${{ steps.check-major-version.outputs.MAJOR }} diff --git a/.github/workflows/issue-opened.yml b/.github/workflows/issue-opened.yml index 866d6965a4..08fe6445c9 100644 --- a/.github/workflows/issue-opened.yml +++ b/.github/workflows/issue-opened.yml @@ -46,7 +46,7 @@ jobs: .yarn - run: yarn workspaces focus @electron/gha-workflows - name: Add labels - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 id: add-labels env: ISSUE_BODY: ${{ github.event.issue.body }} diff --git a/.github/workflows/pipeline-segment-electron-build.yml b/.github/workflows/pipeline-segment-electron-build.yml index ce507de736..0afb8e8180 100644 --- a/.github/workflows/pipeline-segment-electron-build.yml +++ b/.github/workflows/pipeline-segment-electron-build.yml @@ -77,7 +77,6 @@ env: ELECTRON_ARTIFACTS_BLOB_STORAGE: ${{ secrets.ELECTRON_ARTIFACTS_BLOB_STORAGE }} ELECTRON_RBE_JWT: ${{ secrets.ELECTRON_RBE_JWT }} SUDOWOODO_EXCHANGE_URL: ${{ secrets.SUDOWOODO_EXCHANGE_URL }} - SUDOWOODO_EXCHANGE_TOKEN: ${{ secrets.SUDOWOODO_EXCHANGE_TOKEN }} GCLIENT_EXTRA_ARGS: ${{ inputs.target-platform == 'macos' && '--custom-var=checkout_mac=True --custom-var=host_os=mac' || inputs.target-platform == 'win' && '--custom-var=checkout_win=True' || '--custom-var=checkout_arm=True --custom-var=checkout_arm64=True' }} ELECTRON_OUT_DIR: Default ACTIONS_STEP_DEBUG: ${{ secrets.ACTIONS_STEP_DEBUG }} diff --git a/.github/workflows/pipeline-segment-electron-clang-tidy.yml b/.github/workflows/pipeline-segment-electron-clang-tidy.yml index e517020232..0f338a603b 100644 --- a/.github/workflows/pipeline-segment-electron-clang-tidy.yml +++ b/.github/workflows/pipeline-segment-electron-clang-tidy.yml @@ -135,7 +135,7 @@ jobs: run: echo "::add-matcher::src/electron/.github/problem-matchers/clang.json" - name: Run Clang-Tidy run: | - e init -f --root=$(pwd) --out=${ELECTRON_OUT_DIR} testing --target-cpu ${TARGET_ARCH} + e init -f --root=$(pwd) --out=${ELECTRON_OUT_DIR} testing --target-cpu ${TARGET_ARCH} --remote-build none export GN_EXTRA_ARGS="target_cpu=\"${TARGET_ARCH}\"" if [ "${{ inputs.target-platform }}" = "win" ]; then diff --git a/.github/workflows/pipeline-segment-electron-gn-check.yml b/.github/workflows/pipeline-segment-electron-gn-check.yml index 2e23a1466c..ef5b69210c 100644 --- a/.github/workflows/pipeline-segment-electron-gn-check.yml +++ b/.github/workflows/pipeline-segment-electron-gn-check.yml @@ -130,7 +130,7 @@ jobs: run: | for target_cpu in ${{ inputs.target-archs }} do - e init -f --root=$(pwd) --out=Default ${{ inputs.gn-build-type }} --import ${{ inputs.gn-build-type }} --target-cpu $target_cpu + e init -f --root=$(pwd) --out=Default ${{ inputs.gn-build-type }} --import ${{ inputs.gn-build-type }} --target-cpu $target_cpu --remote-build none cd src export GN_EXTRA_ARGS="target_cpu=\"$target_cpu\"" if [ "${{ inputs.target-platform }}" = "linux" ]; then diff --git a/.github/workflows/pipeline-segment-electron-publish.yml b/.github/workflows/pipeline-segment-electron-publish.yml index cddeb6028a..bde985c35a 100644 --- a/.github/workflows/pipeline-segment-electron-publish.yml +++ b/.github/workflows/pipeline-segment-electron-publish.yml @@ -79,7 +79,6 @@ env: ELECTRON_ARTIFACTS_BLOB_STORAGE: ${{ secrets.ELECTRON_ARTIFACTS_BLOB_STORAGE }} ELECTRON_RBE_JWT: ${{ secrets.ELECTRON_RBE_JWT }} SUDOWOODO_EXCHANGE_URL: ${{ secrets.SUDOWOODO_EXCHANGE_URL }} - SUDOWOODO_EXCHANGE_TOKEN: ${{ secrets.SUDOWOODO_EXCHANGE_TOKEN }} GCLIENT_EXTRA_ARGS: ${{ inputs.target-platform == 'macos' && '--custom-var=checkout_mac=True --custom-var=host_os=mac' || inputs.target-platform == 'win' && '--custom-var=checkout_win=True' || diff --git a/.github/workflows/pipeline-segment-electron-test.yml b/.github/workflows/pipeline-segment-electron-test.yml index 2688ee08ed..f79e0be39b 100644 --- a/.github/workflows/pipeline-segment-electron-test.yml +++ b/.github/workflows/pipeline-segment-electron-test.yml @@ -315,7 +315,7 @@ jobs: if: always() && !cancelled() - name: Upload Test Artifacts if: always() - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f #v7.0.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a #v7.0.1 with: name: ${{ inputs.target-platform == 'linux' && format('test_artifacts_{0}_{1}_{2}', env.ARTIFACT_KEY, inputs.display-server, matrix.shard) || format('test_artifacts_{0}_{1}', env.ARTIFACT_KEY, matrix.shard) }} path: src/electron/spec/artifacts diff --git a/.github/workflows/pr-template-check.yml b/.github/workflows/pr-template-check.yml index a354c31ff2..fd91f681e9 100644 --- a/.github/workflows/pr-template-check.yml +++ b/.github/workflows/pr-template-check.yml @@ -25,7 +25,7 @@ jobs: sparse-checkout: .github/PULL_REQUEST_TEMPLATE.md sparse-checkout-cone-mode: false - name: Check for required sections - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: script: | const fs = require('fs'); @@ -38,6 +38,12 @@ jobs: return; } const body = context.payload.pull_request.body || ''; + // Allow through if body contains a valid backport line + const backportRegex = /Backport of (?:#|https:\/\/github.com\/electron\/electron\/pull\/)\d+/i; + if (backportRegex.test(body)) { + console.log('Backport PR detected, skipping required section check.'); + return; + } const missingSections = requiredSections.filter( (section) => !body.includes(section), ); diff --git a/.github/workflows/pr-triage-automation.yml b/.github/workflows/pr-triage-automation.yml index f35a7df913..e8cb0bf431 100644 --- a/.github/workflows/pr-triage-automation.yml +++ b/.github/workflows/pr-triage-automation.yml @@ -36,7 +36,17 @@ jobs: with: creds: ${{ secrets.ISSUE_TRIAGE_GH_APP_CREDS }} org: electron + - name: Get project item status + uses: dsanders11/project-actions/get-item@5767984408ccc6742f83acc8b8d8ea5e09f329af # v2.0.0 + id: get-item + with: + token: ${{ steps.generate-token.outputs.token }} + project-number: 118 + fail-if-item-not-found: false - name: Set status to Needs Review + if: >- + (steps.get-item.outputs.field-status == '🛑 Needs Submitter Response' + || steps.get-item.outputs.field-status == '🟡 WIP') uses: dsanders11/project-actions/edit-item@5767984408ccc6742f83acc8b8d8ea5e09f329af # v2.0.0 with: token: ${{ steps.generate-token.outputs.token }} diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml index d259268e39..2af2547cee 100644 --- a/.github/workflows/scorecards.yml +++ b/.github/workflows/scorecards.yml @@ -43,7 +43,7 @@ jobs: # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF # format to the repository Actions tab. - name: "Upload artifact" - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: SARIF file path: results.sarif diff --git a/docs/tutorial/installation.md b/docs/tutorial/installation.md index 7f798a8ad9..fb4c4901f7 100644 --- a/docs/tutorial/installation.md +++ b/docs/tutorial/installation.md @@ -25,6 +25,27 @@ included in the `electron` package: npx install-electron --no ``` +## Installing prereleases + +Electron [distributes experimental releases of future major versions](./electron-timelines.md) +via npm as well. + +Nightly builds contain the latest changes from the `main` branch: + +```sh +npm install electron-nightly --save-dev +``` + +Alpha and beta builds contain changes slated for the next major version: + +```sh +npm install electron@alpha --save-dev +npm install electron@beta --save-dev +``` + +> [!TIP] +> For more information on available Electron releases, see the [Release Status dashboard](https://releases.electronjs.org). + ## Running Electron ad-hoc If you're in a pinch and would prefer to not use `npm install` in your local diff --git a/lib/browser/api/global-shortcut.ts b/lib/browser/api/global-shortcut.ts index b5efaa7240..48c0751422 100644 --- a/lib/browser/api/global-shortcut.ts +++ b/lib/browser/api/global-shortcut.ts @@ -1,2 +1,39 @@ -const { globalShortcut } = process._linkedBinding('electron_browser_global_shortcut'); -export default globalShortcut; +const { createGlobalShortcut } = process._linkedBinding('electron_browser_global_shortcut'); + +let globalShortcut: Electron.GlobalShortcut; + +const createGlobalShortcutIfNeeded = () => { + if (globalShortcut === undefined) { + globalShortcut = createGlobalShortcut(); + } +}; + +export default new Proxy( + {}, + { + get: (_target, property: keyof Electron.GlobalShortcut) => { + createGlobalShortcutIfNeeded(); + const value = globalShortcut[property]; + if (typeof value === 'function') { + return value.bind(globalShortcut); + } + return value; + }, + set: (_target, property: string, value: unknown) => { + createGlobalShortcutIfNeeded(); + return Reflect.set(globalShortcut, property, value); + }, + ownKeys: () => { + createGlobalShortcutIfNeeded(); + return Reflect.ownKeys(globalShortcut); + }, + has: (_target, property: string) => { + createGlobalShortcutIfNeeded(); + return property in globalShortcut; + }, + getOwnPropertyDescriptor: (_target, property: string) => { + createGlobalShortcutIfNeeded(); + return Reflect.getOwnPropertyDescriptor(globalShortcut, property); + } + } +); diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 33ffb7fc38..8a55962234 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -103,7 +103,6 @@ chore_remove_check_is_test_on_script_injection_tracker.patch fix_restore_original_resize_performance_on_macos.patch feat_allow_code_cache_in_custom_schemes.patch build_run_reclient_cfg_generator_after_chrome.patch -fix_add_support_for_skipping_first_2_no-op_refreshes_in_thumb_cap.patch refactor_expose_file_system_access_blocklist.patch feat_add_support_for_missing_dialog_features_to_shell_dialogs.patch feat_enable_passing_exit_code_on_service_process_crash.patch diff --git a/patches/chromium/chore_provide_iswebcontentscreationoverridden_with_full_params.patch b/patches/chromium/chore_provide_iswebcontentscreationoverridden_with_full_params.patch index 7c8f88a61b..36e5dee236 100644 --- a/patches/chromium/chore_provide_iswebcontentscreationoverridden_with_full_params.patch +++ b/patches/chromium/chore_provide_iswebcontentscreationoverridden_with_full_params.patch @@ -6,79 +6,6 @@ Subject: chore: provide IsWebContentsCreationOverridden with full params Pending upstream patch, this gives us fuller access to the window.open params so that we will be able to decide whether to cancel it or not. -diff --git a/chrome/browser/media/offscreen_tab.cc b/chrome/browser/media/offscreen_tab.cc -index 047f1258f951f763df2ca0ba355b19d19337826b..9fc7114312212fbe38ddec740b4aebbcd72cb0f8 100644 ---- a/chrome/browser/media/offscreen_tab.cc -+++ b/chrome/browser/media/offscreen_tab.cc -@@ -285,8 +285,7 @@ bool OffscreenTab::IsWebContentsCreationOverridden( - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) { -+ const content::mojom::CreateNewWindowParams& params) { - // Disallow creating separate WebContentses. The WebContents implementation - // uses this to spawn new windows/tabs, which is also not allowed for - // offscreen tabs. -diff --git a/chrome/browser/media/offscreen_tab.h b/chrome/browser/media/offscreen_tab.h -index 231e3595f218aeebe28d0b13ce6182e7a4d6f4e1..609bd205d1cd0404cab3471765bef8b0e053d061 100644 ---- a/chrome/browser/media/offscreen_tab.h -+++ b/chrome/browser/media/offscreen_tab.h -@@ -108,8 +108,7 @@ class OffscreenTab final : public ProfileObserver, - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) final; -+ const content::mojom::CreateNewWindowParams& params) override; - void EnterFullscreenModeForTab( - content::RenderFrameHost* requesting_frame, - const blink::mojom::FullscreenOptions& options) final; -diff --git a/chrome/browser/ui/ash/keyboard/chrome_keyboard_web_contents.cc b/chrome/browser/ui/ash/keyboard/chrome_keyboard_web_contents.cc -index 33edb0a90d886dd44956046e03fcc182a0f6bc8e..5b5edd3da3d9f7a248ea3affd195c36bfd64a38e 100644 ---- a/chrome/browser/ui/ash/keyboard/chrome_keyboard_web_contents.cc -+++ b/chrome/browser/ui/ash/keyboard/chrome_keyboard_web_contents.cc -@@ -80,8 +80,7 @@ class ChromeKeyboardContentsDelegate : public content::WebContentsDelegate, - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) override { -+ const content::mojom::CreateNewWindowParams& params) override { - return true; - } - -diff --git a/chrome/browser/ui/ash/web_view/ash_web_view_impl.cc b/chrome/browser/ui/ash/web_view/ash_web_view_impl.cc -index e4e42249c476ccae58f0ba42e7dbae299f1e36bd..670c30ed4b7f1a07eb4b8abaa95e5a8a9d94bd8d 100644 ---- a/chrome/browser/ui/ash/web_view/ash_web_view_impl.cc -+++ b/chrome/browser/ui/ash/web_view/ash_web_view_impl.cc -@@ -121,10 +121,9 @@ bool AshWebViewImpl::IsWebContentsCreationOverridden( - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) { -+ const content::mojom::CreateNewWindowParams& params) { - if (params_.suppress_navigation) { -- NotifyDidSuppressNavigation(target_url, -+ NotifyDidSuppressNavigation(params.target_url, - WindowOpenDisposition::NEW_FOREGROUND_TAB, - /*from_user_gesture=*/true); - return true; -diff --git a/chrome/browser/ui/ash/web_view/ash_web_view_impl.h b/chrome/browser/ui/ash/web_view/ash_web_view_impl.h -index 39fa45f0a0f9076bd7ac0be6f455dd540a276512..3d0381d463eed73470b28085830f2a23751659a7 100644 ---- a/chrome/browser/ui/ash/web_view/ash_web_view_impl.h -+++ b/chrome/browser/ui/ash/web_view/ash_web_view_impl.h -@@ -60,8 +60,7 @@ class AshWebViewImpl : public ash::AshWebView, - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) override; -+ const content::mojom::CreateNewWindowParams& params) override; - content::WebContents* OpenURLFromTab( - content::WebContents* source, - const content::OpenURLParams& params, diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 8f8852b2af1acfa4ec985fd1c8b50563b991b12a..c2f2903545b191c5ab13462bf330efce37d7d08c 100644 --- a/chrome/browser/ui/browser.cc @@ -116,112 +43,6 @@ index acdb28d61badaf549c47e107f4795e1e2adc37c9..b6aca0bf802f2146d09d2a872ff9e091 content::WebContents* CreateCustomWebContents( content::RenderFrameHost* opener, content::SiteInstance* source_site_instance, -diff --git a/chrome/browser/ui/media_router/presentation_receiver_window_controller.cc b/chrome/browser/ui/media_router/presentation_receiver_window_controller.cc -index a05510eadf5c9ff24bb7999aa76229946319280f..a80ecc46f8a6b84de83d608257d45ae61ccc2170 100644 ---- a/chrome/browser/ui/media_router/presentation_receiver_window_controller.cc -+++ b/chrome/browser/ui/media_router/presentation_receiver_window_controller.cc -@@ -206,8 +206,7 @@ bool PresentationReceiverWindowController::IsWebContentsCreationOverridden( - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) { -+ const content::mojom::CreateNewWindowParams& params) { - // Disallow creating separate WebContentses. The WebContents implementation - // uses this to spawn new windows/tabs, which is also not allowed for - // local presentations. -diff --git a/chrome/browser/ui/media_router/presentation_receiver_window_controller.h b/chrome/browser/ui/media_router/presentation_receiver_window_controller.h -index 3fc06be01f20e8cd314d95d73a3f58c2f0742fe9..c07910ae59a185442f37ea6e7b96fdf3a33aba82 100644 ---- a/chrome/browser/ui/media_router/presentation_receiver_window_controller.h -+++ b/chrome/browser/ui/media_router/presentation_receiver_window_controller.h -@@ -106,8 +106,7 @@ class PresentationReceiverWindowController final - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) override; -+ const content::mojom::CreateNewWindowParams& params) override; - - // The profile used for the presentation. - raw_ptr otr_profile_; -diff --git a/chrome/browser/ui/views/hats/hats_next_web_dialog.cc b/chrome/browser/ui/views/hats/hats_next_web_dialog.cc -index 783d05c39ecbe5e556af2e5fd8b4f30fb5aa1196..e1895bcf9d3c6818aa64b1479774a143dae74d53 100644 ---- a/chrome/browser/ui/views/hats/hats_next_web_dialog.cc -+++ b/chrome/browser/ui/views/hats/hats_next_web_dialog.cc -@@ -104,8 +104,7 @@ class HatsNextWebDialog::HatsWebView : public views::WebView { - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) override { -+ const content::mojom::CreateNewWindowParams& params) override { - return true; - } - content::WebContents* CreateCustomWebContents( -diff --git a/components/embedder_support/android/delegate/web_contents_delegate_android.cc b/components/embedder_support/android/delegate/web_contents_delegate_android.cc -index a82c39208a2709d9e292dac5c89bd2c9bf529a98..d578299501e15815ac615528610889d270aaf6ad 100644 ---- a/components/embedder_support/android/delegate/web_contents_delegate_android.cc -+++ b/components/embedder_support/android/delegate/web_contents_delegate_android.cc -@@ -214,15 +214,14 @@ bool WebContentsDelegateAndroid::IsWebContentsCreationOverridden( - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) { -+ const content::mojom::CreateNewWindowParams& params) { - JNIEnv* env = AttachCurrentThread(); - ScopedJavaLocalRef obj = GetJavaDelegate(env); - if (obj.is_null()) { - return false; - } - ScopedJavaLocalRef java_gurl = -- url::GURLAndroid::FromNativeGURL(env, target_url); -+ url::GURLAndroid::FromNativeGURL(env, params.target_url.spec()); - return !Java_WebContentsDelegateAndroid_shouldCreateWebContents(env, obj, - java_gurl); - } -diff --git a/components/embedder_support/android/delegate/web_contents_delegate_android.h b/components/embedder_support/android/delegate/web_contents_delegate_android.h -index 5754a774852d53a99d34568d0b98aa19171add2a..a75d85c97a75fffa5dba6ac427d7608e345c02ef 100644 ---- a/components/embedder_support/android/delegate/web_contents_delegate_android.h -+++ b/components/embedder_support/android/delegate/web_contents_delegate_android.h -@@ -82,8 +82,7 @@ class WebContentsDelegateAndroid : public content::WebContentsDelegate { - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) override; -+ const content::mojom::CreateNewWindowParams& params) override; - void CloseContents(content::WebContents* source) override; - bool DidAddMessageToConsole(content::WebContents* source, - blink::mojom::ConsoleMessageLevel log_level, -diff --git a/components/offline_pages/content/background_loader/background_loader_contents.cc b/components/offline_pages/content/background_loader/background_loader_contents.cc -index 12b38ddee62e3af915083830703a4c2e8e249f00..bf4e8dcbdecd46712c48107cfee554b7bb1e0277 100644 ---- a/components/offline_pages/content/background_loader/background_loader_contents.cc -+++ b/components/offline_pages/content/background_loader/background_loader_contents.cc -@@ -85,8 +85,7 @@ bool BackgroundLoaderContents::IsWebContentsCreationOverridden( - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) { -+ const content::mojom::CreateNewWindowParams& params) { - // Background pages should not create other webcontents/tabs. - return true; - } -diff --git a/components/offline_pages/content/background_loader/background_loader_contents.h b/components/offline_pages/content/background_loader/background_loader_contents.h -index b969f1d97b7e3396119b579cfbe61e19ff7d2dd4..b8d6169652da28266a514938b45b39c58df53573 100644 ---- a/components/offline_pages/content/background_loader/background_loader_contents.h -+++ b/components/offline_pages/content/background_loader/background_loader_contents.h -@@ -66,8 +66,7 @@ class BackgroundLoaderContents : public content::WebContentsDelegate { - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) override; -+ const content::mojom::CreateNewWindowParams& params) override; - - content::WebContents* AddNewContents( - content::WebContents* source, diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index d43e75c20aca09080f4223d339c88381f030c504..8cd59445bae73ff0193e4512d7c36740cbad847f 100644 --- a/content/browser/web_contents/web_contents_impl.cc @@ -356,48 +177,6 @@ index f459dddeb3f8f3a33ffead0e96fba791d18a0108..f7a229b186774ca3a01f2d747eab139a content::WebContents* CreateCustomWebContents( content::RenderFrameHost* opener, content::SiteInstance* source_site_instance, -diff --git a/fuchsia_web/webengine/browser/frame_impl.cc b/fuchsia_web/webengine/browser/frame_impl.cc -index 9c1fb0b2ed4f013ef6108a9844b22f6bfe697621..ef4991adc766d53b03d280395630b83ced38c2e8 100644 ---- a/fuchsia_web/webengine/browser/frame_impl.cc -+++ b/fuchsia_web/webengine/browser/frame_impl.cc -@@ -585,8 +585,7 @@ bool FrameImpl::IsWebContentsCreationOverridden( - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) { -+ const content::mojom::CreateNewWindowParams& params) { - // Specify a generous upper bound for unacknowledged popup windows, so that we - // can catch bad client behavior while not interfering with normal operation. - constexpr size_t kMaxPendingWebContentsCount = 10; -diff --git a/fuchsia_web/webengine/browser/frame_impl.h b/fuchsia_web/webengine/browser/frame_impl.h -index 756d4192271d6a65cfe8e1511737c565b543cb1f..5688f6f745056565c3c01947f741c4d13e27b6ae 100644 ---- a/fuchsia_web/webengine/browser/frame_impl.h -+++ b/fuchsia_web/webengine/browser/frame_impl.h -@@ -308,8 +308,7 @@ class WEB_ENGINE_EXPORT FrameImpl : public fuchsia::web::Frame, - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) override; -+ const content::mojom::CreateNewWindowParams& params) override; - void WebContentsCreated(content::WebContents* source_contents, - int opener_render_process_id, - int opener_render_frame_id, -diff --git a/headless/lib/browser/headless_web_contents_impl.cc b/headless/lib/browser/headless_web_contents_impl.cc -index ae616fa9c352413e23fb509b3e12e0e4fab5a094..0efa65f7d4346cfe78d2f27ba55a0526202315ff 100644 ---- a/headless/lib/browser/headless_web_contents_impl.cc -+++ b/headless/lib/browser/headless_web_contents_impl.cc -@@ -232,8 +232,7 @@ class HeadlessWebContentsImpl::Delegate : public content::WebContentsDelegate { - content::SiteInstance* source_site_instance, - content::mojom::WindowContainerType window_container_type, - const GURL& opener_url, -- const std::string& frame_name, -- const GURL& target_url) override { -+ const content::mojom::CreateNewWindowParams& params) override { - return headless_web_contents_->browser_context() - ->options() - ->block_new_web_contents(); diff --git a/ui/views/controls/webview/web_dialog_view.cc b/ui/views/controls/webview/web_dialog_view.cc index a7d370220136f2c31afd70644ada26f1768b2e0d..e08dd61b20c68398b0532f5ae74e0ffd5968c19b 100644 --- a/ui/views/controls/webview/web_dialog_view.cc diff --git a/patches/chromium/fix_add_support_for_skipping_first_2_no-op_refreshes_in_thumb_cap.patch b/patches/chromium/fix_add_support_for_skipping_first_2_no-op_refreshes_in_thumb_cap.patch deleted file mode 100644 index 7a4f24a8ab..0000000000 --- a/patches/chromium/fix_add_support_for_skipping_first_2_no-op_refreshes_in_thumb_cap.patch +++ /dev/null @@ -1,45 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Samuel Attard -Date: Tue, 13 Feb 2024 17:40:15 -0800 -Subject: fix: add support for skipping first 2 no-op refreshes in thumb cap - -Fixes a bug in the SCK thumbnail capturer, will be reported upstream for a hopefully -less hacky fix. - -The first refresh is "no windows yet, no thumbnails". -The second refresh is "we have windows, we queued the thumbnail requests" -The third refresh (the one we want) is "we have windows, and have thumbnail requests" - -This really isn't ideal at all, we need to refactor desktopCapturer (read completely re-implement it) -to use StartUpdating and handle the events instead of using the "get the list once" method. - -diff --git a/chrome/browser/media/webrtc/desktop_media_list.h b/chrome/browser/media/webrtc/desktop_media_list.h -index 09895ccfa99999e6e0ea24a3190d3f429ee40344..b1189cebeabe49971c0d0d4d013e6fe26e7df5a5 100644 ---- a/chrome/browser/media/webrtc/desktop_media_list.h -+++ b/chrome/browser/media/webrtc/desktop_media_list.h -@@ -149,6 +149,8 @@ class DesktopMediaList { - // source lists that need to be displayed independently from when the - // DesktopMediaList gains focus. - virtual void ShowDelegatedList() = 0; -+ -+ int skip_next_refresh_ = 0; - }; - - #endif // CHROME_BROWSER_MEDIA_WEBRTC_DESKTOP_MEDIA_LIST_H_ -diff --git a/chrome/browser/media/webrtc/desktop_media_list_base.cc b/chrome/browser/media/webrtc/desktop_media_list_base.cc -index f95b2230135dbcd6b19a31215d4f10be3481148c..9e1eb8423969e75d5ece0056690f651c1bb901cd 100644 ---- a/chrome/browser/media/webrtc/desktop_media_list_base.cc -+++ b/chrome/browser/media/webrtc/desktop_media_list_base.cc -@@ -232,7 +232,11 @@ uint32_t DesktopMediaListBase::GetImageHash(const gfx::Image& image) { - void DesktopMediaListBase::OnRefreshComplete() { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK(refresh_callback_); -- std::move(refresh_callback_).Run(); -+ if (skip_next_refresh_ > 0) { -+ skip_next_refresh_--; -+ } else { -+ std::move(refresh_callback_).Run(); -+ } - } - - void DesktopMediaListBase::ScheduleNextRefresh() { diff --git a/script/lint-roller-chromium-changes.mjs b/script/lint-roller-chromium-changes.mjs index 55666db985..4021f2cbea 100644 --- a/script/lint-roller-chromium-changes.mjs +++ b/script/lint-roller-chromium-changes.mjs @@ -275,7 +275,7 @@ async function main() { if (hasErrors) { console.log(' NOTE: Add "Skip-Lint: " to a commit message to skip linting that commit.'); } - process.exit(hasErrors && process.env.CI ? 1 : 0); + process.exit(hasErrors ? 1 : 0); } if ((await fs.realpath(process.argv[1])) === fileURLToPath(import.meta.url)) { diff --git a/script/release/github-token.ts b/script/release/github-token.ts index 9a40346217..86bc5c8e21 100644 --- a/script/release/github-token.ts +++ b/script/release/github-token.ts @@ -5,18 +5,36 @@ import { ElectronReleaseRepo } from './types'; const cachedTokens = Object.create(null); +const SUDOWOODO_OIDC_AUDIENCE = 'sudowoodo-broker'; + +async function getActionsIdToken(): Promise { + const { ACTIONS_ID_TOKEN_REQUEST_URL, ACTIONS_ID_TOKEN_REQUEST_TOKEN } = process.env; + if (!ACTIONS_ID_TOKEN_REQUEST_URL || !ACTIONS_ID_TOKEN_REQUEST_TOKEN) { + throw new Error( + 'ACTIONS_ID_TOKEN_REQUEST_URL/_TOKEN not set — the job needs `permissions: id-token: write` to mint an OIDC token for the sudowoodo exchange' + ); + } + const { value } = await got(ACTIONS_ID_TOKEN_REQUEST_URL + '&audience=' + SUDOWOODO_OIDC_AUDIENCE, { + headers: { + authorization: 'Bearer ' + ACTIONS_ID_TOKEN_REQUEST_TOKEN + } + }).json<{ value: string }>(); + return value; +} + async function ensureToken(repo: ElectronReleaseRepo) { if (!cachedTokens[repo]) { cachedTokens[repo] = await (async () => { - const { ELECTRON_GITHUB_TOKEN, SUDOWOODO_EXCHANGE_URL, SUDOWOODO_EXCHANGE_TOKEN } = process.env; + const { ELECTRON_GITHUB_TOKEN, SUDOWOODO_EXCHANGE_URL } = process.env; if (ELECTRON_GITHUB_TOKEN) { return ELECTRON_GITHUB_TOKEN; } - if (SUDOWOODO_EXCHANGE_URL && SUDOWOODO_EXCHANGE_TOKEN) { + if (SUDOWOODO_EXCHANGE_URL) { + const idToken = await getActionsIdToken(); const resp = await got.post(SUDOWOODO_EXCHANGE_URL + '?repo=' + repo, { headers: { - Authorization: SUDOWOODO_EXCHANGE_TOKEN + Authorization: 'Bearer ' + idToken }, throwHttpErrors: false }); diff --git a/shell/browser/api/electron_api_desktop_capturer.cc b/shell/browser/api/electron_api_desktop_capturer.cc index 509ff092d7..91748b2af0 100644 --- a/shell/browser/api/electron_api_desktop_capturer.cc +++ b/shell/browser/api/electron_api_desktop_capturer.cc @@ -9,13 +9,15 @@ #include #include "base/containers/flat_map.h" +#include "base/memory/raw_ptr.h" #include "base/strings/utf_string_conversions.h" -#include "base/threading/thread_restrictions.h" +#include "base/task/sequenced_task_runner.h" #include "chrome/browser/media/webrtc/desktop_capturer_wrapper.h" #include "chrome/browser/media/webrtc/desktop_media_list.h" +#include "chrome/browser/media/webrtc/desktop_media_list_observer.h" +#include "chrome/browser/media/webrtc/native_desktop_media_list.h" #include "chrome/browser/media/webrtc/thumbnail_capturer_mac.h" #include "chrome/browser/media/webrtc/window_icon_util.h" -#include "content/public/browser/browser_thread.h" #include "content/public/browser/desktop_capture.h" #include "gin/object_template_builder.h" #include "shell/browser/javascript_environment.h" @@ -223,48 +225,79 @@ namespace electron::api { gin::DeprecatedWrapperInfo DesktopCapturer::kWrapperInfo = { gin::kEmbedderNativeGin}; -DesktopCapturer::DesktopCapturer(v8::Isolate* isolate) {} +// Observer that forwards DesktopMediaListObserver events back to +// the owning DesktopCapturer, tagging them with the list type so +// the capturer knows which list fired. +class DesktopCapturer::ListObserver : public DesktopMediaListObserver { + public: + ListObserver(DesktopCapturer* capturer, + DesktopMediaList* list, + bool need_thumbnails) + : capturer_{capturer}, list_{list}, need_thumbnails_{need_thumbnails} {} + ~ListObserver() override = default; + + [[nodiscard]] bool IsReady() const { + if (!has_sources_) + return false; + + if (!need_thumbnails_) + return true; + + // Every source needs a non-empty thumbnail. + // Thumbnails finish one-at-a-time via tasks posted by the worker thread, + // so wait until every thumbnail arrives. + for (int i = 0; i < list_->GetSourceCount(); ++i) { + if (list_->GetSource(i).thumbnail.isNull()) + return false; + } + return true; + } + + private: + // Post OnListReady to the message loop when sources & thumbnails are done. + void MaybeNotifyReady() { + if (!IsReady() || notified_) + return; + notified_ = true; + base::SequencedTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, + base::BindOnce(&DesktopCapturer::OnListReady, + capturer_->weak_ptr_factory_.GetWeakPtr(), list_.get())); + } + + // DesktopMediaListObserver: + void OnSourceAdded(int index) override { + has_sources_ = true; + MaybeNotifyReady(); + } + void OnSourceRemoved(int index) override {} + void OnSourceMoved(int old_index, int new_index) override {} + void OnSourceNameChanged(int index) override {} + void OnSourceThumbnailChanged(int index) override { MaybeNotifyReady(); } + void OnSourcePreviewChanged(size_t index) override {} + void OnDelegatedSourceListSelection() override { + // For delegated source lists (e.g. PipeWire), the selection event means + // the user picked a source. Treat as ready once we also have a thumbnail. + has_sources_ = true; + MaybeNotifyReady(); + } + void OnDelegatedSourceListDismissed() override { + base::SequencedTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, base::BindOnce(&DesktopCapturer::HandleFailure, + capturer_->weak_ptr_factory_.GetWeakPtr())); + } + + raw_ptr capturer_; + raw_ptr list_; + bool need_thumbnails_ = false; + bool has_sources_ = false; + bool notified_ = false; +}; + +DesktopCapturer::DesktopCapturer() = default; DesktopCapturer::~DesktopCapturer() = default; -DesktopCapturer::DesktopListListener::DesktopListListener( - OnceCallback update_callback, - OnceCallback failure_callback, - bool skip_thumbnails) - : update_callback_(std::move(update_callback)), - failure_callback_(std::move(failure_callback)), - have_thumbnail_(skip_thumbnails) {} - -DesktopCapturer::DesktopListListener::~DesktopListListener() = default; - -void DesktopCapturer::DesktopListListener::OnDelegatedSourceListSelection() { - if (have_thumbnail_) { - content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, - std::move(update_callback_)); - } else { - have_selection_ = true; - } -} - -void DesktopCapturer::DesktopListListener::OnSourceThumbnailChanged(int index) { - if (have_selection_) { - // This is called every time a thumbnail is refreshed. Reset variable to - // ensure that the callback is not run again. - have_selection_ = false; - - // PipeWire returns a single source, so index is not relevant. - content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, - std::move(update_callback_)); - } else { - have_thumbnail_ = true; - } -} - -void DesktopCapturer::DesktopListListener::OnDelegatedSourceListDismissed() { - content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, - std::move(failure_callback_)); -} - void DesktopCapturer::StartHandling(bool capture_window, bool capture_screen, const gfx::Size& thumbnail_size, @@ -282,41 +315,8 @@ void DesktopCapturer::StartHandling(bool capture_window, // clear any existing captured sources. captured_sources_.clear(); - - if (capture_window && capture_screen) { - // Some capturers like PipeWire support a single capturer for both screens - // and windows. Use it if possible, treating both as window capture - std::unique_ptr desktop_capturer = - webrtc::DesktopCapturer::CreateGenericCapturer( - content::desktop_capture::CreateDesktopCaptureOptions()); - auto capturer = desktop_capturer ? std::make_unique( - std::move(desktop_capturer)) - : nullptr; - if (capturer && capturer->GetDelegatedSourceListController()) { - capture_screen_ = false; - capture_window_ = capture_window; - window_capturer_ = std::make_unique( - DesktopMediaList::Type::kWindow, std::move(capturer), true, true); - window_capturer_->SetThumbnailSize(thumbnail_size); - - OnceCallback update_callback = base::BindOnce( - &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(), - window_capturer_.get()); - OnceCallback failure_callback = base::BindOnce( - &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr()); - - window_listener_ = std::make_unique( - std::move(update_callback), std::move(failure_callback), - thumbnail_size.IsEmpty()); - window_capturer_->StartUpdating(window_listener_.get()); - - return; - } - } - - // Start listening for captured sources. - capture_window_ = capture_window; - capture_screen_ = capture_screen; + finished_ = false; + pending_lists_ = 0; #if BUILDFLAG(IS_MAC) if (!ui::TryPromptUserForScreenCapture()) { @@ -325,77 +325,72 @@ void DesktopCapturer::StartHandling(bool capture_window, } #endif - { - // Initialize the source list. - // Apply the new thumbnail size and restart capture. - if (capture_window) { - auto capturer = MakeWindowCapturer(); - if (capturer) { - window_capturer_ = std::make_unique( - DesktopMediaList::Type::kWindow, std::move(capturer), true, true); - window_capturer_->SetThumbnailSize(thumbnail_size); -#if BUILDFLAG(IS_MAC) - window_capturer_->skip_next_refresh_ = - ShouldUseThumbnailCapturerMac(DesktopMediaList::Type::kWindow) - ? thumbnail_size.IsEmpty() ? 1 : 2 - : 0; -#endif + const bool need_thumbnails = !thumbnail_size.IsEmpty(); + bool need_screen = capture_screen; - OnceCallback update_callback = base::BindOnce( - &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(), - window_capturer_.get()); - - if (window_capturer_->IsSourceListDelegated()) { - OnceCallback failure_callback = base::BindOnce( - &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr()); - window_listener_ = std::make_unique( - std::move(update_callback), std::move(failure_callback), - thumbnail_size.IsEmpty()); - window_capturer_->StartUpdating(window_listener_.get()); - } else { - window_capturer_->Update(std::move(update_callback), - /* refresh_thumbnails = */ true); - } + if (capture_window) { + // Some capturers like PipeWire support a single capturer for both screens + // and windows. Use it if possible, treating both as window capture. + std::unique_ptr capturer; + bool use_generic = false; + if (capture_screen) { + auto desktop_capturer = webrtc::DesktopCapturer::CreateGenericCapturer( + content::desktop_capture::CreateDesktopCaptureOptions()); + auto wrapper = desktop_capturer + ? std::make_unique( + std::move(desktop_capturer)) + : nullptr; + if (wrapper && wrapper->GetDelegatedSourceListController()) { + capturer = std::move(wrapper); + use_generic = true; + // Generic capturer handles both types as window capture. + need_screen = false; } } + if (!capturer) + capturer = MakeWindowCapturer(); - if (capture_screen) { - auto capturer = MakeScreenCapturer(); - if (capturer) { - screen_capturer_ = std::make_unique( - DesktopMediaList::Type::kScreen, std::move(capturer)); - screen_capturer_->SetThumbnailSize(thumbnail_size); -#if BUILDFLAG(IS_MAC) - screen_capturer_->skip_next_refresh_ = - ShouldUseThumbnailCapturerMac(DesktopMediaList::Type::kScreen) - ? thumbnail_size.IsEmpty() ? 1 : 2 - : 0; -#endif + if (capturer) { + window_capturer_ = std::make_unique( + DesktopMediaList::Type::kWindow, std::move(capturer), + /* add_current_process_windows = */ true, + /* auto_show_delegated_source_list = */ use_generic); + window_capturer_->SetThumbnailSize(thumbnail_size); + window_observer_ = std::make_unique( + this, window_capturer_.get(), need_thumbnails); + window_capturer_->StartUpdating(window_observer_.get()); + pending_lists_++; + } + } - OnceCallback update_callback = base::BindOnce( - &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(), - screen_capturer_.get()); - - if (screen_capturer_->IsSourceListDelegated()) { - OnceCallback failure_callback = base::BindOnce( - &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr()); - screen_listener_ = std::make_unique( - std::move(update_callback), std::move(failure_callback), - thumbnail_size.IsEmpty()); - screen_capturer_->StartUpdating(screen_listener_.get()); - } else { - screen_capturer_->Update(std::move(update_callback), - /* refresh_thumbnails = */ true); - } - } + if (need_screen) { + auto capturer = MakeScreenCapturer(); + if (capturer) { + screen_capturer_ = std::make_unique( + DesktopMediaList::Type::kScreen, std::move(capturer)); + screen_capturer_->SetThumbnailSize(thumbnail_size); + screen_observer_ = std::make_unique( + this, screen_capturer_.get(), need_thumbnails); + screen_capturer_->StartUpdating(screen_observer_.get()); + pending_lists_++; } } } -void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) { - if (capture_window_ && - list->GetMediaListType() == DesktopMediaList::Type::kWindow) { - capture_window_ = false; +void DesktopCapturer::OnListReady(DesktopMediaList* list) { + if (finished_) + return; + + CollectSourcesFrom(list); + + if (--pending_lists_ == 0) + HandleSuccess(); +} + +void DesktopCapturer::CollectSourcesFrom(DesktopMediaList* list) { + const auto type = list->GetMediaListType(); + + if (type == DesktopMediaList::Type::kWindow) { std::vector window_sources; window_sources.reserve(list->GetSourceCount()); for (int i = 0; i < list->GetSourceCount(); i++) { @@ -406,9 +401,7 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) { std::back_inserter(captured_sources_)); } - if (capture_screen_ && - list->GetMediaListType() == DesktopMediaList::Type::kScreen) { - capture_screen_ = false; + if (type == DesktopMediaList::Type::kScreen) { std::vector screen_sources; screen_sources.reserve(list->GetSourceCount()); for (int i = 0; i < list->GetSourceCount(); i++) { @@ -477,16 +470,19 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) { std::move(screen_sources.begin(), screen_sources.end(), std::back_inserter(captured_sources_)); } - - if (!capture_window_ && !capture_screen_) - HandleSuccess(); } void DesktopCapturer::HandleSuccess() { + if (finished_) + return; + finished_ = true; + v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); v8::HandleScope scope(isolate); gin_helper::CallMethod(this, "_onfinished", captured_sources_); + window_observer_.reset(); + screen_observer_.reset(); screen_capturer_.reset(); window_capturer_.reset(); @@ -494,11 +490,17 @@ void DesktopCapturer::HandleSuccess() { } void DesktopCapturer::HandleFailure() { + if (finished_) + return; + finished_ = true; + v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); v8::HandleScope scope(isolate); gin_helper::CallMethod(this, "_onerror", "Failed to get sources."); + window_observer_.reset(); + screen_observer_.reset(); screen_capturer_.reset(); window_capturer_.reset(); @@ -508,7 +510,7 @@ void DesktopCapturer::HandleFailure() { // static gin_helper::Handle DesktopCapturer::Create( v8::Isolate* isolate) { - auto handle = gin_helper::CreateHandle(isolate, new DesktopCapturer(isolate)); + auto handle = gin_helper::CreateHandle(isolate, new DesktopCapturer()); // Keep reference alive until capturing has finished. handle->Pin(isolate); diff --git a/shell/browser/api/electron_api_desktop_capturer.h b/shell/browser/api/electron_api_desktop_capturer.h index 459c97425d..c36a41f8bd 100644 --- a/shell/browser/api/electron_api_desktop_capturer.h +++ b/shell/browser/api/electron_api_desktop_capturer.h @@ -9,8 +9,7 @@ #include #include -#include "chrome/browser/media/webrtc/desktop_media_list_observer.h" -#include "chrome/browser/media/webrtc/native_desktop_media_list.h" +#include "chrome/browser/media/webrtc/desktop_media_list.h" #include "shell/common/gin_helper/pinnable.h" #include "shell/common/gin_helper/wrappable.h" @@ -23,8 +22,7 @@ namespace electron::api { class DesktopCapturer final : public gin_helper::DeprecatedWrappable, - public gin_helper::Pinnable, - private DesktopMediaListObserver { + public gin_helper::Pinnable { public: struct Source { DesktopMediaList::Source media_list_source; @@ -55,57 +53,24 @@ class DesktopCapturer final DesktopCapturer& operator=(const DesktopCapturer&) = delete; protected: - explicit DesktopCapturer(v8::Isolate* isolate); + DesktopCapturer(); ~DesktopCapturer() override; private: - // DesktopMediaListObserver: - void OnSourceAdded(int index) override {} - void OnSourceRemoved(int index) override {} - void OnSourceMoved(int old_index, int new_index) override {} - void OnSourceNameChanged(int index) override {} - void OnSourceThumbnailChanged(int index) override {} - void OnSourcePreviewChanged(size_t index) override {} - void OnDelegatedSourceListSelection() override {} - void OnDelegatedSourceListDismissed() override {} + class ListObserver; - using OnceCallback = base::OnceClosure; - - class DesktopListListener : public DesktopMediaListObserver { - public: - DesktopListListener(OnceCallback update_callback, - OnceCallback failure_callback, - bool skip_thumbnails); - ~DesktopListListener() override; - - protected: - void OnSourceAdded(int index) override {} - void OnSourceRemoved(int index) override {} - void OnSourceMoved(int old_index, int new_index) override {} - void OnSourceNameChanged(int index) override {} - void OnSourceThumbnailChanged(int index) override; - void OnSourcePreviewChanged(size_t index) override {} - void OnDelegatedSourceListSelection() override; - void OnDelegatedSourceListDismissed() override; - - private: - OnceCallback update_callback_; - OnceCallback failure_callback_; - bool have_selection_ = false; - bool have_thumbnail_ = false; - }; - - void UpdateSourcesList(DesktopMediaList* list); + void OnListReady(DesktopMediaList* list); + void CollectSourcesFrom(DesktopMediaList* list); void HandleFailure(); void HandleSuccess(); - std::unique_ptr window_listener_; - std::unique_ptr screen_listener_; + std::unique_ptr window_observer_; + std::unique_ptr screen_observer_; std::unique_ptr window_capturer_; std::unique_ptr screen_capturer_; std::vector captured_sources_; - bool capture_window_ = false; - bool capture_screen_ = false; + int pending_lists_ = 0; + bool finished_ = false; bool fetch_window_icons_ = false; #if BUILDFLAG(IS_WIN) bool using_directx_capturer_ = false; diff --git a/shell/browser/api/electron_api_global_shortcut.cc b/shell/browser/api/electron_api_global_shortcut.cc index e2ccf35ae7..ecc6d9560b 100644 --- a/shell/browser/api/electron_api_global_shortcut.cc +++ b/shell/browser/api/electron_api_global_shortcut.cc @@ -15,14 +15,17 @@ #include "electron/shell/browser/electron_browser_context.h" #include "electron/shell/common/electron_constants.h" #include "extensions/common/command.h" -#include "gin/dictionary.h" #include "gin/object_template_builder.h" +#include "gin/persistent.h" #include "shell/browser/api/electron_api_system_preferences.h" #include "shell/browser/browser.h" #include "shell/common/gin_converters/accelerator_converter.h" #include "shell/common/gin_converters/callback_converter.h" -#include "shell/common/gin_helper/handle.h" +#include "shell/common/gin_helper/dictionary.h" +#include "shell/common/gin_helper/wrappable_pointer_tags.h" #include "shell/common/node_includes.h" +#include "v8/include/cppgc/allocation.h" +#include "v8/include/v8-cppgc.h" #if BUILDFLAG(IS_MAC) #include "base/mac/mac_util.h" @@ -50,21 +53,28 @@ bool MapHasMediaKeys( namespace electron::api { -gin::DeprecatedWrapperInfo GlobalShortcut::kWrapperInfo = { - gin::kEmbedderNativeGin}; +const gin::WrapperInfo GlobalShortcut::kWrapperInfo = + electron::MakeWrapperInfo(electron::kElectronGlobalShortcut); -GlobalShortcut::GlobalShortcut() = default; +GlobalShortcut::GlobalShortcut(v8::Isolate* isolate) { + gin::PerIsolateData* data = gin::PerIsolateData::From(isolate); + data->AddDisposeObserver(this); +} + +GlobalShortcut::~GlobalShortcut() = default; + +void GlobalShortcut::Dispose() { + is_disposed_ = true; -GlobalShortcut::~GlobalShortcut() { auto* instance = ui::GlobalAcceleratorListener::GetInstance(); if (instance && instance->IsRegistrationHandledExternally()) { // Eagerly cancel callbacks so PruneStaleCommands() can clear them before // the WeakPtrFactory destructor runs. - weak_ptr_factory_.InvalidateWeakPtrs(); + weak_factory_.Invalidate(); instance->PruneStaleCommands(); } - UnregisterAll(); + UnregisterAllInternal(); } void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) { @@ -73,7 +83,9 @@ void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) { } else { // This should never occur, because if it does, // ui::GlobalAcceleratorListener notifies us with wrong accelerator. - NOTREACHED(); + if (!is_disposed_) { + NOTREACHED(); + } } } @@ -84,7 +96,9 @@ void GlobalShortcut::ExecuteCommand(const extensions::ExtensionId& extension_id, } else { // This should never occur, because if it does, GlobalAcceleratorListener // notifies us with wrong command. - NOTREACHED(); + if (!is_disposed_) { + NOTREACHED(); + } } } @@ -112,11 +126,14 @@ bool GlobalShortcut::RegisterAll( bool GlobalShortcut::Register(const ui::Accelerator& accelerator, const base::RepeatingClosure& callback) { + v8::Isolate* const isolate = JavascriptEnvironment::GetIsolate(); + if (!electron::Browser::Get()->is_ready()) { - gin_helper::ErrorThrower(JavascriptEnvironment::GetIsolate()) - .ThrowError("globalShortcut cannot be used before the app is ready"); + gin_helper::ErrorThrower(isolate).ThrowError( + "globalShortcut cannot be used before the app is ready"); return false; } + #if BUILDFLAG(IS_MAC) if (accelerator.IsMediaKey()) { if (RegisteringMediaKeyForUntrustedClient(accelerator)) @@ -170,8 +187,10 @@ bool GlobalShortcut::Register(const ui::Accelerator& accelerator, const std::string fake_extension_id = command_str + "+" + profile_id; instance->OnCommandsChanged( fake_extension_id, profile_id, commands, gfx::kNullAcceleratedWidget, - base::BindRepeating(&GlobalShortcut::ExecuteCommand, - weak_ptr_factory_.GetWeakPtr())); + base::BindRepeating( + &GlobalShortcut::ExecuteCommand, + gin::WrapPersistent(weak_factory_.GetWeakCell( + isolate->GetCppHeap()->GetAllocationHandle())))); command_callback_map_[command_str] = callback; return true; } else { @@ -189,19 +208,24 @@ void GlobalShortcut::Unregister(const ui::Accelerator& accelerator) { .ThrowError("globalShortcut cannot be used before the app is ready"); return; } - if (accelerator_callback_map_.erase(accelerator) == 0) + if (!accelerator_callback_map_.contains(accelerator)) return; + if (ui::GlobalAcceleratorListener::GetInstance()) { + ui::GlobalAcceleratorListener::GetInstance()->UnregisterAccelerator( + accelerator, this); + } + + // Remove from local callback map after unregistering from UI listener to + // avoid reentrancy races where in-flight key notifications arrive while the + // platform listener is stopping. + accelerator_callback_map_.erase(accelerator); + #if BUILDFLAG(IS_MAC) if (accelerator.IsMediaKey() && !MapHasMediaKeys(accelerator_callback_map_)) { ui::GlobalAcceleratorListener::SetShouldUseInternalMediaKeyHandling(true); } #endif - - if (ui::GlobalAcceleratorListener::GetInstance()) { - ui::GlobalAcceleratorListener::GetInstance()->UnregisterAccelerator( - accelerator, this); - } } void GlobalShortcut::UnregisterSome( @@ -226,10 +250,15 @@ void GlobalShortcut::UnregisterAll() { .ThrowError("globalShortcut cannot be used before the app is ready"); return; } - accelerator_callback_map_.clear(); + UnregisterAllInternal(); +} + +void GlobalShortcut::UnregisterAllInternal() { if (ui::GlobalAcceleratorListener::GetInstance()) { ui::GlobalAcceleratorListener::GetInstance()->UnregisterAccelerators(this); } + accelerator_callback_map_.clear(); + command_callback_map_.clear(); } void GlobalShortcut::SetSuspended(bool suspend) { @@ -257,16 +286,15 @@ bool GlobalShortcut::IsSuspended() { } // static -gin_helper::Handle GlobalShortcut::Create( - v8::Isolate* isolate) { - return gin_helper::CreateHandle(isolate, new GlobalShortcut()); +GlobalShortcut* GlobalShortcut::Create(v8::Isolate* isolate) { + return cppgc::MakeGarbageCollected( + isolate->GetCppHeap()->GetAllocationHandle(), isolate); } // static gin::ObjectTemplateBuilder GlobalShortcut::GetObjectTemplateBuilder( v8::Isolate* isolate) { - return gin_helper::DeprecatedWrappable< - GlobalShortcut>::GetObjectTemplateBuilder(isolate) + return gin::Wrappable::GetObjectTemplateBuilder(isolate) .SetMethod("registerAll", &GlobalShortcut::RegisterAll) .SetMethod("register", &GlobalShortcut::Register) .SetMethod("isRegistered", &GlobalShortcut::IsRegistered) @@ -276,8 +304,23 @@ gin::ObjectTemplateBuilder GlobalShortcut::GetObjectTemplateBuilder( .SetMethod("isSuspended", &GlobalShortcut::IsSuspended); } -const char* GlobalShortcut::GetTypeName() { - return "GlobalShortcut"; +void GlobalShortcut::Trace(cppgc::Visitor* visitor) const { + gin::Wrappable::Trace(visitor); + visitor->Trace(weak_factory_); +} + +const gin::WrapperInfo* GlobalShortcut::wrapper_info() const { + return &kWrapperInfo; +} + +const char* GlobalShortcut::GetHumanReadableName() const { + return "Electron / GlobalShortcut"; +} + +void GlobalShortcut::OnBeforeMicrotasksRunnerDispose(v8::Isolate* isolate) { + gin::PerIsolateData* data = gin::PerIsolateData::From(isolate); + data->RemoveDisposeObserver(this); + Dispose(); } } // namespace electron::api @@ -289,8 +332,9 @@ void Initialize(v8::Local exports, v8::Local context, void* priv) { v8::Isolate* const isolate = electron::JavascriptEnvironment::GetIsolate(); - gin::Dictionary dict{isolate, exports}; - dict.Set("globalShortcut", electron::api::GlobalShortcut::Create(isolate)); + gin_helper::Dictionary dict{isolate, exports}; + dict.SetMethod("createGlobalShortcut", + base::BindRepeating(&electron::api::GlobalShortcut::Create)); } } // namespace diff --git a/shell/browser/api/electron_api_global_shortcut.h b/shell/browser/api/electron_api_global_shortcut.h index b7bff4c2e4..b4584b27ce 100644 --- a/shell/browser/api/electron_api_global_shortcut.h +++ b/shell/browser/api/electron_api_global_shortcut.h @@ -11,42 +11,47 @@ #include "base/functional/callback_forward.h" #include "base/memory/weak_ptr.h" #include "extensions/common/extension_id.h" -#include "shell/common/gin_helper/wrappable.h" +#include "gin/per_isolate_data.h" +#include "gin/weak_cell.h" +#include "gin/wrappable.h" #include "ui/base/accelerators/accelerator.h" #include "ui/base/accelerators/global_accelerator_listener/global_accelerator_listener.h" -namespace gin_helper { -template -class Handle; -} // namespace gin_helper - namespace electron::api { -class GlobalShortcut final - : private ui::GlobalAcceleratorListener::Observer, - public gin_helper::DeprecatedWrappable { +class GlobalShortcut final : public gin::Wrappable, + private ui::GlobalAcceleratorListener::Observer, + public gin::PerIsolateData::DisposeObserver { public: - static gin_helper::Handle Create(v8::Isolate* isolate); + static GlobalShortcut* Create(v8::Isolate* isolate); - // gin_helper::Wrappable - static gin::DeprecatedWrapperInfo kWrapperInfo; + // gin::Wrappable + static const gin::WrapperInfo kWrapperInfo; + void Trace(cppgc::Visitor* visitor) const override; + const gin::WrapperInfo* wrapper_info() const override; + const char* GetHumanReadableName() const override; gin::ObjectTemplateBuilder GetObjectTemplateBuilder( v8::Isolate* isolate) override; - const char* GetTypeName() override; + + // gin::PerIsolateData::DisposeObserver + void OnBeforeDispose(v8::Isolate* isolate) override {} + void OnBeforeMicrotasksRunnerDispose(v8::Isolate* isolate) override; + void OnDisposed() override {} + + // Make public for cppgc::MakeGarbageCollected. + explicit GlobalShortcut(v8::Isolate* isolate); + ~GlobalShortcut() override; // disable copy GlobalShortcut(const GlobalShortcut&) = delete; GlobalShortcut& operator=(const GlobalShortcut&) = delete; - protected: - GlobalShortcut(); - ~GlobalShortcut() override; - private: typedef std::map AcceleratorCallbackMap; typedef std::map CommandCallbackMap; + void Dispose(); bool RegisterAll(const std::vector& accelerators, const base::RepeatingClosure& callback); bool Register(const ui::Accelerator& accelerator, @@ -55,6 +60,7 @@ class GlobalShortcut final void Unregister(const ui::Accelerator& accelerator); void UnregisterSome(const std::vector& accelerators); void UnregisterAll(); + void UnregisterAllInternal(); void SetSuspended(bool suspend); bool IsSuspended(); @@ -65,8 +71,9 @@ class GlobalShortcut final AcceleratorCallbackMap accelerator_callback_map_; CommandCallbackMap command_callback_map_; + bool is_disposed_ = false; - base::WeakPtrFactory weak_ptr_factory_{this}; + gin::WeakCellFactory weak_factory_{this}; }; } // namespace electron::api diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 8a4040f0f3..3982aa2485 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -2316,9 +2316,16 @@ void WebContents::DevToolsOpened() { v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); v8::HandleScope handle_scope(isolate); DCHECK(inspectable_web_contents_); - DCHECK(inspectable_web_contents_->GetDevToolsWebContents()); - auto handle = FromOrCreate( - isolate, inspectable_web_contents_->GetDevToolsWebContents()); + + // GetDevToolsWebContents() can be null here when DevTools were closed + // re-entrantly during InspectableWebContents::LoadCompleted() — e.g. when + // a JS handler for `devtools-focused` (fired from the activate path inside + // SetIsDocked) calls closeDevTools() before LoadCompleted finishes. + content::WebContents* const dtwc = GetDevToolsWebContents(); + if (!dtwc) + return; + + auto handle = FromOrCreate(isolate, dtwc); devtools_web_contents_.Reset(isolate, handle.ToV8()); // Set inspected tabID. @@ -2326,12 +2333,11 @@ void WebContents::DevToolsOpened() { "DevToolsAPI", "setInspectedTabId", base::Value(ID())); // Inherit owner window in devtools when it doesn't have one. - auto* devtools = inspectable_web_contents_->GetDevToolsWebContents(); - bool has_window = devtools->GetUserData(NativeWindowRelay::UserDataKey()); - if (owner_window() && !has_window) { - CHECK(!owner_window_.WasInvalidated()); + bool has_window = dtwc->GetUserData(NativeWindowRelay::UserDataKey()); + if (owner_window_ && !has_window) { + DCHECK(!owner_window_.WasInvalidated()); DCHECK_EQ(handle->owner_window(), nullptr); - handle->SetOwnerWindow(devtools, owner_window()); + handle->SetOwnerWindow(dtwc, owner_window()); } Emit("devtools-opened"); @@ -3017,7 +3023,7 @@ void WebContents::InspectElement(int x, int y) { if (!enable_devtools_ || !inspectable_web_contents_) return; - if (!inspectable_web_contents_->GetDevToolsWebContents()) + if (!GetDevToolsWebContents()) OpenDevTools(nullptr); inspectable_web_contents_->InspectElement(x, y); } diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index 1c52d002d4..75e11ba0cd 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -1912,23 +1912,15 @@ views::ClientView* NativeWindowViews::CreateClientView(views::Widget* widget) { std::unique_ptr NativeWindowViews::CreateFrameView( views::Widget* widget) { #if BUILDFLAG(IS_WIN) - auto frame_view = std::make_unique(); - frame_view->Init(this, widget); - return frame_view; + return std::make_unique(this, widget); #else - if (has_frame() && !has_client_frame()) { - return std::make_unique(this, widget); - } else { - if (has_frame() && has_client_frame()) { - auto frame_view = std::make_unique(); - frame_view->Init(this, widget); - return frame_view; - } else { - auto frame_view = std::make_unique(); - frame_view->Init(this, widget); - return frame_view; - } - } + if (!has_frame()) + return std::make_unique(this, widget); + + if (has_client_frame()) + return std::make_unique(this, widget); + + return std::make_unique(this, widget); #endif } diff --git a/shell/browser/ui/inspectable_web_contents.cc b/shell/browser/ui/inspectable_web_contents.cc index a2da3fa1f8..61ca3dd869 100644 --- a/shell/browser/ui/inspectable_web_contents.cc +++ b/shell/browser/ui/inspectable_web_contents.cc @@ -11,6 +11,7 @@ #include #include +#include "base/auto_reset.h" #include "base/base64.h" #include "base/containers/fixed_flat_set.h" #include "base/containers/span.h" @@ -448,13 +449,17 @@ void InspectableWebContents::ShowDevTools(bool activate) { } void InspectableWebContents::CloseDevTools() { + if (is_showing_devtools_) { + close_devtools_pending_ = true; + return; + } if (GetDevToolsWebContents()) { frontend_loaded_ = false; + embedder_message_dispatcher_.reset(); if (managed_devtools_web_contents_) { view_->CloseDevTools(); managed_devtools_web_contents_.reset(); } - embedder_message_dispatcher_.reset(); if (!is_guest()) web_contents_->Focus(); } @@ -549,52 +554,74 @@ void InspectableWebContents::CloseWindow() { void InspectableWebContents::LoadCompleted() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - frontend_loaded_ = true; - if (managed_devtools_web_contents_) - view_->ShowDevTools(activate_); + if (!GetDevToolsWebContents()) + return; - // If the devtools can dock, "SetIsDocked" will be called by devtools itself. - if (!can_dock_) { - SetIsDocked(DispatchCallback(), false); - if (!devtools_title_.empty()) { - view_->SetTitle(devtools_title_); - } - } else { - if (dock_state_.empty()) { - const base::DictValue& prefs = - pref_service_->GetDict(kDevToolsPreferences); - const std::string* current_dock_state = - prefs.FindString("currentDockState"); - if (current_dock_state) { - std::string sanitized; - base::RemoveChars(*current_dock_state, "\"", &sanitized); - dock_state_ = IsValidDockState(sanitized) ? sanitized : "right"; - } else { - dock_state_ = "right"; + frontend_loaded_ = true; + + // ShowDevTools and SetIsDocked trigger focus on the DevTools WebContents. + // Focus events fire JS handlers via V8 microtask checkpoints, and those + // handlers can call closeDevTools() re-entrantly. Guard the entire show + // phase so that any re-entrant close is deferred until the stack unwinds. + { + base::AutoReset guard(&is_showing_devtools_, true); + + if (managed_devtools_web_contents_) + view_->ShowDevTools(activate_); + + // If the devtools can dock, "SetIsDocked" will be called by devtools + // itself. + if (!can_dock_) { + SetIsDocked(DispatchCallback(), false); + if (!devtools_title_.empty()) { + view_->SetTitle(devtools_title_); } - } -#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) - auto* api_web_contents = api::WebContents::From(GetWebContents()); - if (api_web_contents) { - auto* win = - static_cast(api_web_contents->owner_window()); - // When WCO is enabled, undock the devtools if the current dock - // position overlaps with the position of window controls to avoid - // broken layout. - if (win && win->IsWindowControlsOverlayEnabled()) { - if (IsAppRTL() && dock_state_ == "left") { - dock_state_ = "undocked"; - } else if (dock_state_ == "right") { - dock_state_ = "undocked"; + } else { + if (dock_state_.empty()) { + const base::DictValue& prefs = + pref_service_->GetDict(kDevToolsPreferences); + const std::string* current_dock_state = + prefs.FindString("currentDockState"); + if (current_dock_state) { + std::string sanitized; + base::RemoveChars(*current_dock_state, "\"", &sanitized); + dock_state_ = IsValidDockState(sanitized) ? sanitized : "right"; + } else { + dock_state_ = "right"; + } + } +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) + auto* api_web_contents = api::WebContents::From(GetWebContents()); + if (api_web_contents) { + auto* win = + static_cast(api_web_contents->owner_window()); + // When WCO is enabled, undock the devtools if the current dock + // position overlaps with the position of window controls to avoid + // broken layout. + if (win && win->IsWindowControlsOverlayEnabled()) { + if (IsAppRTL() && dock_state_ == "left") { + dock_state_ = "undocked"; + } else if (dock_state_ == "right") { + dock_state_ = "undocked"; + } } } - } #endif - std::u16string javascript = base::UTF8ToUTF16( - "EUI.DockController.DockController.instance().setDockSide(\"" + - dock_state_ + "\");"); - GetDevToolsWebContents()->GetPrimaryMainFrame()->ExecuteJavaScript( - javascript, base::NullCallback()); + std::u16string javascript = base::UTF8ToUTF16( + "EUI.DockController.DockController.instance().setDockSide(\"" + + dock_state_ + "\");"); + GetDevToolsWebContents()->GetPrimaryMainFrame()->ExecuteJavaScript( + javascript, base::NullCallback()); + } + } + + // If CloseDevTools was called re-entrantly during the show phase (e.g. from + // a JS devtools-focused handler), execute the deferred close now that the + // focus notification stack has fully unwound. + if (close_devtools_pending_) { + close_devtools_pending_ = false; + CloseDevTools(); + return; } #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) diff --git a/shell/browser/ui/inspectable_web_contents.h b/shell/browser/ui/inspectable_web_contents.h index 7f1393625b..33b6f3107f 100644 --- a/shell/browser/ui/inspectable_web_contents.h +++ b/shell/browser/ui/inspectable_web_contents.h @@ -268,6 +268,14 @@ class InspectableWebContents std::unique_ptr view_; bool frontend_loaded_ = false; + + // Re-entrancy guard: ShowDevTools triggers focus on the DevTools WebContents, + // which fires JS events whose microtask checkpoint can re-entrantly call + // CloseDevTools(). Destroying the WebContents or its widget while the focus + // notification is still iterating observers is a CHECK/UAF. These flags defer + // the close until the show path has fully unwound. + bool is_showing_devtools_ = false; + bool close_devtools_pending_ = false; scoped_refptr agent_host_; std::unique_ptr frontend_host_; std::unique_ptr diff --git a/shell/browser/ui/views/client_frame_view_linux.cc b/shell/browser/ui/views/client_frame_view_linux.cc index de4287147b..4b2341b2de 100644 --- a/shell/browser/ui/views/client_frame_view_linux.cc +++ b/shell/browser/ui/views/client_frame_view_linux.cc @@ -58,8 +58,10 @@ ui::NavButtonProvider::ButtonState ButtonStateToNavButtonProviderState( } // namespace -ClientFrameViewLinux::ClientFrameViewLinux() - : theme_(ui::NativeTheme::GetInstanceForNativeUi()), +ClientFrameViewLinux::ClientFrameViewLinux(NativeWindowViews* window, + views::Widget* frame) + : FramelessView{window, frame}, + theme_{ui::NativeTheme::GetInstanceForNativeUi()}, nav_button_provider_( ui::LinuxUiTheme::GetForProfile(nullptr)->CreateNavButtonProvider()), nav_buttons_{ @@ -101,23 +103,12 @@ ClientFrameViewLinux::ClientFrameViewLinux() ui->AddWindowButtonOrderObserver(this); OnWindowButtonOrderingChange(); } -} - -ClientFrameViewLinux::~ClientFrameViewLinux() { - if (auto* ui = ui::LinuxUi::instance()) - ui->RemoveWindowButtonOrderObserver(this); - theme_->RemoveObserver(this); -} - -void ClientFrameViewLinux::Init(NativeWindowViews* window, - views::Widget* frame) { - FramelessView::Init(window, frame); linux_frame_layout_ = std::make_unique(window); // Unretained() is safe because the subscription is saved into an instance // member and thus will be cancelled upon the instance's destruction. paint_as_active_changed_subscription_ = - frame_->RegisterPaintAsActiveChangedCallback(base::BindRepeating( + frame->RegisterPaintAsActiveChangedCallback(base::BindRepeating( &ClientFrameViewLinux::PaintAsActiveChanged, base::Unretained(this))); UpdateWindowTitle(); @@ -134,6 +125,12 @@ void ClientFrameViewLinux::Init(NativeWindowViews* window, UpdateThemeValues(); } +ClientFrameViewLinux::~ClientFrameViewLinux() { + if (auto* ui = ui::LinuxUi::instance()) + ui->RemoveWindowButtonOrderObserver(this); + theme_->RemoveObserver(this); +} + gfx::Insets ClientFrameViewLinux::RestoredFrameBorderInsets() const { return linux_frame_layout_->RestoredFrameBorderInsets(); } diff --git a/shell/browser/ui/views/client_frame_view_linux.h b/shell/browser/ui/views/client_frame_view_linux.h index 3904be6aef..9feee46864 100644 --- a/shell/browser/ui/views/client_frame_view_linux.h +++ b/shell/browser/ui/views/client_frame_view_linux.h @@ -36,11 +36,9 @@ class ClientFrameViewLinux : public FramelessView, METADATA_HEADER(ClientFrameViewLinux, FramelessView) public: - ClientFrameViewLinux(); + ClientFrameViewLinux(NativeWindowViews* window, views::Widget* frame); ~ClientFrameViewLinux() override; - void Init(NativeWindowViews* window, views::Widget* frame) override; - // FramelessView: gfx::Insets RestoredFrameBorderInsets() const override; LinuxFrameLayout* GetLinuxFrameLayout() const override; diff --git a/shell/browser/ui/views/frameless_view.cc b/shell/browser/ui/views/frameless_view.cc index f573eded6d..8111aaa976 100644 --- a/shell/browser/ui/views/frameless_view.cc +++ b/shell/browser/ui/views/frameless_view.cc @@ -21,15 +21,11 @@ const int kResizeAreaCornerSize = 16; } // namespace -FramelessView::FramelessView() = default; +FramelessView::FramelessView(NativeWindowViews* window, views::Widget* frame) + : window_{window}, frame_{frame} {} FramelessView::~FramelessView() = default; -void FramelessView::Init(NativeWindowViews* window, views::Widget* frame) { - window_ = window; - frame_ = frame; -} - gfx::Insets FramelessView::RestoredFrameBorderInsets() const { return gfx::Insets(); } diff --git a/shell/browser/ui/views/frameless_view.h b/shell/browser/ui/views/frameless_view.h index 7472e7ace7..c7dae12b01 100644 --- a/shell/browser/ui/views/frameless_view.h +++ b/shell/browser/ui/views/frameless_view.h @@ -26,15 +26,13 @@ class FramelessView : public views::FrameView { METADATA_HEADER(FramelessView, views::FrameView) public: - FramelessView(); + FramelessView(NativeWindowViews* window, views::Widget* frame); ~FramelessView() override; // disable copy FramelessView(const FramelessView&) = delete; FramelessView& operator=(const FramelessView&) = delete; - virtual void Init(NativeWindowViews* window, views::Widget* frame); - // Returns whether the |point| is on frameless window's resizing border. virtual int ResizingBorderHitTest(const gfx::Point& point); @@ -80,8 +78,8 @@ class FramelessView : public views::FrameView { gfx::Size GetMaximumSize() const override; // Not owned. - raw_ptr window_ = nullptr; - raw_ptr frame_ = nullptr; + const raw_ptr window_; + const raw_ptr frame_; }; } // namespace electron diff --git a/shell/browser/ui/views/opaque_frame_view.cc b/shell/browser/ui/views/opaque_frame_view.cc index 735912b9f0..d0ee48952f 100644 --- a/shell/browser/ui/views/opaque_frame_view.cc +++ b/shell/browser/ui/views/opaque_frame_view.cc @@ -57,12 +57,10 @@ const int kCaptionButtonBottomPadding = 3; // The content edge images have a shadow built into them. const int OpaqueFrameView::kContentEdgeShadowThickness = 2; -OpaqueFrameView::OpaqueFrameView() - : frame_background_(std::make_unique()) {} -OpaqueFrameView::~OpaqueFrameView() = default; - -void OpaqueFrameView::Init(NativeWindowViews* window, views::Widget* frame) { - FramelessView::Init(window, frame); +OpaqueFrameView::OpaqueFrameView(NativeWindowViews* window, + views::Widget* frame) + : FramelessView{window, frame}, + frame_background_{std::make_unique()} { linux_frame_layout_ = LinuxFrameLayout::Create( window, window->HasShadow(), LinuxFrameLayout::CSDStyle::kCustom); @@ -100,6 +98,7 @@ void OpaqueFrameView::Init(NativeWindowViews* window, views::Widget* frame) { base::Unretained(frame), views::Widget::ClosedReason::kCloseButtonClicked)); } +OpaqueFrameView::~OpaqueFrameView() = default; int OpaqueFrameView::ResizingBorderHitTest(const gfx::Point& point) { return ResizingBorderHitTestImpl( diff --git a/shell/browser/ui/views/opaque_frame_view.h b/shell/browser/ui/views/opaque_frame_view.h index 18ed4f358c..f64719849d 100644 --- a/shell/browser/ui/views/opaque_frame_view.h +++ b/shell/browser/ui/views/opaque_frame_view.h @@ -37,11 +37,10 @@ class OpaqueFrameView : public FramelessView { static constexpr int kNonClientExtraTopThickness = 1; - OpaqueFrameView(); + OpaqueFrameView(NativeWindowViews* window, views::Widget* frame); ~OpaqueFrameView() override; // FramelessView: - void Init(NativeWindowViews* window, views::Widget* frame) override; int ResizingBorderHitTest(const gfx::Point& point) override; void InvalidateCaptionButtons() override; gfx::Insets RestoredFrameBorderInsets() const override; diff --git a/shell/browser/ui/views/win_frame_view.cc b/shell/browser/ui/views/win_frame_view.cc index c76a07a849..aad1ca1503 100644 --- a/shell/browser/ui/views/win_frame_view.cc +++ b/shell/browser/ui/views/win_frame_view.cc @@ -25,20 +25,16 @@ namespace electron { -WinFrameView::WinFrameView() = default; - -WinFrameView::~WinFrameView() = default; - -void WinFrameView::Init(NativeWindowViews* window, views::Widget* frame) { - window_ = window; - frame_ = frame; - +WinFrameView::WinFrameView(NativeWindowViews* window, views::Widget* frame) + : FramelessView{window, frame} { if (window->IsWindowControlsOverlayEnabled()) { caption_button_container_ = AddChildView(std::make_unique(this)); } } +WinFrameView::~WinFrameView() = default; + void WinFrameView::InvalidateCaptionButtons() { if (!caption_button_container_) return; @@ -261,7 +257,7 @@ void WinFrameView::LayoutWindowControlsOverlay() { window()->NotifyLayoutWindowControlsOverlay(); } -bool WinFrameView::GetShouldPaintAsActive() { +bool WinFrameView::GetShouldPaintAsActive() const { return ShouldPaintAsActive(); } diff --git a/shell/browser/ui/views/win_frame_view.h b/shell/browser/ui/views/win_frame_view.h index b0dc84463d..423a1ab59d 100644 --- a/shell/browser/ui/views/win_frame_view.h +++ b/shell/browser/ui/views/win_frame_view.h @@ -23,10 +23,9 @@ class WinFrameView : public FramelessView { METADATA_HEADER(WinFrameView, FramelessView) public: - WinFrameView(); + WinFrameView(NativeWindowViews* window, views::Widget* frame); ~WinFrameView() override; - void Init(NativeWindowViews* window, views::Widget* frame) override; void InvalidateCaptionButtons() override; // views::FrameView: @@ -50,7 +49,7 @@ class WinFrameView : public FramelessView { int TitlebarMaximizedVisualHeight() const; // Returns true if the frame should be painted as active. - bool GetShouldPaintAsActive(); + [[nodiscard]] bool GetShouldPaintAsActive() const; protected: // views::View: diff --git a/shell/common/gin_helper/wrappable_pointer_tags.h b/shell/common/gin_helper/wrappable_pointer_tags.h index e7ba3d93c6..edb08777ac 100644 --- a/shell/common/gin_helper/wrappable_pointer_tags.h +++ b/shell/common/gin_helper/wrappable_pointer_tags.h @@ -16,6 +16,7 @@ enum ElectronWrappablePointerTag : uint16_t { kElectronDataPipeHolder, // electron::api::DataPipeHolder kElectronDebugger, // electron::api::Debugger kElectronEvent, // gin_helper::internal::Event + kElectronGlobalShortcut, // electron::api::GlobalShortcut kElectronExtensions, // electron::api::Extensions kElectronMenu, // electron::api::Menu kElectronNetLog, // electron::api::NetLog diff --git a/spec/api-autoupdater-darwin-spec.ts b/spec/api-autoupdater-darwin-spec.ts index bb9f152dab..1eef32038b 100644 --- a/spec/api-autoupdater-darwin-spec.ts +++ b/spec/api-autoupdater-darwin-spec.ts @@ -391,7 +391,8 @@ ifdescribe(shouldRunCodesignTests)('autoUpdater behavior', function () { ); }); - it('should hit the download endpoint when an update is available and update successfully when the zip is provided even after a different update was staged', async () => { + it('should hit the download endpoint when an update is available and update successfully when the zip is provided even after a different update was staged', async function () { + this.timeout(180000); await withUpdatableApp( { nextVersion: '2.0.0', @@ -537,7 +538,8 @@ ifdescribe(shouldRunCodesignTests)('autoUpdater behavior', function () { ); }); - it('should keep the update directory count bounded across repeated checks', async () => { + it('should keep the update directory count bounded across repeated checks', async function () { + this.timeout(240000); // Verifies the orphan prune actually fires: after a second download // completes and rewrites ShipItState.plist, the first directory is no // longer referenced and must be removed when a third check begins. diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts index ff18699e24..bc69cea867 100644 --- a/spec/api-web-contents-spec.ts +++ b/spec/api-web-contents-spec.ts @@ -1267,6 +1267,22 @@ describe('webContents module', () => { await devtoolsOpened2; expect(w.webContents.isDevToolsOpened()).to.be.true(); }); + + it('does not crash when closing DevTools immediately after opening', async () => { + const w = new BrowserWindow({ show: true }); + await w.loadURL('about:blank'); + + const devToolsFocused = once(w.webContents, 'devtools-focused'); + w.webContents.openDevTools({ mode: 'detach' }); + w.webContents.inspectElement(100, 100); + await devToolsFocused; + + const devtoolsClosed = once(w.webContents, 'devtools-closed'); + w.webContents.closeDevTools(); + await devtoolsClosed; + + expect(w.webContents.isDevToolsOpened()).to.be.false(); + }); }); describe('setDevToolsTitle() API', () => { diff --git a/spec/fixtures/auto-update/update-triple-stack/index.js b/spec/fixtures/auto-update/update-triple-stack/index.js index ad6a08aa18..d57b673252 100644 --- a/spec/fixtures/auto-update/update-triple-stack/index.js +++ b/spec/fixtures/auto-update/update-triple-stack/index.js @@ -46,7 +46,7 @@ if (feedUrl === 'remain-open') { } else { setTimeout(() => { autoUpdater.checkForUpdates(); - }, 1000); + }, 5000); } }); diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index bde26ae95b..f9e090e869 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -243,7 +243,7 @@ declare namespace NodeJS { _linkedBinding(name: 'electron_browser_crash_reporter'): CrashReporterBinding; _linkedBinding(name: 'electron_browser_desktop_capturer'): { createDesktopCapturer(): ElectronInternal.DesktopCapturer; isDisplayMediaSystemPickerAvailable(): boolean; }; _linkedBinding(name: 'electron_browser_event_emitter'): { setEventEmitterPrototype(prototype: Object): void; }; - _linkedBinding(name: 'electron_browser_global_shortcut'): { globalShortcut: Electron.GlobalShortcut }; + _linkedBinding(name: 'electron_browser_global_shortcut'): { createGlobalShortcut(): Electron.GlobalShortcut }; _linkedBinding(name: 'electron_browser_image_view'): { ImageView: any }; _linkedBinding(name: 'electron_browser_in_app_purchase'): { inAppPurchase: Electron.InAppPurchase }; _linkedBinding(name: 'electron_browser_message_port'): { createPair(): { port1: Electron.MessagePortMain, port2: Electron.MessagePortMain }; };