From dd5ee28b2ff8389239089c7876a76d1a7950f998 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 23 Aug 2016 09:51:54 -0700 Subject: [PATCH 1/4] Access window through native view --- atom/browser/api/atom_api_web_contents.cc | 2 +- atom/browser/api/atom_api_web_contents_mac.mm | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index e77338253f..b1ef28e731 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1175,7 +1175,7 @@ bool WebContents::IsFocused() const { if (!view) return false; if (GetType() != BACKGROUND_PAGE) { - auto window = web_contents()->GetTopLevelNativeWindow(); + auto window = web_contents()->GetNativeView()->GetTopLevelWindow(); if (window && !window->IsVisible()) return false; } diff --git a/atom/browser/api/atom_api_web_contents_mac.mm b/atom/browser/api/atom_api_web_contents_mac.mm index 913951737f..c60639f161 100644 --- a/atom/browser/api/atom_api_web_contents_mac.mm +++ b/atom/browser/api/atom_api_web_contents_mac.mm @@ -4,9 +4,7 @@ #include "atom/browser/api/atom_api_web_contents.h" -@interface NSWindow -- (BOOL)isKeyWindow; -@end +#import namespace atom { @@ -17,7 +15,7 @@ bool WebContents::IsFocused() const { if (!view) return false; if (GetType() != BACKGROUND_PAGE) { - auto window = web_contents()->GetTopLevelNativeWindow(); + auto window = [web_contents()->GetNativeView() window]; // On Mac the render widget host view does not lose focus when the window // loses focus so check if the top level window is the key window. if (window && ![window isKeyWindow]) From 399470e281e4bdb1832e3ac32d6fe9eb87f4d4d2 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 23 Aug 2016 10:06:05 -0700 Subject: [PATCH 2/4] Add spec for detached window crash --- spec/api-web-contents-spec.js | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index 64a2f4aac3..7e77c39bdb 100644 --- a/spec/api-web-contents-spec.js +++ b/spec/api-web-contents-spec.js @@ -49,11 +49,9 @@ describe('webContents module', function () { }) describe('getFocusedWebContents() API', function () { - if (isCi) { - return - } - it('returns the focused web contents', function (done) { + if (isCi) return done() + const specWebContents = remote.getCurrentWebContents() assert.equal(specWebContents.getId(), webContents.getFocusedWebContents().getId()) @@ -69,6 +67,27 @@ describe('webContents module', function () { specWebContents.openDevTools() }) + + it('does not crash when called on a detached dev tools window', function (done) { + const specWebContents = w.webContents + + specWebContents.once('devtools-opened', function () { + assert.doesNotThrow(function () { + webContents.getFocusedWebContents() + }) + specWebContents.closeDevTools() + }) + + specWebContents.once('devtools-closed', function () { + assert.doesNotThrow(function () { + webContents.getFocusedWebContents() + }) + done() + }) + + specWebContents.openDevTools({mode: 'detach'}) + w.inspectElement(100, 100) + }) }) describe('isFocused() API', function () { From 463b3de199401bda2de48d3e065adbd2fa473da0 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 23 Aug 2016 10:12:55 -0700 Subject: [PATCH 3/4] Use correct GetToplevelWindow signature --- atom/browser/api/atom_api_web_contents.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index b1ef28e731..f04e496f9a 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1175,7 +1175,7 @@ bool WebContents::IsFocused() const { if (!view) return false; if (GetType() != BACKGROUND_PAGE) { - auto window = web_contents()->GetNativeView()->GetTopLevelWindow(); + auto window = web_contents()->GetNativeView()->GetToplevelWindow(); if (window && !window->IsVisible()) return false; } From 21de91d6e2356d6a61d309bf6d544faf96c645f7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 23 Aug 2016 15:26:13 -0700 Subject: [PATCH 4/4] Don't open dev tools if already initialized --- atom/browser/api/atom_api_web_contents.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index f04e496f9a..d9722f3cc5 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1004,7 +1004,8 @@ void WebContents::InspectElement(int x, int y) { if (type_ == REMOTE) return; - OpenDevTools(nullptr); + if (!managed_web_contents()->GetDevToolsWebContents()) + OpenDevTools(nullptr); scoped_refptr agent( content::DevToolsAgentHost::GetOrCreateFor(web_contents())); agent->InspectElement(x, y);