diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index 959e16eb38205..e0336ccf62885 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -680,16 +680,6 @@ WebContents.prototype._init = function () { postBody }; windowOpenOverriddenOptions = this._callWindowOpenHandler(event, details); - // if attempting to use this API with the deprecated new-window event, - // windowOpenOverriddenOptions will always return null. This ensures - // short-term backwards compatibility until new-window is removed. - const parsedFeatures = parseFeatures(rawFeatures); - const overriddenFeatures: BrowserWindowConstructorOptions = { - ...parsedFeatures.options, - webPreferences: parsedFeatures.webPreferences - }; - windowOpenOverriddenOptions = windowOpenOverriddenOptions || overriddenFeatures; - if (!event.defaultPrevented) { const secureOverrideWebPreferences = windowOpenOverriddenOptions ? { // Allow setting of backgroundColor as a webPreference even though @@ -699,9 +689,19 @@ WebContents.prototype._init = function () { transparent: windowOpenOverriddenOptions.transparent, ...windowOpenOverriddenOptions.webPreferences } : undefined; - this._setNextChildWebPreferences( - makeWebPreferences({ embedder: event.sender, secureOverrideWebPreferences }) - ); + // TODO(zcbenz): The features string is parsed twice: here where it is + // passed to C++, and in |makeBrowserWindowOptions| later where it is + // not actually used since the WebContents is created here. + // We should be able to remove the latter once the |nativeWindowOpen| + // option is removed. + const { webPreferences: parsedWebPreferences } = parseFeatures(rawFeatures); + // Parameters should keep same with |makeBrowserWindowOptions|. + const webPreferences = makeWebPreferences({ + embedder: event.sender, + insecureParsedWebPreferences: parsedWebPreferences, + secureOverrideWebPreferences + }); + this._setNextChildWebPreferences(webPreferences); } }); diff --git a/lib/browser/guest-window-manager.ts b/lib/browser/guest-window-manager.ts index 08b2ecbc92137..980cc705912eb 100644 --- a/lib/browser/guest-window-manager.ts +++ b/lib/browser/guest-window-manager.ts @@ -217,6 +217,10 @@ function makeBrowserWindowOptions ({ embedder, features, overrideOptions }: { height: 600, ...parsedOptions, ...overrideOptions, + // Note that for |nativeWindowOpen: true| the WebContents is created in + // |api::WebContents::WebContentsCreatedWithFullParams|, with prefs + // parsed in the |-will-add-new-contents| event. + // The |webPreferences| here is only used by |nativeWindowOpen: false|. webPreferences: makeWebPreferences({ embedder, insecureParsedWebPreferences: parsedWebPreferences, diff --git a/patches/chromium/allow_in-process_windows_to_have_different_web_prefs.patch b/patches/chromium/allow_in-process_windows_to_have_different_web_prefs.patch index 3963d32c0b057..07f6bee6b8375 100644 --- a/patches/chromium/allow_in-process_windows_to_have_different_web_prefs.patch +++ b/patches/chromium/allow_in-process_windows_to_have_different_web_prefs.patch @@ -8,10 +8,10 @@ WebPreferences of in-process child windows, rather than relying on process-level command line switches, as before. diff --git a/third_party/blink/common/web_preferences/web_preferences.cc b/third_party/blink/common/web_preferences/web_preferences.cc -index db2d0536ed7a8143a60cebf1c5d7fee1acf4d10d..6cea8d7ce6ff75ae80a4db03c25f913915624342 100644 +index db2d0536ed7a8143a60cebf1c5d7fee1acf4d10d..b04b5bbd05af67038995d723c058fc96aaeddc92 100644 --- a/third_party/blink/common/web_preferences/web_preferences.cc +++ b/third_party/blink/common/web_preferences/web_preferences.cc -@@ -145,6 +145,22 @@ WebPreferences::WebPreferences() +@@ -145,6 +145,23 @@ WebPreferences::WebPreferences() fake_no_alloc_direct_call_for_testing_enabled(false), v8_cache_options(blink::mojom::V8CacheOptions::kDefault), record_whole_document(false), @@ -30,12 +30,13 @@ index db2d0536ed7a8143a60cebf1c5d7fee1acf4d10d..6cea8d7ce6ff75ae80a4db03c25f9139 + enable_plugins(false), + enable_websql(false), + webview_tag(false), ++ browser_side_navigation(true), + // End Electron-specific WebPreferences. cookie_enabled(true), accelerated_video_decode_enabled(false), animation_policy( diff --git a/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc b/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc -index e9f2e215ee1220c66549112982df04201c68fe1a..a8e08adfdeaf3acde4d190766b65ad3fbacfdf58 100644 +index e9f2e215ee1220c66549112982df04201c68fe1a..3fac0a75bb5550b5adb5e6b6d0229157bdc55658 100644 --- a/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc +++ b/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc @@ -23,6 +23,10 @@ bool StructTraitslazy_frame_loading_distance_thresholds_px) || !data.ReadLazyImageLoadingDistanceThresholdsPx( -@@ -158,6 +162,21 @@ bool StructTraitsv8_cache_options = data.v8_cache_options(); out->record_whole_document = data.record_whole_document(); @@ -67,12 +68,13 @@ index e9f2e215ee1220c66549112982df04201c68fe1a..a8e08adfdeaf3acde4d190766b65ad3f + out->enable_plugins = data.enable_plugins(); + out->enable_websql = data.enable_websql(); + out->webview_tag = data.webview_tag(); ++ out->browser_side_navigation = data.browser_side_navigation(); + // End Electron-specific WebPreferences.s out->cookie_enabled = data.cookie_enabled(); out->accelerated_video_decode_enabled = data.accelerated_video_decode_enabled(); diff --git a/third_party/blink/public/common/web_preferences/web_preferences.h b/third_party/blink/public/common/web_preferences/web_preferences.h -index 0d74719b2f8fb91f094772fab96a880cc8787c77..bc447e53a87852aac03fd2487b77aa1009472d36 100644 +index 0d74719b2f8fb91f094772fab96a880cc8787c77..133eceb96f0d51012b0347ebbb77fd63e8bc8f76 100644 --- a/third_party/blink/public/common/web_preferences/web_preferences.h +++ b/third_party/blink/public/common/web_preferences/web_preferences.h @@ -10,6 +10,7 @@ @@ -83,7 +85,7 @@ index 0d74719b2f8fb91f094772fab96a880cc8787c77..bc447e53a87852aac03fd2487b77aa10 #include "net/nqe/effective_connection_type.h" #include "third_party/blink/public/common/common_export.h" #include "third_party/blink/public/mojom/css/preferred_color_scheme.mojom-shared.h" -@@ -163,6 +164,24 @@ struct BLINK_COMMON_EXPORT WebPreferences { +@@ -163,6 +164,25 @@ struct BLINK_COMMON_EXPORT WebPreferences { blink::mojom::V8CacheOptions v8_cache_options; bool record_whole_document; @@ -103,13 +105,14 @@ index 0d74719b2f8fb91f094772fab96a880cc8787c77..bc447e53a87852aac03fd2487b77aa10 + bool enable_plugins; + bool enable_websql; + bool webview_tag; ++ bool browser_side_navigation; + // End Electron-specific WebPreferences. + // This flags corresponds to a Page's Settings' setCookieEnabled state. It // only controls whether or not the "document.cookie" field is properly // connected to the backing store, for instance if you wanted to be able to diff --git a/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h b/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h -index e0084598921ddcb0cf2aeb33875f05da0b5457e9..90bf73d7a1f2daf921f5a0ae9e4b3c292efaaaa0 100644 +index e0084598921ddcb0cf2aeb33875f05da0b5457e9..594f873622b3ed295cf8d3b7d8e876e656ea5da9 100644 --- a/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h +++ b/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h @@ -6,6 +6,7 @@ @@ -120,7 +123,7 @@ index e0084598921ddcb0cf2aeb33875f05da0b5457e9..90bf73d7a1f2daf921f5a0ae9e4b3c29 #include "mojo/public/cpp/bindings/struct_traits.h" #include "net/nqe/effective_connection_type.h" #include "third_party/blink/public/common/common_export.h" -@@ -456,6 +457,68 @@ struct BLINK_COMMON_EXPORT StructTraitsdownload_policy.ApplyDownloadFramePolicy( /*is_opener_navigation=*/false, request.HasUserGesture(), // `openee_can_access_opener_origin` only matters for opener navigations, +@@ -352,6 +357,10 @@ WebView* RenderViewImpl::CreateView( + view_params->web_preferences = webview_->GetWebPreferences(); + view_params->view_id = reply->route_id; + ++ // Hint that this is a renderer side navigation, Electron will override this ++ // when doing a browser side navigation later. ++ view_params->web_preferences.browser_side_navigation = false; ++ + view_params->replication_state = blink::mojom::FrameReplicationState::New(); + view_params->replication_state->frame_policy.sandbox_flags = sandbox_flags; + view_params->replication_state->name = frame_name_utf8; diff --git a/content/web_test/browser/web_test_content_browser_client.cc b/content/web_test/browser/web_test_content_browser_client.cc index 99d4577526d64e4a73591be4b5bb4d67826abb1a..213db9dc65d10d70b6e02ee3b9b95d38bd951ba3 100644 --- a/content/web_test/browser/web_test_content_browser_client.cc diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index 80e3844b5b414..8656df750be3c 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/shell/browser/web_contents_preferences.cc @@ -521,6 +521,9 @@ void WebContentsPreferences::OverrideWebkitPrefs( prefs->enable_websql = enable_websql_; prefs->v8_cache_options = v8_cache_options_; + + // This is a browser side navigation if we are overriding prefs in browser. + prefs->browser_side_navigation = true; } WEB_CONTENTS_USER_DATA_KEY_IMPL(WebContentsPreferences); diff --git a/shell/renderer/electron_render_frame_observer.cc b/shell/renderer/electron_render_frame_observer.cc index 0f10e16fed207..5d3f11d6f9a8d 100644 --- a/shell/renderer/electron_render_frame_observer.cc +++ b/shell/renderer/electron_render_frame_observer.cc @@ -58,12 +58,54 @@ ElectronRenderFrameObserver::ElectronRenderFrameObserver( } void ElectronRenderFrameObserver::DidClearWindowObject() { + // Do a delayed Node.js initialization for child window. + // Check DidInstallConditionalFeatures below for the background. + auto* web_frame = render_frame_->GetWebFrame(); + if (has_delayed_node_initialization_ && web_frame->Opener() && + render_frame_->GetBlinkPreferences().browser_side_navigation) { + v8::Isolate* isolate = blink::MainThreadIsolate(); + v8::HandleScope handle_scope(isolate); + v8::MicrotasksScope microtasks_scope( + isolate, v8::MicrotasksScope::kDoNotRunMicrotasks); + v8::Handle context = web_frame->MainWorldScriptContext(); + v8::Context::Scope context_scope(context); + // DidClearWindowObject only emits for the main world. + DidInstallConditionalFeatures(context, MAIN_WORLD_ID); + } + renderer_client_->DidClearWindowObject(render_frame_); } void ElectronRenderFrameObserver::DidInstallConditionalFeatures( v8::Handle context, int world_id) { + // When a child window is created with window.open, its WebPreferences will + // be copied from its parent, and Chromium will intialize JS context in it + // immediately. + // Normally the WebPreferences is overriden in browser before navigation, + // but this behavior bypasses the browser side navigation and the child + // window will get wrong WebPreferences in the initialization. + // This will end up initializing Node.js in the child window with wrong + // WebPreferences, leads to problem that child window having node integration + // while "nodeIntegration=no" is passed. + // We work around this issue by delaying the child window's initialization of + // Node.js if this is a renderer side navigation, and only do it when the + // acutal page has started to load. + auto prefs = render_frame_->GetBlinkPreferences(); + auto* web_frame = render_frame_->GetWebFrame(); + if (web_frame->Opener() && !prefs.browser_side_navigation) { + // FIXME(zcbenz): Chromium does not do any browser side navigation for + // window.open('about:blank'), so there is no way to override WebPreferences + // of it. We should not delay Node.js initialization as there will be no + // further loadings. + GURL url = render_frame_->GetWebFrame()->GetDocument().Url(); + if (!url.IsAboutBlank()) { + has_delayed_node_initialization_ = true; + return; + } + } + has_delayed_node_initialization_ = false; + auto* isolate = context->GetIsolate(); v8::MicrotasksScope microtasks_scope( isolate, v8::MicrotasksScope::kDoNotRunMicrotasks); @@ -71,7 +113,6 @@ void ElectronRenderFrameObserver::DidInstallConditionalFeatures( if (ShouldNotifyClient(world_id)) renderer_client_->DidCreateScriptContext(context, render_frame_); - auto prefs = render_frame_->GetBlinkPreferences(); bool use_context_isolation = prefs.context_isolation; // This logic matches the EXPLAINED logic in electron_renderer_client.cc // to avoid explaining it twice go check that implementation in diff --git a/shell/renderer/electron_render_frame_observer.h b/shell/renderer/electron_render_frame_observer.h index bbfd4970489c6..b7a3718172ac7 100644 --- a/shell/renderer/electron_render_frame_observer.h +++ b/shell/renderer/electron_render_frame_observer.h @@ -44,6 +44,7 @@ class ElectronRenderFrameObserver : public content::RenderFrameObserver { void OnTakeHeapSnapshot(IPC::PlatformFileForTransit file_handle, const std::string& channel); + bool has_delayed_node_initialization_ = false; content::RenderFrame* render_frame_; RendererClientBase* renderer_client_; }; diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index b6e4546ffb94b..951c158fb969b 100644 --- a/spec-main/chromium-spec.ts +++ b/spec-main/chromium-spec.ts @@ -814,6 +814,17 @@ describe('chromium features', () => { expect(typeofProcessGlobal).to.equal('undefined'); }); + it('can disable node integration when it is enabled on the parent window with nativeWindowOpen: true', async () => { + const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, nativeWindowOpen: true } }); + w.loadURL('about:blank'); + w.webContents.executeJavaScript(` + { b = window.open('about:blank', '', 'nodeIntegration=no,show=no'); null } + `); + const [, contents] = await emittedOnce(app, 'web-contents-created'); + const typeofProcessGlobal = await contents.executeJavaScript('typeof process'); + expect(typeofProcessGlobal).to.equal('undefined'); + }); + it('disables JavaScript when it is disabled on the parent window', async () => { const w = new BrowserWindow({ show: true, webPreferences: { nodeIntegration: true } }); w.webContents.loadFile(path.resolve(__dirname, 'fixtures', 'blank.html')); diff --git a/spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html b/spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html index 9bb131aef4548..f0c803fc08c8e 100644 --- a/spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html +++ b/spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html @@ -1,10 +1,12 @@