fix: window.open not overriding parent's webPreferences

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);
       }
     });
 

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,

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 StructTraits<blink::mojom::WebPreferencesDataView,
@@ -49,7 +50,7 @@ index e9f2e215ee1220c66549112982df04201c68fe1a..a8e08adfdeaf3acde4d190766b65ad3f
        !data.ReadLazyFrameLoadingDistanceThresholdsPx(
            &out->lazy_frame_loading_distance_thresholds_px) ||
        !data.ReadLazyImageLoadingDistanceThresholdsPx(
-@@ -158,6 +162,21 @@ bool StructTraits<blink::mojom::WebPreferencesDataView,
+@@ -158,6 +162,22 @@ bool StructTraits<blink::mojom::WebPreferencesDataView,
        data.fake_no_alloc_direct_call_for_testing_enabled();
    out->v8_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 StructTraits<blink::mojom::WebPreferencesDataView,
+@@ -456,6 +457,72 @@ struct BLINK_COMMON_EXPORT StructTraits<blink::mojom::WebPreferencesDataView,
      return r.record_whole_document;
    }
 
@@ -184,13 +187,17 @@ index e0084598921ddcb0cf2aeb33875f05da0b5457e9..90bf73d7a1f2daf921f5a0ae9e4b3c29
 +  static bool webview_tag(const blink::web_pref::WebPreferences& r) {
 +    return r.webview_tag;
 +  }
++
++  static bool browser_side_navigation(const blink::web_pref::WebPreferences& r) {
++    return r.browser_side_navigation;
++  }
 +  // End Electron-specific WebPreferences.
 +
    static bool cookie_enabled(const blink::web_pref::WebPreferences& r) {
      return r.cookie_enabled;
    }
 diff --git a/third_party/blink/public/mojom/webpreferences/web_preferences.mojom b/third_party/blink/public/mojom/webpreferences/web_preferences.mojom
-index af5ba07466b95ab61befdd3994d731ed8f7ef0a6..2623a82a333767f789da2e952adb2bc1997ca4fd 100644
+index af5ba07466b95ab61befdd3994d731ed8f7ef0a6..6b9650c0be064d2c2a56b38166c1df7a3388f688 100644
 --- a/third_party/blink/public/mojom/webpreferences/web_preferences.mojom
 +++ b/third_party/blink/public/mojom/webpreferences/web_preferences.mojom
 @@ -10,6 +10,7 @@ import "third_party/blink/public/mojom/v8_cache_options.mojom";
@@ -201,7 +208,7 @@ index af5ba07466b95ab61befdd3994d731ed8f7ef0a6..2623a82a333767f789da2e952adb2bc1
 
  enum PointerType {
    kPointerNone                              = 1,             // 1 << 0
-@@ -215,6 +216,24 @@ struct WebPreferences {
+@@ -215,6 +216,25 @@ struct WebPreferences {
    V8CacheOptions v8_cache_options;
    bool record_whole_document;
 
@@ -221,6 +228,7 @@ index af5ba07466b95ab61befdd3994d731ed8f7ef0a6..2623a82a333767f789da2e952adb2bc1
 +  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

patches/chromium/can_create_window.patch

@@ -150,7 +150,7 @@ index 177bc1230a1aebd367dca42e92b63fb8be482805..f82d5c524ebf331c37280f0a93a5d3dc
    // typically happens when popups are created.
    virtual void WebContentsCreated(WebContents* source_contents,
 diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
-index 9eb0bc6d5460f640dc95cc170c9808b8e3f5fb16..190b517cea51bd3eae29695ba45efb22c4c82877 100644
+index 9eb0bc6d5460f640dc95cc170c9808b8e3f5fb16..d2c7f6f90a9ca5b70ce891af3af5232a9f49e8d1 100644
 --- a/content/renderer/render_view_impl.cc
 +++ b/content/renderer/render_view_impl.cc
 @@ -31,6 +31,7 @@
@@ -172,6 +172,17 @@ index 9eb0bc6d5460f640dc95cc170c9808b8e3f5fb16..190b517cea51bd3eae29695ba45efb22
    params->download_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

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);

shell/renderer/electron_render_frame_observer.cc

@@ -58,20 +58,61 @@ 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<v8::Context> 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<v8::Context> 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);
 
   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

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_;
 };

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'));

spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html

@@ -1,21 +1,21 @@
 <html>
 <body>
 <script>
-// `setImmediate` schedules a function to be run on the next UV tick, which
-//  ensures that `window.open` is run from `UvRunOnce()`.
-setImmediate(async () => {
-  if (!location.hash) {
+if (location.hash) {
+  window.opener.postMessage('foo', '*');
+} else {
+  // `setImmediate` schedules a function to be run on the next UV tick, which
+  //  ensures that `window.open` is run from `UvRunOnce()`.
+  setImmediate(async () => {
     const p = new Promise(resolve => {
       window.addEventListener('message', resolve, { once: true });
     });
     window.open('#foo', '', 'nodeIntegration=no,show=no');
     const e = await p;
 
     window.close();
-  } else {
-    window.opener.postMessage('foo', '*');
-  }
-});
+  });
+}
 </script>
 </body>
 </html>