Backport (2-0-x) - Set appropriate defaults for webview options (#12292)

* Persist defaults to webPreferences object to JS land can read the inferred values instead of just user defined values

* Test inherited default propogation

* Refactor to remove coupling from fetching values and defaults

* Test description type

* Fix up tests

atom/browser/api/atom_api_web_contents.cc

@@ -1806,6 +1806,14 @@ v8::Local<v8::Value> WebContents::GetWebPreferences(v8::Isolate* isolate) {
   return mate::ConvertToV8(isolate, *web_preferences->web_preferences());
 }
 
+v8::Local<v8::Value> WebContents::GetLastWebPreferences(v8::Isolate* isolate) {
+  WebContentsPreferences* web_preferences =
+      WebContentsPreferences::FromWebContents(web_contents());
+  if (!web_preferences)
+    return v8::Null(isolate);
+  return mate::ConvertToV8(isolate, *web_preferences->last_web_preferences());
+}
+
 v8::Local<v8::Value> WebContents::GetOwnerBrowserWindow() {
   if (owner_window())
     return Window::From(isolate(), owner_window());
@@ -1957,6 +1965,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("_getZoomFactor", &WebContents::GetZoomFactor)
       .SetMethod("getType", &WebContents::GetType)
       .SetMethod("getWebPreferences", &WebContents::GetWebPreferences)
+      .SetMethod("getLastWebPreferences", &WebContents::GetLastWebPreferences)
       .SetMethod("getOwnerBrowserWindow", &WebContents::GetOwnerBrowserWindow)
       .SetMethod("hasServiceWorker", &WebContents::HasServiceWorker)
       .SetMethod("unregisterServiceWorker",

atom/browser/api/atom_api_web_contents.h

@@ -214,6 +214,7 @@ class WebContents : public mate::TrackableObject<WebContents>,
 
   // Returns the web preferences of current WebContents.
   v8::Local<v8::Value> GetWebPreferences(v8::Isolate* isolate);
+  v8::Local<v8::Value> GetLastWebPreferences(v8::Isolate* isolate);
 
   // Returns the owner window.
   v8::Local<v8::Value> GetOwnerBrowserWindow();

atom/browser/web_contents_preferences.cc

@@ -48,6 +48,28 @@ WebContentsPreferences::WebContentsPreferences(
   web_contents->SetUserData(UserDataKey(), base::WrapUnique(this));
 
   instances_.push_back(this);
+
+  // Set WebPreferences defaults onto the JS object
+  SetDefaultBoolIfUndefined("plugins", false);
+  SetDefaultBoolIfUndefined(options::kExperimentalFeatures, false);
+  SetDefaultBoolIfUndefined(options::kExperimentalCanvasFeatures, false);
+  bool node = SetDefaultBoolIfUndefined(options::kNodeIntegration, true);
+  SetDefaultBoolIfUndefined(options::kNodeIntegrationInWorker, false);
+  SetDefaultBoolIfUndefined(options::kWebviewTag, node);
+  SetDefaultBoolIfUndefined("sandbox", false);
+  SetDefaultBoolIfUndefined("nativeWindowOpen", false);
+  SetDefaultBoolIfUndefined(options::kContextIsolation, false);
+  SetDefaultBoolIfUndefined("javascript", true);
+  SetDefaultBoolIfUndefined("images", true);
+  SetDefaultBoolIfUndefined("textAreasAreResizable", true);
+  SetDefaultBoolIfUndefined("webgl", true);
+  SetDefaultBoolIfUndefined("webSecurity", true);
+  SetDefaultBoolIfUndefined("allowRunningInsecureContent", false);
+  #if defined(OS_MACOSX)
+  SetDefaultBoolIfUndefined(options::kScrollBounce, false);
+  #endif
+  SetDefaultBoolIfUndefined("offscreen", false);
+  last_web_preferences_.MergeDictionary(&web_preferences_);
 }
 
 WebContentsPreferences::~WebContentsPreferences() {
@@ -56,6 +78,16 @@ WebContentsPreferences::~WebContentsPreferences() {
       instances_.end());
 }
 
+bool WebContentsPreferences::SetDefaultBoolIfUndefined(const std::string key,
+                                                       bool val) {
+  bool existing;
+  if (!web_preferences_.GetBoolean(key, &existing)) {
+    web_preferences_.SetBoolean(key, val);
+    return val;
+  }
+  return existing;
+}
+
 void WebContentsPreferences::Merge(const base::DictionaryValue& extend) {
   web_preferences_.MergeDictionary(&extend);
 }
@@ -80,6 +112,12 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches(
 
   base::DictionaryValue& web_preferences = self->web_preferences_;
 
+  // We are appending args to a webContents so let's save the current state
+  // of our preferences object so that during the lifetime of the WebContents
+  // we can fetch the options used to initally configure the WebContents
+  self->last_web_preferences_.Clear();
+  self->last_web_preferences_.MergeDictionary(&web_preferences);
+
   bool b;
   // Check if plugins are enabled.
   if (web_preferences.GetBoolean("plugins", &b) && b)

atom/browser/web_contents_preferences.h

@@ -53,10 +53,16 @@ class WebContentsPreferences
 
   // Returns the web preferences.
   base::DictionaryValue* web_preferences() { return &web_preferences_; }
+  base::DictionaryValue* last_web_preferences() {
+    return &last_web_preferences_;
+  }
 
  private:
   friend class content::WebContentsUserData<WebContentsPreferences>;
 
+  // Set preference value to given bool if user did not provide value
+  bool SetDefaultBoolIfUndefined(const std::string key, bool val);
+
   // Get preferences value as integer possibly coercing it from a string
   bool GetInteger(const std::string& attributeName, int* intValue);
 
@@ -64,6 +70,7 @@ class WebContentsPreferences
 
   content::WebContents* web_contents_;
   base::DictionaryValue web_preferences_;
+  base::DictionaryValue last_web_preferences_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsPreferences);
 };

lib/browser/guest-view-manager.js

@@ -156,7 +156,7 @@ const createGuest = function (embedder, params) {
   // Forward internal web contents event to embedder to handle
   // native window.open setup
   guest.on('-add-new-contents', (...args) => {
-    if (guest.getWebPreferences().nativeWindowOpen === true) {
+    if (guest.getLastWebPreferences().nativeWindowOpen === true) {
       const embedder = getEmbedder(guestInstanceId)
       if (embedder != null) {
         embedder.emit('-add-new-contents', ...args)
@@ -164,7 +164,7 @@ const createGuest = function (embedder, params) {
     }
   })
   guest.on('-web-contents-created', (...args) => {
-    if (guest.getWebPreferences().nativeWindowOpen === true) {
+    if (guest.getLastWebPreferences().nativeWindowOpen === true) {
       const embedder = getEmbedder(guestInstanceId)
       if (embedder != null) {
         embedder.emit('-web-contents-created', ...args)

lib/browser/guest-window-manager.js

@@ -59,12 +59,12 @@ const mergeBrowserWindowOptions = function (embedder, options) {
     mergeOptions(options, parentOptions)
   } else {
     // Or only inherit webPreferences if it is a webview.
-    mergeOptions(options.webPreferences, embedder.getWebPreferences())
+    mergeOptions(options.webPreferences, embedder.getLastWebPreferences())
   }
 
   // Inherit certain option values from parent window
   for (const [name, value] of inheritedWebPreferences) {
-    if (embedder.getWebPreferences()[name] === value) {
+    if (embedder.getLastWebPreferences()[name] === value) {
       options.webPreferences[name] = value
     }
   }
@@ -177,8 +177,8 @@ const getGuestWindow = function (guestContents) {
 // The W3C does not have anything on this, but from my understanding of the
 // security model of |window.opener|, this should be fine.
 const canAccessWindow = function (sender, target) {
-  return (target.getWebPreferences().openerId === sender.id) ||
-         (sender.getWebPreferences().nodeIntegration === true) ||
+  return (target.getLastWebPreferences().openerId === sender.id) ||
+         (sender.getLastWebPreferences().nodeIntegration === true) ||
          isSameOrigin(sender.getURL(), target.getURL())
 }
 

lib/renderer/security-warnings.js

@@ -84,7 +84,7 @@ const getWebPreferences = function () {
     }
 
     const { remote } = require('electron')
-    webPreferences = remote.getCurrentWebContents().getWebPreferences()
+    webPreferences = remote.getCurrentWebContents().getLastWebPreferences()
     return webPreferences
   } catch (error) {
     return null

spec/api-browser-window-spec.js

@@ -2965,7 +2965,7 @@ describe('BrowserWindow module', () => {
     })
     it('enables context isolation on child windows', (done) => {
       app.once('browser-window-created', (event, window) => {
-        assert.equal(window.webContents.getWebPreferences().contextIsolation, true)
+        assert.equal(window.webContents.getLastWebPreferences().contextIsolation, true)
         done()
       })
       w.loadURL(`file://${fixtures}/pages/window-open.html`)

spec/chromium-spec.js

@@ -266,6 +266,26 @@ describe('chromium feature', () => {
       b = window.open(windowUrl, '', 'nodeIntegration=no,show=no')
     })
 
+    it('disables webviewTag when node integration is disabled on the parent window', (done) => {
+      let b
+      listener = (event) => {
+        assert.equal(event.data.isWebViewUndefined, true)
+        b.close()
+        done()
+      }
+      window.addEventListener('message', listener)
+
+      const windowUrl = require('url').format({
+        pathname: `${fixtures}/pages/window-opener-no-web-view-tag.html`,
+        protocol: 'file',
+        query: {
+          p: `${fixtures}/pages/window-opener-web-view.html`
+        },
+        slashes: true
+      })
+      b = window.open(windowUrl, '', 'nodeIntegration=no,show=no')
+    })
+
     it('disables node integration when it is disabled on the parent window for chrome devtools URLs', (done) => {
       let b
       app.once('web-contents-created', (event, contents) => {
@@ -285,7 +305,7 @@ describe('chromium feature', () => {
       app.once('web-contents-created', (event, contents) => {
         contents.once('did-finish-load', () => {
           app.once('browser-window-created', (event, window) => {
-            const preferences = window.webContents.getWebPreferences()
+            const preferences = window.webContents.getLastWebPreferences()
             assert.equal(preferences.javascript, false)
             window.destroy()
             b.close()
@@ -507,7 +527,7 @@ describe('chromium feature', () => {
         done()
       }
       window.addEventListener('message', listener)
-      w = window.open(url, '', 'show=no')
+      w = window.open(url, '', 'show=no,nodeIntegration=no')
     })
 
     it('works when origin matches', (done) => {
@@ -516,7 +536,7 @@ describe('chromium feature', () => {
         done()
       }
       window.addEventListener('message', listener)
-      w = window.open(`file://${fixtures}/pages/window-opener-location.html`, '', 'show=no')
+      w = window.open(`file://${fixtures}/pages/window-opener-location.html`, '', 'show=no,nodeIntegration=no')
     })
 
     it('works when origin does not match opener but has node integration', (done) => {

spec/fixtures/pages/window-opener-no-web-view-tag.html

@@ -0,0 +1,15 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  var windowUrl = decodeURIComponent(window.location.search.substring(3))
+  var opened = window.open('file://' + windowUrl, '', 'webviewTag=yes,show=no')
+  window.addEventListener('message', function (event) {
+    try {
+      opened.close()
+    } finally {
+      window.opener.postMessage(event.data, '*')
+    }
+  })
+</script>
+</body>
+</html>

spec/fixtures/pages/window-opener-web-view.html

@@ -0,0 +1,9 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  window.onload = () => {
+    window.opener.postMessage({isWebViewUndefined: typeof WebView === 'undefined'}, '*')
+  }
+</script>
+</body>
+</html>