From f25c1f864b9777f279c8b9eb5843aeaaed18d969 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 13 Jun 2016 17:01:13 -0700 Subject: [PATCH 01/10] Use RenderProcessPreferences for non-remote web contents --- .../atom_api_render_process_preferences.cc | 19 +++++++++---------- .../api/atom_api_render_process_preferences.h | 2 +- atom/browser/api/atom_api_web_contents.cc | 4 ++++ atom/browser/api/atom_api_web_contents.h | 1 + lib/browser/chrome-extension.js | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/atom/browser/api/atom_api_render_process_preferences.cc b/atom/browser/api/atom_api_render_process_preferences.cc index 59ae07b45c..da4cf8f676 100644 --- a/atom/browser/api/atom_api_render_process_preferences.cc +++ b/atom/browser/api/atom_api_render_process_preferences.cc @@ -4,6 +4,7 @@ #include "atom/browser/api/atom_api_render_process_preferences.h" +#include "atom/browser/api/atom_api_web_contents.h" #include "atom/browser/atom_browser_client.h" #include "atom/browser/native_window.h" #include "atom/browser/window_list.h" @@ -19,18 +20,15 @@ namespace api { namespace { -bool IsBrowserWindow(content::RenderProcessHost* process) { +bool IsWebContents(v8::Isolate* isolate, content::RenderProcessHost* process) { content::WebContents* web_contents = static_cast(AtomBrowserClient::Get())-> GetWebContentsFromProcessID(process->GetID()); if (!web_contents) return false; - NativeWindow* window = NativeWindow::FromWebContents(web_contents); - if (!window) - return false; - - return true; + auto api_web_contents = WebContents::CreateFrom(isolate, web_contents); + return !api_web_contents->IsRemote(); } } // namespace @@ -63,10 +61,11 @@ void RenderProcessPreferences::BuildPrototype( // static mate::Handle -RenderProcessPreferences::ForAllBrowserWindow(v8::Isolate* isolate) { +RenderProcessPreferences::ForAllWebContents(v8::Isolate* isolate) { return mate::CreateHandle( isolate, - new RenderProcessPreferences(isolate, base::Bind(&IsBrowserWindow))); + new RenderProcessPreferences(isolate, + base::Bind(&IsWebContents, isolate))); } } // namespace api @@ -78,8 +77,8 @@ namespace { void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { mate::Dictionary dict(context->GetIsolate(), exports); - dict.SetMethod("forAllBrowserWindow", - &atom::api::RenderProcessPreferences::ForAllBrowserWindow); + dict.SetMethod("forAllWebContents", + &atom::api::RenderProcessPreferences::ForAllWebContents); } } // namespace diff --git a/atom/browser/api/atom_api_render_process_preferences.h b/atom/browser/api/atom_api_render_process_preferences.h index a305f1361b..3fa197ed04 100644 --- a/atom/browser/api/atom_api_render_process_preferences.h +++ b/atom/browser/api/atom_api_render_process_preferences.h @@ -17,7 +17,7 @@ class RenderProcessPreferences : public mate::Wrappable { public: static mate::Handle - ForAllBrowserWindow(v8::Isolate* isolate); + ForAllWebContents(v8::Isolate* isolate); static void BuildPrototype(v8::Isolate* isolate, v8::Local prototype); diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 6cb9efb961..21c4359d44 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1197,6 +1197,10 @@ bool WebContents::IsGuest() const { return type_ == WEB_VIEW; } +bool WebContents::IsRemote() const { + return type_ == REMOTE; +} + v8::Local WebContents::GetWebPreferences(v8::Isolate* isolate) { WebContentsPreferences* web_preferences = WebContentsPreferences::FromWebContents(web_contents()); diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index 13c6ce5a7b..075eb3a032 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -139,6 +139,7 @@ class WebContents : public mate::TrackableObject, // Methods for creating . void SetSize(const SetSizeParams& params); bool IsGuest() const; + bool IsRemote() const; // Callback triggered on permission response. void OnEnterFullscreenModeForTab(content::WebContents* source, diff --git a/lib/browser/chrome-extension.js b/lib/browser/chrome-extension.js index 8ba5f29f0d..bff436409b 100644 --- a/lib/browser/chrome-extension.js +++ b/lib/browser/chrome-extension.js @@ -1,6 +1,6 @@ const {app, ipcMain, session, webContents, BrowserWindow} = require('electron') const {getAllWebContents} = process.atomBinding('web_contents') -const renderProcessPreferences = process.atomBinding('render_process_preferences').forAllBrowserWindow() +const renderProcessPreferences = process.atomBinding('render_process_preferences').forAllWebContents() const fs = require('fs') const path = require('path') From fc2b5eebc081fa38e37d3c644c77a4365ad863fe Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 13 Jun 2016 17:08:19 -0700 Subject: [PATCH 02/10] Remove unused includes --- atom/browser/api/atom_api_render_process_preferences.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/atom/browser/api/atom_api_render_process_preferences.cc b/atom/browser/api/atom_api_render_process_preferences.cc index da4cf8f676..6add082963 100644 --- a/atom/browser/api/atom_api_render_process_preferences.cc +++ b/atom/browser/api/atom_api_render_process_preferences.cc @@ -6,8 +6,6 @@ #include "atom/browser/api/atom_api_web_contents.h" #include "atom/browser/atom_browser_client.h" -#include "atom/browser/native_window.h" -#include "atom/browser/window_list.h" #include "atom/common/native_mate_converters/value_converter.h" #include "atom/common/node_includes.h" #include "content/public/browser/render_process_host.h" From c7b2545b1b49b8fad26d4c7ee4da052556682dad Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Jun 2016 09:09:54 -0700 Subject: [PATCH 03/10] Use web contents type enum and add converter --- .../atom_api_render_process_preferences.cc | 2 +- atom/browser/api/atom_api_web_contents.cc | 29 ++++++++++++------- atom/browser/api/atom_api_web_contents.h | 15 +++++----- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/atom/browser/api/atom_api_render_process_preferences.cc b/atom/browser/api/atom_api_render_process_preferences.cc index 6add082963..4781f0603a 100644 --- a/atom/browser/api/atom_api_render_process_preferences.cc +++ b/atom/browser/api/atom_api_render_process_preferences.cc @@ -26,7 +26,7 @@ bool IsWebContents(v8::Isolate* isolate, content::RenderProcessHost* process) { return false; auto api_web_contents = WebContents::CreateFrom(isolate, web_contents); - return !api_web_contents->IsRemote(); + return api_web_contents->GetType() != WebContents::Type::REMOTE; } } // namespace diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 21c4359d44..0591129e98 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -187,6 +187,22 @@ struct Converter { } }; +template<> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + atom::api::WebContents::Type val) { + std::string type = ""; + switch (val) { + case atom::api::WebContents::Type::BROWSER_WINDOW: type = "window"; break; + case atom::api::WebContents::Type::WEB_VIEW: type = "webview"; break; + case atom::api::WebContents::Type::REMOTE: type = "remote"; break; + default: break; + } + return mate::ConvertToV8(isolate, type); + } +}; + + } // namespace mate @@ -744,13 +760,8 @@ int WebContents::GetID() const { return web_contents()->GetRenderProcessHost()->GetID(); } -std::string WebContents::GetType() const { - switch (type_) { - case BROWSER_WINDOW: return "window"; - case WEB_VIEW: return "webview"; - case REMOTE: return "remote"; - default: return ""; - } +WebContents::Type WebContents::GetType() const { + return type_; } bool WebContents::Equal(const WebContents* web_contents) const { @@ -1197,10 +1208,6 @@ bool WebContents::IsGuest() const { return type_ == WEB_VIEW; } -bool WebContents::IsRemote() const { - return type_ == REMOTE; -} - v8::Local WebContents::GetWebPreferences(v8::Isolate* isolate) { WebContentsPreferences* web_preferences = WebContentsPreferences::FromWebContents(web_contents()); diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index 075eb3a032..9bf9dd3ad7 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -43,6 +43,12 @@ class WebContents : public mate::TrackableObject, public CommonWebContentsDelegate, public content::WebContentsObserver { public: + enum Type { + BROWSER_WINDOW, // Used by BrowserWindow. + WEB_VIEW, // Used by . + REMOTE, // Thin wrap around an existing WebContents. + }; + // For node.js callback function type: function(error, buffer) using PrintToPDFCallback = base::Callback, v8::Local)>; @@ -59,7 +65,7 @@ class WebContents : public mate::TrackableObject, v8::Local prototype); int GetID() const; - std::string GetType() const; + Type GetType() const; bool Equal(const WebContents* web_contents) const; void LoadURL(const GURL& url, const mate::Dictionary& options); void DownloadURL(const GURL& url); @@ -139,7 +145,6 @@ class WebContents : public mate::TrackableObject, // Methods for creating . void SetSize(const SetSizeParams& params); bool IsGuest() const; - bool IsRemote() const; // Callback triggered on permission response. void OnEnterFullscreenModeForTab(content::WebContents* source, @@ -269,12 +274,6 @@ class WebContents : public mate::TrackableObject, void DevToolsClosed() override; private: - enum Type { - BROWSER_WINDOW, // Used by BrowserWindow. - WEB_VIEW, // Used by . - REMOTE, // Thin wrap around an existing WebContents. - }; - AtomBrowserContext* GetBrowserContext() const; uint32_t GetNextRequestId() { From ee0bab63890e43720b37c415fb3014c04f37e090 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Jun 2016 09:23:03 -0700 Subject: [PATCH 04/10] Specify type instead of isGuest --- atom/browser/api/atom_api_web_contents.cc | 24 ++++++++++++++++------- lib/browser/guest-view-manager.js | 2 +- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 0591129e98..97fbbd1580 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -200,8 +200,20 @@ struct Converter { } return mate::ConvertToV8(isolate, type); } -}; + static bool FromV8(v8::Isolate* isolate, v8::Local val, + atom::api::WebContents::Type* out) { + std::string type; + if (!ConvertFromV8(isolate, val, &type)) + return false; + if (type == "webview") { + *out = atom::api::WebContents::Type::WEB_VIEW; + } else { + return false; + } + return true; + } +}; } // namespace mate @@ -254,10 +266,8 @@ WebContents::WebContents(v8::Isolate* isolate, // Read options. options.Get("backgroundThrottling", &background_throttling_); - // Whether it is a guest WebContents. - bool is_guest = false; - options.Get("isGuest", &is_guest); - type_ = is_guest ? WEB_VIEW : BROWSER_WINDOW; + type_ = BROWSER_WINDOW; + options.Get("type", &type_); // Obtain the session. std::string partition; @@ -277,7 +287,7 @@ WebContents::WebContents(v8::Isolate* isolate, session_.Reset(isolate, session.ToV8()); content::WebContents* web_contents; - if (is_guest) { + if (IsGuest()) { scoped_refptr site_instance = content::SiteInstance::CreateForURL( session->browser_context(), GURL("chrome-guest://fake-host")); @@ -306,7 +316,7 @@ WebContents::WebContents(v8::Isolate* isolate, web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent()); - if (is_guest) { + if (IsGuest()) { guest_delegate_->Initialize(this); NativeWindow* owner_window = nullptr; diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js index fdd426c229..fc51b3083e 100644 --- a/lib/browser/guest-view-manager.js +++ b/lib/browser/guest-view-manager.js @@ -63,7 +63,7 @@ const createGuest = function (embedder, params) { const id = getNextInstanceId(embedder) const guest = webContents.create({ - isGuest: true, + type: 'webview', partition: params.partition, embedder: embedder }) From f29801ad2b741318ded4e46f6e0ec4eee40063c6 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Jun 2016 09:27:36 -0700 Subject: [PATCH 05/10] Add background page web contents type --- atom/browser/api/atom_api_web_contents.cc | 20 ++++++++++++++++---- atom/browser/api/atom_api_web_contents.h | 3 ++- lib/browser/chrome-extension.js | 1 + 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 97fbbd1580..4f168c1cd0 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -193,10 +193,20 @@ struct Converter { atom::api::WebContents::Type val) { std::string type = ""; switch (val) { - case atom::api::WebContents::Type::BROWSER_WINDOW: type = "window"; break; - case atom::api::WebContents::Type::WEB_VIEW: type = "webview"; break; - case atom::api::WebContents::Type::REMOTE: type = "remote"; break; - default: break; + case atom::api::WebContents::Type::BACKGROUND_PAGE: + type = "backgroundPage"; + break; + case atom::api::WebContents::Type::BROWSER_WINDOW: + type = "window"; + break; + case atom::api::WebContents::Type::REMOTE: + type = "remote"; + break; + case atom::api::WebContents::Type::WEB_VIEW: + type = "webview"; + break; + default: + break; } return mate::ConvertToV8(isolate, type); } @@ -208,6 +218,8 @@ struct Converter { return false; if (type == "webview") { *out = atom::api::WebContents::Type::WEB_VIEW; + } else if (type == "backgroundPage") { + *out = atom::api::WebContents::Type::BACKGROUND_PAGE; } else { return false; } diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index 9bf9dd3ad7..938a541597 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -44,9 +44,10 @@ class WebContents : public mate::TrackableObject, public content::WebContentsObserver { public: enum Type { + BACKGROUND_PAGE, // A DevTools extension background page. BROWSER_WINDOW, // Used by BrowserWindow. - WEB_VIEW, // Used by . REMOTE, // Thin wrap around an existing WebContents. + WEB_VIEW, // Used by . }; // For node.js callback function type: function(error, buffer) diff --git a/lib/browser/chrome-extension.js b/lib/browser/chrome-extension.js index bff436409b..78d6516c3f 100644 --- a/lib/browser/chrome-extension.js +++ b/lib/browser/chrome-extension.js @@ -73,6 +73,7 @@ const startBackgroundPages = function (manifest) { const html = new Buffer(`${scripts}`) const contents = webContents.create({ + type: 'backgroundPage', commandLineSwitches: ['--background-page'] }) backgroundPages[manifest.extensionId] = { html: html, webContents: contents } From ee09c7534a37e01a8589ab82ac028aa4ba4d6421 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Jun 2016 09:30:06 -0700 Subject: [PATCH 06/10] Only add extensions to windows and webviews --- lib/browser/chrome-extension.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/browser/chrome-extension.js b/lib/browser/chrome-extension.js index 78d6516c3f..9a5e8e99fc 100644 --- a/lib/browser/chrome-extension.js +++ b/lib/browser/chrome-extension.js @@ -19,6 +19,11 @@ const generateExtensionIdFromName = function (name) { return name.replace(/[\W_]+/g, '-').toLowerCase() } +const isWindowOrWebView = function (webContents) { + const type = webContents.getType() + return type === 'window' || type === 'webview' +} + // Create or get manifest object from |srcDirectory|. const getManifestFromPath = function (srcDirectory) { let manifest @@ -238,7 +243,7 @@ const loadDevToolsExtensions = function (win, manifests) { } app.on('web-contents-created', function (event, webContents) { - if (webContents.getType() === 'remote') return + if (!isWindowOrWebView(webContents)) return hookWebContentsForTabEvents(webContents) webContents.on('devtools-opened', function () { @@ -323,7 +328,7 @@ app.once('ready', function () { const manifest = getManifestFromPath(srcDirectory) if (manifest) { for (const webContents of getAllWebContents()) { - if (webContents.getType() !== 'remote') { + if (isWindowOrWebView(webContents)) { loadDevToolsExtensions(webContents, [manifest]) } } From 85517a1eeae0b01210c9bcd927c1d2bdc537c80d Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Jun 2016 09:32:59 -0700 Subject: [PATCH 07/10] Add 2 spaces before comment --- atom/browser/api/atom_api_web_contents.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index 938a541597..6151110887 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -44,7 +44,7 @@ class WebContents : public mate::TrackableObject, public content::WebContentsObserver { public: enum Type { - BACKGROUND_PAGE, // A DevTools extension background page. + BACKGROUND_PAGE, // A DevTools extension background page. BROWSER_WINDOW, // Used by BrowserWindow. REMOTE, // Thin wrap around an existing WebContents. WEB_VIEW, // Used by . From bf791c110ff3d6ae9290c9437b536df212b75087 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Jun 2016 09:37:55 -0700 Subject: [PATCH 08/10] Tweak type check to consider windows and webviews --- atom/browser/api/atom_api_render_process_preferences.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_render_process_preferences.cc b/atom/browser/api/atom_api_render_process_preferences.cc index 4781f0603a..3d9495d932 100644 --- a/atom/browser/api/atom_api_render_process_preferences.cc +++ b/atom/browser/api/atom_api_render_process_preferences.cc @@ -26,7 +26,9 @@ bool IsWebContents(v8::Isolate* isolate, content::RenderProcessHost* process) { return false; auto api_web_contents = WebContents::CreateFrom(isolate, web_contents); - return api_web_contents->GetType() != WebContents::Type::REMOTE; + auto type = api_web_contents->GetType(); + return type == WebContents::Type::BROWSER_WINDOW || + type == WebContents::Type::WEB_VIEW; } } // namespace From 4e896565387f712fa663a0c036558b0fd4bdec92 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Jun 2016 10:05:25 -0700 Subject: [PATCH 09/10] :art: --- atom/browser/api/atom_api_web_contents.cc | 25 ++++++++--------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 4f168c1cd0..82c048d9a6 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -191,35 +191,28 @@ template<> struct Converter { static v8::Local ToV8(v8::Isolate* isolate, atom::api::WebContents::Type val) { + using Type = atom::api::WebContents::Type; std::string type = ""; switch (val) { - case atom::api::WebContents::Type::BACKGROUND_PAGE: - type = "backgroundPage"; - break; - case atom::api::WebContents::Type::BROWSER_WINDOW: - type = "window"; - break; - case atom::api::WebContents::Type::REMOTE: - type = "remote"; - break; - case atom::api::WebContents::Type::WEB_VIEW: - type = "webview"; - break; - default: - break; + case Type::BACKGROUND_PAGE: type = "backgroundPage"; break; + case Type::BROWSER_WINDOW: type = "window"; break; + case Type::REMOTE: type = "remote"; break; + case Type::WEB_VIEW: type = "webview"; break; + default: break; } return mate::ConvertToV8(isolate, type); } static bool FromV8(v8::Isolate* isolate, v8::Local val, atom::api::WebContents::Type* out) { + using Type = atom::api::WebContents::Type; std::string type; if (!ConvertFromV8(isolate, val, &type)) return false; if (type == "webview") { - *out = atom::api::WebContents::Type::WEB_VIEW; + *out = Type::WEB_VIEW; } else if (type == "backgroundPage") { - *out = atom::api::WebContents::Type::BACKGROUND_PAGE; + *out = Type::BACKGROUND_PAGE; } else { return false; } From ae6ffa6d5edbedd8b0645989d3b4f1a15c6e1c47 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Jun 2016 15:39:07 -0700 Subject: [PATCH 10/10] unkown -> unknown --- lib/browser/chrome-extension.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/browser/chrome-extension.js b/lib/browser/chrome-extension.js index 9a5e8e99fc..50fe2a5452 100644 --- a/lib/browser/chrome-extension.js +++ b/lib/browser/chrome-extension.js @@ -117,7 +117,7 @@ let nextId = 0 ipcMain.on('CHROME_RUNTIME_CONNECT', function (event, extensionId, connectInfo) { const page = backgroundPages[extensionId] if (!page) { - console.error(`Connect to unkown extension ${extensionId}`) + console.error(`Connect to unknown extension ${extensionId}`) return } @@ -138,7 +138,7 @@ ipcMain.on('CHROME_I18N_MANIFEST', function (event, extensionId) { ipcMain.on('CHROME_RUNTIME_SENDMESSAGE', function (event, extensionId, message) { const page = backgroundPages[extensionId] if (!page) { - console.error(`Connect to unkown extension ${extensionId}`) + console.error(`Connect to unknown extension ${extensionId}`) return } @@ -148,7 +148,7 @@ ipcMain.on('CHROME_RUNTIME_SENDMESSAGE', function (event, extensionId, message) ipcMain.on('CHROME_TABS_SEND_MESSAGE', function (event, tabId, extensionId, isBackgroundPage, message) { const contents = webContents.fromId(tabId) if (!contents) { - console.error(`Sending message to unkown tab ${tabId}`) + console.error(`Sending message to unknown tab ${tabId}`) return } @@ -160,7 +160,7 @@ ipcMain.on('CHROME_TABS_SEND_MESSAGE', function (event, tabId, extensionId, isBa ipcMain.on('CHROME_TABS_EXECUTESCRIPT', function (event, requestId, tabId, extensionId, details) { const contents = webContents.fromId(tabId) if (!contents) { - console.error(`Sending message to unkown tab ${tabId}`) + console.error(`Sending message to unknown tab ${tabId}`) return }