diff --git a/docs/api/window-open.md b/docs/api/window-open.md index dd4105675b95d..039d25836dfe7 100644 --- a/docs/api/window-open.md +++ b/docs/api/window-open.md @@ -64,6 +64,9 @@ window.open('https://github.com', '_blank', 'top=500,left=200,frame=false,nodeIn `features` will be passed to any registered `webContents`'s `did-create-window` event handler in the `options` argument. * `frameName` follows the specification of `windowName` located in the [native documentation](https://developer.mozilla.org/en-US/docs/Web/API/Window/open#parameters). +* When opening `about:blank`, the child window's `WebPreferences` will be copied + from the parent window, and there is no way to override it because Chromium + skips browser side navigation in this case. To customize or cancel the creation of the window, you can optionally set an override handler with `webContents.setWindowOpenHandler()` from the main diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index 8bb863a0eb212..4b05196f7a22b 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -669,16 +669,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 @@ -688,9 +678,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 4547419820e78..8d8ce28a84fde 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/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc index ad786c0f5d1ac..d86acad6de403 100644 --- a/shell/renderer/api/electron_api_web_frame.cc +++ b/shell/renderer/api/electron_api_web_frame.cc @@ -495,6 +495,14 @@ class WebFrameRenderer : public gin::Wrappable, return gin::ConvertToV8(isolate, prefs.context_isolation); } else if (pref_name == options::kGuestInstanceID) { // NOTE: guestInstanceId is internal-only. + // FIXME(zcbenz): For child windows opened with window.open('') from + // webview, the WebPreferences is inherited from webview and the value + // of |guest_instance_id| is wrong. + // Please check ElectronRenderFrameObserver::DidInstallConditionalFeatures + // for the background. + auto* web_frame = render_frame->GetWebFrame(); + if (web_frame->Opener()) + return gin::ConvertToV8(isolate, 0); return gin::ConvertToV8(isolate, prefs.guest_instance_id); } else if (pref_name == options::kHiddenPage) { // NOTE: hiddenPage is internal-only. diff --git a/shell/renderer/electron_render_frame_observer.cc b/shell/renderer/electron_render_frame_observer.cc index 0f10e16fed207..9d5911984afc7 100644 --- a/shell/renderer/electron_render_frame_observer.cc +++ b/shell/renderer/electron_render_frame_observer.cc @@ -31,6 +31,7 @@ #include "third_party/blink/public/web/web_element.h" #include "third_party/blink/public/web/web_local_frame.h" #include "third_party/blink/public/web/web_script_source.h" +#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h" // nogncheck #include "ui/base/resource/resource_bundle.h" namespace electron { @@ -58,12 +59,57 @@ ElectronRenderFrameObserver::ElectronRenderFrameObserver( } void ElectronRenderFrameObserver::DidClearWindowObject() { + // Do a delayed Node.js initialization for child window. + // Check DidInstallConditionalFeatures below for the background. + auto* web_frame = + static_cast(render_frame_->GetWebFrame()); + if (has_delayed_node_initialization_ && web_frame->Opener() && + web_frame->HasCommittedFirstRealLoad()) { + 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 the initial empty document, and only do it when the + // acutal page has started to load. + auto* web_frame = + static_cast(render_frame_->GetWebFrame()); + if (web_frame->Opener() && !web_frame->HasCommittedFirstRealLoad()) { + // 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. + // Please check http://crbug.com/1215096 for updates which may help remove + // this hack. + GURL url = web_frame->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); diff --git a/shell/renderer/electron_render_frame_observer.h b/shell/renderer/electron_render_frame_observer.h index 5366bf256e36e..88b61ecdba8b8 100644 --- a/shell/renderer/electron_render_frame_observer.h +++ b/shell/renderer/electron_render_frame_observer.h @@ -39,6 +39,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/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index a7a98839fb54e..c507bdcffccbb 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -157,6 +157,7 @@ void ElectronRendererClient::WillReleaseScriptContext( // We also do this if we have disable electron site instance overrides to // avoid memory leaks auto prefs = render_frame->GetBlinkPreferences(); + gin_helper::MicrotasksScope microtasks_scope(env->isolate()); node::FreeEnvironment(env); if (env == node_bindings_->uv_env()) node::FreeIsolateData(node_bindings_->isolate_data()); diff --git a/shell/renderer/web_worker_observer.cc b/shell/renderer/web_worker_observer.cc index 2faec5ffc4347..287be9ead13f0 100644 --- a/shell/renderer/web_worker_observer.cc +++ b/shell/renderer/web_worker_observer.cc @@ -37,6 +37,8 @@ WebWorkerObserver::WebWorkerObserver() WebWorkerObserver::~WebWorkerObserver() { lazy_tls.Pointer()->Set(nullptr); + gin_helper::MicrotasksScope microtasks_scope( + node_bindings_->uv_env()->isolate()); node::FreeEnvironment(node_bindings_->uv_env()); node::FreeIsolateData(node_bindings_->isolate_data()); } diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index 322dbe3eff623..b0fa03504055a 100644 --- a/spec-main/chromium-spec.ts +++ b/spec-main/chromium-spec.ts @@ -813,6 +813,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 @@ diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 26e43b12ae749..86737f78860a1 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -154,16 +154,6 @@ describe('chromium feature', () => { }); }); - describe('window.postMessage', () => { - it('throws an exception when the targetOrigin cannot be converted to a string', () => { - const b = window.open(''); - expect(() => { - b.postMessage('test', { toString: null }); - }).to.throw('Cannot convert object to primitive value'); - b.close(); - }); - }); - describe('window.opener.postMessage', () => { it('sets source and origin correctly', async () => { const message = waitForEvent(window, 'message'); diff --git a/spec/static/main.js b/spec/static/main.js index f60970d3998b7..f259a3612fd31 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -109,8 +109,7 @@ app.whenReady().then(async function () { backgroundThrottling: false, nodeIntegration: true, webviewTag: true, - contextIsolation: false, - nativeWindowOpen: false + contextIsolation: false } }); window.loadFile('static/index.html', {