From 9c1b47361f58946b9e8a6ae2edfcdc0b3a09a9b8 Mon Sep 17 00:00:00 2001 From: Emmanuel Kimmerlin Date: Wed, 13 Dec 2017 11:57:41 +0100 Subject: [PATCH] Add an "affinity" option to webPreferences --- atom/browser/atom_browser_client.cc | 64 ++++++++-- atom/browser/atom_browser_client.h | 6 + atom/browser/web_contents_preferences.cc | 22 ++++ atom/browser/web_contents_preferences.h | 5 + docs/api/browser-window.md | 4 + spec/api-browser-window-affinity-spec.js | 153 +++++++++++++++++++++++ 6 files changed, 242 insertions(+), 12 deletions(-) create mode 100644 spec/api-browser-window-affinity-spec.js diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 6bd2e9f1fff3..e2add8564e9c 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -218,21 +218,61 @@ void AtomBrowserClient::OverrideSiteInstanceForNavigation( current_instance, url)) return; - scoped_refptr site_instance = - content::SiteInstance::CreateForURL(browser_context, url); + bool is_new_instance = true; + scoped_refptr site_instance; + + // Do we have an affinity site to manage ? + std::string affinity; + auto web_contents = + content::WebContents::FromRenderFrameHost(render_frame_host); + if (WebContentsPreferences::GetAffinity(web_contents, &affinity) + && !affinity.empty()) { + affinity = base::ToLowerASCII(affinity); + auto iter = site_per_affinities.find(affinity); + if (iter != site_per_affinities.end()) { + site_instance = iter->second; + is_new_instance = false; + } else { + // We must not provide the url. + // This site is "isolated" and must not be taken into account + // when Chromium looking at a candidate for an url. + site_instance = content::SiteInstance::Create( + browser_context); + site_per_affinities[affinity] = site_instance.get(); + } + } else { + site_instance = content::SiteInstance::CreateForURL( + browser_context, + url); + } *new_instance = site_instance.get(); - // Make sure the |site_instance| is not freed when this function returns. - // FIXME(zcbenz): We should adjust OverrideSiteInstanceForNavigation's - // interface to solve this. - content::BrowserThread::PostTask( - content::BrowserThread::UI, FROM_HERE, - base::Bind(&Noop, base::RetainedRef(site_instance))); + if (is_new_instance) { + // Make sure the |site_instance| is not freed + // when this function returns. + // FIXME(zcbenz): We should adjust + // OverrideSiteInstanceForNavigation's interface to solve this. + content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind(&Noop, base::RetainedRef(site_instance))); - // Remember the original web contents for the pending renderer process. - auto pending_process = (*new_instance)->GetProcess(); - pending_processes_[pending_process->GetID()] = - content::WebContents::FromRenderFrameHost(render_frame_host); + // Remember the original web contents for the pending renderer process. + auto pending_process = site_instance->GetProcess(); + pending_processes_[pending_process->GetID()] = web_contents; + } +} + +// We are storing weak_ptr, is it fundamental to maintain the map up-to-date +// when an instance is destroyed. +void AtomBrowserClient::SiteInstanceDeleting( + content::SiteInstance* site_instance) { + for (auto iter = site_per_affinities.begin(); + iter != site_per_affinities.end(); ++iter) { + if (iter->second == site_instance) { + site_per_affinities.erase(iter); + break; + } + } } void AtomBrowserClient::AppendExtraCommandLineSwitches( diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h index 59053dce2e24..1e35d5c426fa 100644 --- a/atom/browser/atom_browser_client.h +++ b/atom/browser/atom_browser_client.h @@ -113,6 +113,8 @@ class AtomBrowserClient : public brightray::BrowserClient, base::TerminationStatus status, int exit_code) override; + void SiteInstanceDeleting(content::SiteInstance* site_instance) override; + private: bool ShouldCreateNewSiteInstance(content::RenderFrameHost* render_frame_host, content::BrowserContext* browser_context, @@ -134,6 +136,10 @@ class AtomBrowserClient : public brightray::BrowserClient, std::map process_preferences_; std::map render_process_host_pids_; + + // list of site per affinity. weak_ptr to prevent instance locking + std::map site_per_affinities; + base::Lock process_preferences_lock_; std::unique_ptr diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 5e9a4f926e4c..85ccb47a5960 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -237,6 +237,22 @@ bool WebContentsPreferences::IsPreferenceEnabled( return bool_value; } +bool WebContentsPreferences::GetPreferenceString( + const std::string& attribute_name, + content::WebContents* web_contents, + std::string* strValue) { + WebContentsPreferences* self; + if (!web_contents) + return false; + + self = FromWebContents(web_contents); + if (!self) + return false; + + base::DictionaryValue& web_preferences = self->web_preferences_; + return web_preferences.GetString(attribute_name, strValue); +} + bool WebContentsPreferences::IsSandboxed(content::WebContents* web_contents) { return IsPreferenceEnabled("sandbox", web_contents); } @@ -256,6 +272,12 @@ bool WebContentsPreferences::DisablePopups( return IsPreferenceEnabled("disablePopups", web_contents); } +bool WebContentsPreferences::GetAffinity( + content::WebContents* web_contents, + std::string* string_value) { + return GetPreferenceString("affinity", web_contents, string_value); +} + // static void WebContentsPreferences::OverrideWebkitPrefs( content::WebContents* web_contents, content::WebPreferences* prefs) { diff --git a/atom/browser/web_contents_preferences.h b/atom/browser/web_contents_preferences.h index 00f582b00617..650e5bbdc587 100644 --- a/atom/browser/web_contents_preferences.h +++ b/atom/browser/web_contents_preferences.h @@ -39,10 +39,15 @@ class WebContentsPreferences static bool IsPreferenceEnabled(const std::string& attribute_name, content::WebContents* web_contents); + static bool GetPreferenceString(const std::string& attribute_name, + content::WebContents* web_contents, + std::string* strValue); static bool IsSandboxed(content::WebContents* web_contents); static bool UsesNativeWindowOpen(content::WebContents* web_contents); static bool DisablePopups(content::WebContents* web_contents); static bool IsPluginsEnabled(content::WebContents* web_contents); + static bool GetAffinity(content::WebContents* web_contents, + std::string* string_value); // Modify the WebPreferences according to |web_contents|'s preferences. static void OverrideWebkitPrefs( diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 9434e2ee66b8..27adb6a892fc 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -280,6 +280,10 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. same `partition`. If there is no `persist:` prefix, the page will use an in-memory session. By assigning the same `partition`, multiple pages can share the same session. Default is the default session. + * `affinity` String (optional) - Sets the expected process hosting the page. Allow to gather + several pages in the same process. There are known limitations: + you can not host in the same site, pages with different preload file, + nodeIntegration or sandbox preferences. * `zoomFactor` Number (optional) - The default zoom factor of the page, `3.0` represents `300%`. Default is `1.0`. * `javascript` Boolean (optional) - Enables JavaScript support. Default is `true`. diff --git a/spec/api-browser-window-affinity-spec.js b/spec/api-browser-window-affinity-spec.js new file mode 100644 index 000000000000..f4b70cb6e136 --- /dev/null +++ b/spec/api-browser-window-affinity-spec.js @@ -0,0 +1,153 @@ +'use strict' + +const assert = require('assert') +const path = require('path') + +const { remote } = require('electron') +const { ipcMain, BrowserWindow } = remote +const {closeWindow} = require('./window-helpers') + +describe('BrowserWindow with affinity module', () => { + const fixtures = path.resolve(__dirname, 'fixtures') + const myAffinityName = 'myAffinity' + const myAffinityNameUpper = 'MYAFFINITY' + const anotherAffinityName = 'anotherAffinity' + + function createWindowWithWebPrefs (webPrefs) { + return new Promise((resolve, reject) => { + const w = new BrowserWindow({ + show: false, + width: 400, + height: 400, + webPreferences: webPrefs || {} + }) + w.webContents.on('did-finish-load', () => { + resolve(w) + }) + w.loadURL('file://' + path.join(fixtures, 'api', 'blank.html')) + }) + } + + describe(`BrowserWindow with an affinity '${myAffinityName}'`, () => { + let mAffinityWindow + before((done) => { + createWindowWithWebPrefs({ affinity: myAffinityName }) + .then((w) => { + mAffinityWindow = w + done() + }) + }) + + after((done) => { + closeWindow(mAffinityWindow, {assertSingleWindow: false}).then(() => { + mAffinityWindow = null + done() + }) + }) + + it('should have a different process id than a default window', (done) => { + createWindowWithWebPrefs({}) + .then((w) => { + assert.notEqual(mAffinityWindow.webContents.getOSProcessId(), w.webContents.getOSProcessId(), 'Should have the different OS process Id/s') + closeWindow(w, {assertSingleWindow: false}).then(() => { + done() + }) + }) + }) + + it(`should have a different process id than a window with a different affinity '${anotherAffinityName}'`, (done) => { + createWindowWithWebPrefs({ affinity: anotherAffinityName }) + .then((w) => { + assert.notEqual(mAffinityWindow.webContents.getOSProcessId(), w.webContents.getOSProcessId(), 'Should have the different OS process Id/s') + closeWindow(w, {assertSingleWindow: false}).then(() => { + done() + }) + }) + }) + + it(`should have the same OS process id than a window with the same affinity '${myAffinityName}'`, (done) => { + createWindowWithWebPrefs({ affinity: myAffinityName }) + .then((w) => { + assert.equal(mAffinityWindow.webContents.getOSProcessId(), w.webContents.getOSProcessId(), 'Should have the same OS process Id') + closeWindow(w, {assertSingleWindow: false}).then(() => { + done() + }) + }) + }) + + it(`should have the same OS process id than a window with an equivalent affinity '${myAffinityNameUpper}' (case insensitive)`, (done) => { + createWindowWithWebPrefs({ affinity: myAffinityNameUpper }) + .then((w) => { + assert.equal(mAffinityWindow.webContents.getOSProcessId(), w.webContents.getOSProcessId(), 'Should have the same OS process Id') + closeWindow(w, {assertSingleWindow: false}).then(() => { + done() + }) + }) + }) + }) + + describe(`BrowserWindow with an affinity : nodeIntegration=false`, () => { + const preload = path.join(fixtures, 'module', 'send-later.js') + const affinityWithNodeTrue = 'affinityWithNodeTrue' + const affinityWithNodeFalse = 'affinityWithNodeFalse' + + function testNodeIntegration (present) { + return new Promise((resolve, reject) => { + ipcMain.once('answer', (event, typeofProcess, typeofBuffer) => { + if (present) { + assert.notEqual(typeofProcess, 'undefined') + assert.notEqual(typeofBuffer, 'undefined') + } else { + assert.equal(typeofProcess, 'undefined') + assert.equal(typeofBuffer, 'undefined') + } + resolve() + }) + }) + } + + it('disables node integration when specified to false', (done) => { + Promise.all([testNodeIntegration(false), createWindowWithWebPrefs({ affinity: affinityWithNodeTrue, preload: preload, nodeIntegration: false })]) + .then((args) => { + closeWindow(args[1], {assertSingleWindow: false}).then(() => { + done() + }) + }) + }) + it('disables node integration when first window is false', (done) => { + Promise.all([testNodeIntegration(false), createWindowWithWebPrefs({ affinity: affinityWithNodeTrue, preload: preload, nodeIntegration: false })]) + .then((args) => { + let w1 = args[1] + return Promise.all([testNodeIntegration(false), w1, createWindowWithWebPrefs({ affinity: affinityWithNodeTrue, preload: preload, nodeIntegration: true })]) + }) + .then((ws) => { + return Promise.all([closeWindow(ws[1], {assertSingleWindow: false}), closeWindow(ws[2], {assertSingleWindow: false})]) + }) + .then(() => { + done() + }) + }) + + it('enables node integration when specified to true', (done) => { + Promise.all([testNodeIntegration(true), createWindowWithWebPrefs({ affinity: affinityWithNodeFalse, preload: preload, nodeIntegration: true })]) + .then((args) => { + closeWindow(args[1], {assertSingleWindow: false}).then(() => { + done() + }) + }) + }) + it('enables node integration when first window is true', (done) => { + Promise.all([testNodeIntegration(true), createWindowWithWebPrefs({ affinity: affinityWithNodeFalse, preload: preload, nodeIntegration: true })]) + .then((args) => { + let w1 = args[1] + return Promise.all([testNodeIntegration(true), w1, createWindowWithWebPrefs({ affinity: affinityWithNodeFalse, preload: preload, nodeIntegration: false })]) + }) + .then((ws) => { + return Promise.all([closeWindow(ws[1], {assertSingleWindow: false}), closeWindow(ws[2], {assertSingleWindow: false})]) + }) + .then(() => { + done() + }) + }) + }) +})