From 11ea40f5fbc2d378589c2e7b8a77d86858fbc37b Mon Sep 17 00:00:00 2001 From: Andy Locascio Date: Thu, 12 Dec 2019 15:01:31 -0800 Subject: [PATCH 1/4] fix: enable workaround for nativeWindowOpen hang --- .../browser/api/electron_api_web_contents.cc | 19 +++++++++++++++++++ shell/browser/api/electron_api_web_contents.h | 13 +++++++++++++ 2 files changed, 32 insertions(+) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index f455a2897c513..1f8decb3004b1 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -654,6 +654,25 @@ void WebContents::WebContentsCreated(content::WebContents* source_contents, tracker->frame_name = frame_name; } +bool WebContents::ShouldCreateWebContents( + content::WebContents* web_contents, + content::RenderFrameHost* opener, + content::SiteInstance* source_site_instance, + int32_t route_id, + int32_t main_frame_route_id, + int32_t main_frame_widget_route_id, + content::mojom::WindowContainerType window_container_type, + const GURL& opener_url, + const std::string& frame_name, + const GURL& target_url, + const std::string& partition_id, + content::SessionStorageNamespace* session_storage_namespace) { + if (Emit("-will-add-new-contents", target_url, frame_name)) { + return false; + } + return true; +} + void WebContents::AddNewContents( content::WebContents* source, std::unique_ptr new_contents, diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index 5566b43b3c3d8..0ad0f272df324 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -376,6 +376,19 @@ class WebContents : public gin_helper::TrackableObject, const base::string16& message, int32_t line_no, const base::string16& source_id) override; + bool ShouldCreateWebContents( + content::WebContents* web_contents, + content::RenderFrameHost* opener, + content::SiteInstance* source_site_instance, + int32_t route_id, + int32_t main_frame_route_id, + int32_t main_frame_widget_route_id, + content::mojom::WindowContainerType window_container_type, + const GURL& opener_url, + const std::string& frame_name, + const GURL& target_url, + const std::string& partition_id, + content::SessionStorageNamespace* session_storage_namespace) override; void WebContentsCreated(content::WebContents* source_contents, int opener_render_process_id, int opener_render_frame_id, From 9cf2421d782b727063c5ad9ab897fdf11ea0a422 Mon Sep 17 00:00:00 2001 From: Andy Locascio Date: Thu, 12 Dec 2019 15:30:16 -0800 Subject: [PATCH 2/4] add test --- spec-main/api-web-contents-spec.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts index 336984c40b197..896555aa3fb29 100644 --- a/spec-main/api-web-contents-spec.ts +++ b/spec-main/api-web-contents-spec.ts @@ -1752,4 +1752,25 @@ describe('webContents module', () => { expect(body).to.equal('401') }) }) + + it('emits a cancelable event before creating a child webcontents', async () => { + const w = new BrowserWindow({ + show: false, + webPreferences: { + sandbox: true + } + }) + w.webContents.on('-will-add-new-contents' as any, (event: any, url: any) => { + expect(url).to.equal('about:blank') + event.preventDefault() + }) + let wasCalled = false + w.webContents.on('new-window' as any, () => { + wasCalled = true + }) + await w.loadURL('about:blank') + await w.webContents.executeJavaScript(`window.open('about:blank')`) + await new Promise((resolve) => { process.nextTick(resolve) }) + expect(wasCalled).to.equal(false) + }) }) From a9092d48c325d8fbbe45dba0c52247158dc4ec0a Mon Sep 17 00:00:00 2001 From: Andy Locascio Date: Thu, 12 Dec 2019 21:22:14 -0800 Subject: [PATCH 3/4] test: ensure window doesn't leak into other test --- spec-main/api-web-contents-spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts index 896555aa3fb29..55546f4fd871d 100644 --- a/spec-main/api-web-contents-spec.ts +++ b/spec-main/api-web-contents-spec.ts @@ -1772,5 +1772,6 @@ describe('webContents module', () => { await w.webContents.executeJavaScript(`window.open('about:blank')`) await new Promise((resolve) => { process.nextTick(resolve) }) expect(wasCalled).to.equal(false) + await closeAllWindows() }) }) From 27f659c4a08fccb675b7348db917857fda077e29 Mon Sep 17 00:00:00 2001 From: Andy Locascio Date: Wed, 18 Mar 2020 16:12:43 -0700 Subject: [PATCH 4/4] update to use new webcontents delegate methods --- .../browser/api/electron_api_web_contents.cc | 25 +++++++++++-------- shell/browser/api/electron_api_web_contents.h | 14 ++++++----- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 1f8decb3004b1..5f95213dfd61b 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -654,23 +654,28 @@ void WebContents::WebContentsCreated(content::WebContents* source_contents, tracker->frame_name = frame_name; } -bool WebContents::ShouldCreateWebContents( - content::WebContents* web_contents, - content::RenderFrameHost* opener, +bool WebContents::IsWebContentsCreationOverridden( content::SiteInstance* source_site_instance, - int32_t route_id, - int32_t main_frame_route_id, - int32_t main_frame_widget_route_id, content::mojom::WindowContainerType window_container_type, const GURL& opener_url, const std::string& frame_name, + const GURL& target_url) { + if (Emit("-will-add-new-contents", target_url, frame_name)) { + return true; + } + return false; +} + +content::WebContents* WebContents::CreateCustomWebContents( + content::RenderFrameHost* opener, + content::SiteInstance* source_site_instance, + bool is_new_browsing_instance, + const GURL& opener_url, + const std::string& frame_name, const GURL& target_url, const std::string& partition_id, content::SessionStorageNamespace* session_storage_namespace) { - if (Emit("-will-add-new-contents", target_url, frame_name)) { - return false; - } - return true; + return nullptr; } void WebContents::AddNewContents( diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index 0ad0f272df324..af54186a3c303 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -376,16 +376,18 @@ class WebContents : public gin_helper::TrackableObject, const base::string16& message, int32_t line_no, const base::string16& source_id) override; - bool ShouldCreateWebContents( - content::WebContents* web_contents, - content::RenderFrameHost* opener, + bool IsWebContentsCreationOverridden( content::SiteInstance* source_site_instance, - int32_t route_id, - int32_t main_frame_route_id, - int32_t main_frame_widget_route_id, content::mojom::WindowContainerType window_container_type, const GURL& opener_url, const std::string& frame_name, + const GURL& target_url) override; + content::WebContents* CreateCustomWebContents( + content::RenderFrameHost* opener, + content::SiteInstance* source_site_instance, + bool is_new_browsing_instance, + const GURL& opener_url, + const std::string& frame_name, const GURL& target_url, const std::string& partition_id, content::SessionStorageNamespace* session_storage_namespace) override;