chore: add deprecation warning for the default of contextIsolation (electron#23507)

MarshallOfSound · nornagon · web-flow · commit 16a3f41fd39c · 2020-06-25T10:55:17.000-07:00
* chore: add deprecation warning for the default of contextIsolation

* chore: add to breaking changes

* Update docs/breaking-changes.md

Co-authored-by: Jeremy Apthorp &lt;jeremya@chromium.org&gt;

* chore: fix specs on windows

Co-authored-by: Jeremy Apthorp &lt;jeremya@chromium.org&gt;
diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md
@@ -14,6 +14,15 @@ This document uses the following convention to categorize breaking changes:
 
 ## Planned Breaking API Changes (12.0)
 
+### Default Changed: `contextIsolation` defaults to `true`
+
+In Electron 12, `contextIsolation` will be enabled by default.  To restore
+the previous behavior, `contextIsolation: false` must be specified in WebPreferences.
+
+We [recommend having contextIsolation enabled](https://github.com/electron/electron/blob/master/docs/tutorial/security.md#3-enable-context-isolation-for-remote-content) for the security of your application.
+
+For more details see: https://github.com/electron/electron/issues/23506
+
 ### Removed: `crashReporter` methods in the renderer process
 
 The following `crashReporter` methods are no longer available in the renderer
diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc
@@ -26,6 +26,7 @@
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/options_switches.h"
+#include "shell/common/process_util.h"
 #include "third_party/blink/public/mojom/v8_cache_options.mojom.h"
 
 #if defined(OS_WIN)
@@ -126,6 +127,15 @@ WebContentsPreferences::WebContentsPreferences(
   SetDefaultBoolIfUndefined(options::kWebviewTag, false);
   SetDefaultBoolIfUndefined(options::kSandbox, false);
   SetDefaultBoolIfUndefined(options::kNativeWindowOpen, false);
+  if (IsUndefined(options::kContextIsolation)) {
+    node::Environment* env = node::Environment::GetCurrent(isolate);
+    EmitWarning(env,
+                "The default of contextIsolation is deprecated and will be "
+                "changing from false to true in a future release of Electron.  "
+                "See https://github.com/electron/electron/issues/23506 for "
+                "more information",
+                "electron");
+  }
   SetDefaultBoolIfUndefined(options::kContextIsolation, false);
   SetDefaultBoolIfUndefined(options::kJavaScript, true);
   SetDefaultBoolIfUndefined(options::kImages, true);
@@ -183,6 +193,10 @@ void WebContentsPreferences::SetDefaults() {
   last_preference_ = preference_.Clone();
 }
 
+bool WebContentsPreferences::IsUndefined(base::StringPiece key) {
+  return !preference_.FindKeyOfType(key, base::Value::Type::BOOLEAN);
+}
+
 bool WebContentsPreferences::SetDefaultBoolIfUndefined(base::StringPiece key,
                                                        bool val) {
   auto* current_value =
diff --git a/shell/browser/web_contents_preferences.h b/shell/browser/web_contents_preferences.h
@@ -72,6 +72,9 @@ class WebContentsPreferences
   // Get WebContents according to process ID.
   static content::WebContents* GetWebContentsFromProcessID(int process_id);
 
+  // Checks if the key is not defined
+  bool IsUndefined(base::StringPiece key);
+
   // Set preference value to given bool if user did not provide value
   bool SetDefaultBoolIfUndefined(base::StringPiece key, bool val);
 
diff --git a/spec/fixtures/api/gpu-info.js b/spec/fixtures/api/gpu-info.js
@@ -4,7 +4,7 @@ app.commandLine.appendSwitch('--disable-software-rasterizer');
 
 app.whenReady().then(() => {
   const infoType = process.argv.pop();
-  const w = new BrowserWindow({ show: false });
+  const w = new BrowserWindow({ show: false, webPreferences: { contextIsolation: true } });
   w.webContents.once('did-finish-load', () => {
     app.getGPUInfo(infoType).then(
       (gpuInfo) => {
diff --git a/spec/fixtures/api/site-instance-overrides/main.js b/spec/fixtures/api/site-instance-overrides/main.js
@@ -28,7 +28,8 @@ app.whenReady().then(() => {
   win = new BrowserWindow({
     show: false,
     webPreferences: {
-      preload: path.resolve(__dirname, 'preload.js')
+      preload: path.resolve(__dirname, 'preload.js'),
+      contextIsolation: true
     }
   });
   win.loadFile('index.html');
diff --git a/spec/fixtures/api/window-all-closed/main.js b/spec/fixtures/api/window-all-closed/main.js
@@ -15,6 +15,10 @@ app.on('quit', () => {
 });
 
 app.whenReady().then(() => {
-  const win = new BrowserWindow();
+  const win = new BrowserWindow({
+    webPreferences: {
+      contextIsolation: true
+    }
+  });
   win.close();
 });
