Merge pull request #11501 from emmkimme/Enh_WebPrefs_Affinity_Option

Feature : Affinity option for gathering several pages in a single process

atom/browser/atom_browser_client.cc

@@ -167,11 +167,14 @@ void AtomBrowserClient::RenderProcessWillLaunch(
 
   content::WebContents* web_contents = GetWebContentsFromProcessID(process_id);
   ProcessPreferences process_prefs;
-  process_prefs.sandbox = WebContentsPreferences::IsSandboxed(web_contents);
+  process_prefs.sandbox =
-  process_prefs.native_window_open
+      WebContentsPreferences::IsPreferenceEnabled("sandbox", web_contents);
-      = WebContentsPreferences::UsesNativeWindowOpen(web_contents);
+  process_prefs.native_window_open =
-  process_prefs.disable_popups
+      WebContentsPreferences::IsPreferenceEnabled("nativeWindowOpen",
-      = WebContentsPreferences::DisablePopups(web_contents);
+                                                  web_contents);
+  process_prefs.disable_popups =
+      WebContentsPreferences::IsPreferenceEnabled("disablePopups",
+                                                  web_contents);
   AddProcessPreferences(host->GetID(), process_prefs);
   // ensure the ProcessPreferences is removed later
   host->AddObserver(this);
@@ -204,7 +207,7 @@ void AtomBrowserClient::OverrideWebkitPrefs(
 }
 
 void AtomBrowserClient::OverrideSiteInstanceForNavigation(
-    content::RenderFrameHost* render_frame_host,
+    content::RenderFrameHost* rfh,
     content::BrowserContext* browser_context,
     content::SiteInstance* current_instance,
     const GURL& url,
@@ -214,25 +217,53 @@ void AtomBrowserClient::OverrideSiteInstanceForNavigation(
     return;
   }
 
-  if (!ShouldCreateNewSiteInstance(render_frame_host, browser_context,
+  if (!ShouldCreateNewSiteInstance(rfh, browser_context, current_instance, url))
-                                   current_instance, url))
     return;
 
-  scoped_refptr<content::SiteInstance> site_instance =
+  bool is_new_instance = true;
-      content::SiteInstance::CreateForURL(browser_context, url);
+  scoped_refptr<content::SiteInstance> site_instance;
+
+  // Do we have an affinity site to manage ?
+  std::string affinity;
+  auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
+  auto* web_preferences = web_contents ?
+      WebContentsPreferences::FromWebContents(web_contents) : nullptr;
+  if (web_preferences &&
+      web_preferences->web_preferences()->GetString("affinity", &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.
+  if (is_new_instance) {
-  // FIXME(zcbenz): We should adjust OverrideSiteInstanceForNavigation's
+    // Make sure the |site_instance| is not freed
-  // interface to solve this.
+    // when this function returns.
-  content::BrowserThread::PostTask(
+    // FIXME(zcbenz): We should adjust
-      content::BrowserThread::UI, FROM_HERE,
+    // OverrideSiteInstanceForNavigation's interface to solve this.
-      base::Bind(&Noop, base::RetainedRef(site_instance)));
+    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.
+    // Remember the original web contents for the pending renderer process.
-  auto pending_process = (*new_instance)->GetProcess();
+    auto pending_process = site_instance->GetProcess();
-  pending_processes_[pending_process->GetID()] =
+    pending_processes_[pending_process->GetID()] = web_contents;
-      content::WebContents::FromRenderFrameHost(render_frame_host);
+  }
 }
 
 void AtomBrowserClient::AppendExtraCommandLineSwitches(
@@ -396,6 +427,19 @@ void AtomBrowserClient::GetAdditionalAllowedSchemesForFileSystem(
   additional_schemes->push_back(content::kChromeDevToolsScheme);
 }
 
+void AtomBrowserClient::SiteInstanceDeleting(
+    content::SiteInstance* site_instance) {
+  // We are storing weak_ptr, is it fundamental to maintain the map up-to-date
+  // when an instance is destroyed.
+  for (auto iter = site_per_affinities.begin();
+      iter != site_per_affinities.end(); ++iter) {
+    if (iter->second == site_instance) {
+      site_per_affinities.erase(iter);
+      break;
+    }
+  }
+}
+
 brightray::BrowserMainParts* AtomBrowserClient::OverrideCreateBrowserMainParts(
     const content::MainFunctionParams&) {
   v8::V8::Initialize();  // Init V8 before creating main parts.

atom/browser/atom_browser_client.h

@@ -98,6 +98,7 @@ class AtomBrowserClient : public brightray::BrowserClient,
       bool* no_javascript_access) override;
   void GetAdditionalAllowedSchemesForFileSystem(
       std::vector<std::string>* schemes) override;
+  void SiteInstanceDeleting(content::SiteInstance* site_instance) override;
 
   // brightray::BrowserClient:
   brightray::BrowserMainParts* OverrideCreateBrowserMainParts(
@@ -119,9 +120,9 @@ class AtomBrowserClient : public brightray::BrowserClient,
                                    content::SiteInstance* current_instance,
                                    const GURL& dest_url);
   struct ProcessPreferences {
-    bool sandbox;
+    bool sandbox = false;
-    bool native_window_open;
+    bool native_window_open = false;
-    bool disable_popups;
+    bool disable_popups = false;
   };
   void AddProcessPreferences(int process_id, ProcessPreferences prefs);
   void RemoveProcessPreferences(int process_id);
@@ -134,6 +135,10 @@ class AtomBrowserClient : public brightray::BrowserClient,
 
   std::map<int, ProcessPreferences> process_preferences_;
   std::map<int, base::ProcessId> render_process_host_pids_;
+
+  // list of site per affinity. weak_ptr to prevent instance locking
+  std::map<std::string, content::SiteInstance*> site_per_affinities;
+
   base::Lock process_preferences_lock_;
 
   std::unique_ptr<AtomResourceDispatcherHostDelegate>

atom/browser/atom_resource_dispatcher_host_delegate.cc

@@ -75,7 +75,7 @@ void OnPdfResourceIntercepted(
   if (!web_contents)
     return;
 
-  if (!WebContentsPreferences::IsPluginsEnabled(web_contents)) {
+  if (!WebContentsPreferences::IsPreferenceEnabled("plugins", web_contents)) {
     auto browser_context = web_contents->GetBrowserContext();
     auto download_manager =
         content::BrowserContext::GetDownloadManager(browser_context);

atom/browser/web_contents_preferences.cc

@@ -112,7 +112,8 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches(
   // If the `sandbox` option was passed to the BrowserWindow's webPreferences,
   // pass `--enable-sandbox` to the renderer so it won't have any node.js
   // integration.
-  if (IsSandboxed(web_contents)) {
+  bool sandbox = false;
+  if (web_preferences.GetBoolean("sandbox", &sandbox) && sandbox) {
     command_line->AppendSwitch(switches::kEnableSandbox);
   } else if (!command_line->HasSwitch(switches::kEnableSandbox)) {
     command_line->AppendSwitch(::switches::kNoSandbox);
@@ -237,25 +238,6 @@ bool WebContentsPreferences::IsPreferenceEnabled(
   return bool_value;
 }
 
-bool WebContentsPreferences::IsSandboxed(content::WebContents* web_contents) {
-  return IsPreferenceEnabled("sandbox", web_contents);
-}
-
-bool WebContentsPreferences::UsesNativeWindowOpen(
-    content::WebContents* web_contents) {
-  return IsPreferenceEnabled("nativeWindowOpen", web_contents);
-}
-
-bool WebContentsPreferences::IsPluginsEnabled(
-    content::WebContents* web_contents) {
-  return IsPreferenceEnabled("plugins", web_contents);
-}
-
-bool WebContentsPreferences::DisablePopups(
-    content::WebContents* web_contents) {
-  return IsPreferenceEnabled("disablePopups", web_contents);
-}
-
 // static
 void WebContentsPreferences::OverrideWebkitPrefs(
     content::WebContents* web_contents, content::WebPreferences* prefs) {

atom/browser/web_contents_preferences.h

@@ -39,10 +39,6 @@ class WebContentsPreferences
 
   static bool IsPreferenceEnabled(const std::string& attribute_name,
                                   content::WebContents* web_contents);
-  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);
 
   // Modify the WebPreferences according to |web_contents|'s preferences.
   static void OverrideWebkitPrefs(

docs/api/browser-window.md

@@ -280,6 +280,13 @@ 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) - When specified, web pages with the same
+      `affinity` will run in the same renderer process. Note that due to reusing
+      the renderer process, certain `webPreferences` options will also be shared
+      between the web pages even when you specified different values for them,
+      including but not limited to `preload`, `sandbox` and `nodeIntegration`.
+      So it is suggested to use exact same `webPreferences` for web pages with
+      the same `affinity`.
     * `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`.

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()
+      })
+    })
+  })
+})