Add support for native chromium popups on sandboxed renderers.

- Allow `api::Window` instances to be created from existing `api::WebContents`.
- Override `WebContentsCreated` and `AddNewContents` to wrap renderer-created
  `content::WebContents` into `api::WebContents`.
- For `content::WebContents` that should be displayed in new windows, pass the
  wrapped `api::WebContents` object to window manager.

atom/browser/api/atom_api_web_contents.cc

@@ -423,6 +423,31 @@ void WebContents::OnCreateWindow(const GURL& target_url,
     Emit("new-window", target_url, frame_name, disposition);
 }
 
+void WebContents::WebContentsCreated(content::WebContents* source_contents,
+                                     int opener_render_frame_id,
+                                     const std::string& frame_name,
+                                     const GURL& target_url,
+                                     content::WebContents* new_contents) {
+  v8::Locker locker(isolate());
+  v8::HandleScope handle_scope(isolate());
+  auto api_web_contents = CreateFrom(isolate(), new_contents, BROWSER_WINDOW);
+  Emit("-web-contents-created", api_web_contents, target_url, frame_name);
+}
+
+void WebContents::AddNewContents(content::WebContents* source,
+                                 content::WebContents* new_contents,
+                                 WindowOpenDisposition disposition,
+                                 const gfx::Rect& initial_rect,
+                                 bool user_gesture,
+                                 bool* was_blocked) {
+  v8::Locker locker(isolate());
+  v8::HandleScope handle_scope(isolate());
+  auto api_web_contents = CreateFrom(isolate(), new_contents);
+  Emit("-add-new-contents", api_web_contents, disposition, user_gesture,
+      initial_rect.x(), initial_rect.y(), initial_rect.width(),
+      initial_rect.height());
+}
+
 content::WebContents* WebContents::OpenURLFromTab(
     content::WebContents* source,
     const content::OpenURLParams& params) {

atom/browser/api/atom_api_web_contents.h

@@ -210,6 +210,17 @@ class WebContents : public mate::TrackableObject<WebContents>,
                            const base::string16& message,
                            int32_t line_no,
                            const base::string16& source_id) override;
+  void WebContentsCreated(content::WebContents* source_contents,
+                          int opener_render_frame_id,
+                          const std::string& frame_name,
+                          const GURL& target_url,
+                          content::WebContents* new_contents) override;
+  void AddNewContents(content::WebContents* source,
+                      content::WebContents* new_contents,
+                      WindowOpenDisposition disposition,
+                      const gfx::Rect& initial_rect,
+                      bool user_gesture,
+                      bool* was_blocked) override;
   content::WebContents* OpenURLFromTab(
       content::WebContents* source,
       const content::OpenURLParams& params) override;

atom/browser/api/atom_api_window.cc

@@ -72,20 +72,25 @@ v8::Local<v8::Value> ToBuffer(v8::Isolate* isolate, void* val, int size) {
 
 Window::Window(v8::Isolate* isolate, v8::Local<v8::Object> wrapper,
                const mate::Dictionary& options) {
-  // Use options.webPreferences to create WebContents.
-  mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate);
-  options.Get(options::kWebPreferences, &web_preferences);
-
-  // Copy the backgroundColor to webContents.
-  v8::Local<v8::Value> value;
-  if (options.Get(options::kBackgroundColor, &value))
-    web_preferences.Set(options::kBackgroundColor, value);
-
-  v8::Local<v8::Value> transparent;
-  if (options.Get("transparent", &transparent))
-    web_preferences.Set("transparent", transparent);
-  // Creates the WebContents used by BrowserWindow.
-  auto web_contents = WebContents::Create(isolate, web_preferences);
+  mate::Handle<class WebContents> web_contents;
+  // If no WebContents was passed to the constructor, create it from options.
+  if (!options.Get("webContents", &web_contents)) {
+    // Use options.webPreferences to create WebContents.
+    mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate);
+    options.Get(options::kWebPreferences, &web_preferences);
+
+    // Copy the backgroundColor to webContents.
+    v8::Local<v8::Value> value;
+    if (options.Get(options::kBackgroundColor, &value))
+      web_preferences.Set(options::kBackgroundColor, value);
+
+    v8::Local<v8::Value> transparent;
+    if (options.Get("transparent", &transparent))
+      web_preferences.Set("transparent", transparent);
+
+    // Creates the WebContents used by BrowserWindow.
+    web_contents = WebContents::Create(isolate, web_preferences);
+  }
 
   Init(isolate, wrapper, options, web_contents);
 }

atom/browser/native_window.cc

@@ -391,6 +391,10 @@ void NativeWindow::RequestToClosePage() {
   if (window_unresposive_closure_.IsCancelled())
     ScheduleUnresponsiveEvent(5000);
 
+  if (!web_contents())
+    // Already closed by renderer
+    return;
+
   if (web_contents()->NeedToFireBeforeUnload())
     web_contents()->DispatchBeforeUnload();
   else

lib/browser/api/browser-window.js

@@ -3,6 +3,7 @@
 const {ipcMain} = require('electron')
 const {EventEmitter} = require('events')
 const {BrowserWindow} = process.atomBinding('window')
+const v8Util = process.atomBinding('v8_util')
 
 Object.setPrototypeOf(BrowserWindow.prototype, EventEmitter.prototype)
 
@@ -26,6 +27,34 @@ BrowserWindow.prototype._init = function () {
     ipcMain.emit('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', event, url, frameName, disposition, options)
   })
 
+  this.webContents.on('-web-contents-created', (event, webContents, url,
+                                                frameName) => {
+    v8Util.setHiddenValue(webContents, 'url-framename', {url, frameName})
+  })
+  // Create a new browser window for the native implementation of
+  // "window.open"(sandbox mode only)
+  this.webContents.on('-add-new-contents', (event, webContents, disposition,
+                                            userGesture, left, top, width,
+                                            height) => {
+    let urlFrameName = v8Util.getHiddenValue(webContents, 'url-framename')
+    if ((disposition !== 'foreground-tab' && disposition !== 'new-window') ||
+        !urlFrameName) {
+      return
+    }
+
+    let {url, frameName} = urlFrameName
+    v8Util.deleteHiddenValue(webContents, 'url-framename')
+    const options = {
+      show: true,
+      x: left,
+      y: top,
+      width: width || 800,
+      height: height || 600,
+      webContents: webContents
+    }
+    ipcMain.emit('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', event, url, frameName, disposition, options)
+  })
+
   // window.resizeTo(...)
   // window.moveTo(...)
   this.webContents.on('move', (event, size) => {

lib/browser/guest-window-manager.js

@@ -57,7 +57,30 @@ const createGuest = function (embedder, url, frameName, options) {
   }
   options.webPreferences.openerId = embedder.id
   guest = new BrowserWindow(options)
-  guest.loadURL(url)
+  if (!options.webContents || url !== 'about:blank') {
+    // We should not call `loadURL` if the window was constructed from an
+    // existing webContents(window.open in a sandboxed renderer) and if the url
+    // is not 'about:blank'.
+    //
+    // Navigating to the url when creating the window from an existing
+    // webContents would not be necessary(it will navigate there anyway), but
+    // apparently there's a bug that allows the child window to be scripted by
+    // the opener, even when the child window is from another origin.
+    //
+    // That's why the second condition(url !== "about:blank") is required: to
+    // force `OverrideSiteInstanceForNavigation` to be called and consequently
+    // spawn a new renderer if the new window is targeting a different origin.
+    //
+    // If the URL is "about:blank", then it is very likely that the opener just
+    // wants to synchronously script the popup, for example:
+    //
+    //     let popup = window.open()
+    //     popup.document.body.write('<h1>hello</h1>')
+    //
+    // The above code would not work if a navigation to "about:blank" is done
+    // here, since the window would be cleared of all changes in the next tick.
+    guest.loadURL(url)
+  }
 
   // When |embedder| is destroyed we should also destroy attached guest, and if
   // guest is closed by user then we should prevent |embedder| from double
@@ -72,8 +95,19 @@ const createGuest = function (embedder, url, frameName, options) {
     embedder.send('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSED_' + guestId)
     embedder.removeListener('render-view-deleted', closedByEmbedder)
   }
-  embedder.once('render-view-deleted', closedByEmbedder)
-  guest.once('closed', closedByUser)
+  if (!options.webPreferences.sandbox) {
+    // These events should only be handled when the guest window is opened by a
+    // non-sandboxed renderer for two reasons:
+    //
+    // - `render-view-deleted` is emitted when the popup is closed by the user,
+    //   and that will eventually result in NativeWindow::NotifyWindowClosed
+    //   using a dangling pointer since `destroy()` would have been called by
+    //   `closeByEmbedded`
+    // - No need to emit `ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSED_` since
+    //   there's no renderer code listening to it.,
+    embedder.once('render-view-deleted', closedByEmbedder)
+    guest.once('closed', closedByUser)
+  }
 
   if (frameName) {
     frameToGuest[frameName] = guest