Merge remote-tracking branch 'origin/master' into chrome58

atom/browser/api/atom_api_browser_view.cc

@@ -75,7 +75,7 @@ void BrowserView::Init(v8::Isolate* isolate,
 }
 
 BrowserView::~BrowserView() {
-  api_web_contents_->DestroyWebContents();
+  api_web_contents_->DestroyWebContents(true /* async */);
 }
 
 // static

atom/browser/api/atom_api_debugger.cc

@@ -9,7 +9,6 @@
 #include "atom/browser/atom_browser_main_parts.h"
 #include "atom/common/native_mate_converters/callback.h"
 #include "atom/common/native_mate_converters/value_converter.h"
-#include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/web_contents.h"
@@ -45,12 +44,23 @@ void Debugger::DispatchProtocolMessage(DevToolsAgentHost* agent_host,
                                        const std::string& message) {
   DCHECK(agent_host == agent_host_.get());
 
-  std::unique_ptr<base::Value> parsed_message(base::JSONReader::Read(message));
-  if (!parsed_message->IsType(base::Value::Type::DICTIONARY))
-    return;
+  v8::Locker locker(isolate());
+  v8::HandleScope handle_scope(isolate());
+
+  v8::Local<v8::String> local_message =
+      v8::String::NewFromUtf8(isolate(), message.data());
+  v8::MaybeLocal<v8::Value> parsed_message = v8::JSON::Parse(
+      isolate()->GetCurrentContext(), local_message);
+  if (parsed_message.IsEmpty()) {
+    return;
+  }
+
+  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+  if (!mate::ConvertFromV8(isolate(), parsed_message.ToLocalChecked(),
+                           dict.get())) {
+    return;
+  }
 
-  base::DictionaryValue* dict =
-      static_cast<base::DictionaryValue*>(parsed_message.get());
   int id;
   if (!dict->GetInteger("id", &id)) {
     std::string method;

atom/browser/api/atom_api_web_contents.cc

@@ -418,15 +418,28 @@ WebContents::~WebContents() {
       guest_delegate_->Destroy();
 
     RenderViewDeleted(web_contents()->GetRenderViewHost());
-    DestroyWebContents();
+
+    if (type_ == WEB_VIEW) {
+      DestroyWebContents(false /* async */);
+    } else {
+      if (type_ == BROWSER_WINDOW && owner_window()) {
+        owner_window()->CloseContents(nullptr);
+      } else {
+        DestroyWebContents(true /* async */);
+      }
+      // The WebContentsDestroyed will not be called automatically because we
+      // destroy the webContents in the next tick. So we have to manually
+      // call it here to make sure "destroyed" event is emitted.
+      WebContentsDestroyed();
+    }
   }
 }
 
-void WebContents::DestroyWebContents() {
+void WebContents::DestroyWebContents(bool async) {
   // This event is only for internal use, which is emitted when WebContents is
   // being destroyed.
   Emit("will-destroy");
-  ResetManagedWebContents();
+  ResetManagedWebContents(async);
 }
 
 bool WebContents::DidAddMessageToConsole(content::WebContents* source,
@@ -478,7 +491,7 @@ void WebContents::AddNewContents(content::WebContents* source,
   if (Emit("-add-new-contents", api_web_contents, disposition, user_gesture,
       initial_rect.x(), initial_rect.y(), initial_rect.width(),
       initial_rect.height())) {
-    api_web_contents->DestroyWebContents();
+    api_web_contents->DestroyWebContents(true /* async */);
   }
 }
 
@@ -817,10 +830,8 @@ void WebContents::DidFinishNavigation(
 
 void WebContents::TitleWasSet(content::NavigationEntry* entry,
                               bool explicit_set) {
-  if (entry)
-    Emit("-page-title-updated", entry->GetTitle(), explicit_set);
-  else
-    Emit("-page-title-updated", "", explicit_set);
+  auto title = entry ? entry->GetTitle() : base::string16();
+  Emit("page-title-updated", title, explicit_set);
 }
 
 void WebContents::DidUpdateFaviconURL(

atom/browser/api/atom_api_web_contents.h

@@ -78,7 +78,7 @@ class WebContents : public mate::TrackableObject<WebContents>,
                              v8::Local<v8::FunctionTemplate> prototype);
 
   // Notifies to destroy any guest web contents before destroying self.
-  void DestroyWebContents();
+  void DestroyWebContents(bool async);
 
   int64_t GetID() const;
   int GetProcessID() const;

atom/browser/api/atom_api_window.cc

@@ -173,7 +173,7 @@ void Window::WillDestroyNativeObject() {
 }
 
 void Window::OnWindowClosed() {
-  api_web_contents_->DestroyWebContents();
+  api_web_contents_->DestroyWebContents(true /* async */);
 
   RemoveFromWeakMap();
   window_->RemoveObserver(this);

atom/browser/common_web_contents_delegate.cc

@@ -188,8 +188,13 @@ void CommonWebContentsDelegate::SetOwnerWindow(
   }
 }
 
-void CommonWebContentsDelegate::ResetManagedWebContents() {
-  web_contents_.reset();
+void CommonWebContentsDelegate::ResetManagedWebContents(bool async) {
+  if (async) {
+    base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
+                                                    web_contents_.release());
+  } else {
+    web_contents_.reset();
+  }
 }
 
 content::WebContents* CommonWebContentsDelegate::GetWebContents() const {

atom/browser/common_web_contents_delegate.h

@@ -112,7 +112,7 @@ class CommonWebContentsDelegate
 #endif
 
   // Destroy the managed InspectableWebContents object.
-  void ResetManagedWebContents();
+  void ResetManagedWebContents(bool async);
 
  private:
   // Callback for when DevToolsSaveToFile has completed.

atom/browser/resources/mac/Info.plist

@@ -17,9 +17,9 @@
   <key>CFBundleIconFile</key>
   <string>electron.icns</string>
   <key>CFBundleVersion</key>
-  <string>1.6.7</string>
+  <string>1.6.8</string>
   <key>CFBundleShortVersionString</key>
-  <string>1.6.7</string>
+  <string>1.6.8</string>
   <key>LSApplicationCategoryType</key>
   <string>public.app-category.developer-tools</string>
   <key>LSMinimumSystemVersion</key>

atom/browser/resources/win/atom.rc

@@ -56,8 +56,8 @@ END
 //
 
 VS_VERSION_INFO VERSIONINFO
- FILEVERSION 1,6,7,0
- PRODUCTVERSION 1,6,7,0
+ FILEVERSION 1,6,8,0
+ PRODUCTVERSION 1,6,8,0
  FILEFLAGSMASK 0x3fL
 #ifdef _DEBUG
  FILEFLAGS 0x1L
@@ -74,12 +74,12 @@ BEGIN
         BEGIN
             VALUE "CompanyName", "GitHub, Inc."
             VALUE "FileDescription", "Electron"
-            VALUE "FileVersion", "1.6.7"
+            VALUE "FileVersion", "1.6.8"
             VALUE "InternalName", "electron.exe"
             VALUE "LegalCopyright", "Copyright (C) 2015 GitHub, Inc. All rights reserved."
             VALUE "OriginalFilename", "electron.exe"
             VALUE "ProductName", "Electron"
-            VALUE "ProductVersion", "1.6.7"
+            VALUE "ProductVersion", "1.6.8"
             VALUE "SquirrelAwareVersion", "1"
         END
     END

atom/common/api/object_life_monitor.cc

@@ -12,8 +12,7 @@ namespace atom {
 
 ObjectLifeMonitor::ObjectLifeMonitor(v8::Isolate* isolate,
                                      v8::Local<v8::Object> target)
-    : context_(isolate, isolate->GetCurrentContext()),
-      target_(isolate, target),
+    : target_(isolate, target),
       weak_ptr_factory_(this) {
   target_.SetWeak(this, OnObjectGC, v8::WeakCallbackType::kParameter);
 }

atom/common/api/object_life_monitor.h

@@ -22,7 +22,6 @@ class ObjectLifeMonitor {
   static void OnObjectGC(const v8::WeakCallbackInfo<ObjectLifeMonitor>& data);
   static void Free(const v8::WeakCallbackInfo<ObjectLifeMonitor>& data);
 
-  v8::Global<v8::Context> context_;
   v8::Global<v8::Object> target_;
 
   base::WeakPtrFactory<ObjectLifeMonitor> weak_ptr_factory_;

atom/common/atom_version.h

@@ -7,7 +7,7 @@
 
 #define ATOM_MAJOR_VERSION 1
 #define ATOM_MINOR_VERSION 6
-#define ATOM_PATCH_VERSION 7
+#define ATOM_PATCH_VERSION 8
 
 #define ATOM_VERSION_IS_RELEASE 1
 

docs/api/menu.md

@@ -241,7 +241,7 @@ Linux. Here are some notes on making your app's menu more native-like.
 
 On macOS there are many system-defined standard menus, like the `Services` and
 `Windows` menus. To make your menu a standard menu, you should set your menu's
-`role` to one of following and Electron will recognize them and make them
+`role` to one of the following and Electron will recognize them and make them
 become standard menus:
 
 * `window`

docs/api/touch-bar.md

@@ -138,3 +138,13 @@ app.once('ready', () => {
   window.setTouchBar(touchBar)
 })
 ```
+
+### Running the above example
+
+To run the example above, you'll need to (assuming you've got a terminal open in the dirtectory you want to run the example):
+
+1. Save the above file to your computer as `touchbar.js`
+2. Install Electron via `npm install electron`
+3. Run the example inside Electron: `./node_modules/.bin/electron touchbar.js`
+
+You should then see a new Electron window and the app running in your touch bar (or touch bar emulator).

electron.gyp

@@ -4,7 +4,7 @@
     'product_name%': 'Electron',
     'company_name%': 'GitHub, Inc',
     'company_abbr%': 'github',
-    'version%': '1.6.7',
+    'version%': '1.6.8',
     'js2c_input_dir': '<(SHARED_INTERMEDIATE_DIR)/js2c',
   },
   'includes': [

lib/browser/api/browser-window.js

@@ -76,13 +76,9 @@ BrowserWindow.prototype._init = function () {
 
   // Change window title to page title.
   this.webContents.on('page-title-updated', (event, title) => {
-    // The page-title-updated event is not emitted immediately (see #3645), so
-    // when the callback is called the BrowserWindow might have been closed.
-    if (this.isDestroyed()) return
-
     // Route the event to BrowserWindow.
     this.emit('page-title-updated', event, title)
-    if (!event.defaultPrevented) this.setTitle(title)
+    if (!this.isDestroyed() && !event.defaultPrevented) this.setTitle(title)
   })
 
   // Sometimes the webContents doesn't get focus when window is shown, so we

lib/browser/api/web-contents.js

@@ -268,13 +268,6 @@ WebContents.prototype._init = function () {
     this.reload()
   })
 
-  // Delays the page-title-updated event to next tick.
-  this.on('-page-title-updated', function (...args) {
-    setImmediate(() => {
-      this.emit('page-title-updated', ...args)
-    })
-  })
-
   app.emit('web-contents-created', {}, this)
 }
 

lib/browser/guest-window-manager.js

@@ -309,7 +309,7 @@ ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_POSTMESSAGE', function (event,
   // The W3C does not seem to have word on how postMessage should work when the
   // origins do not match, so we do not do |canAccessWindow| check here since
   // postMessage across origins is useful and not harmful.
-  if (guestContents.getURL().indexOf(targetOrigin) === 0 || targetOrigin === '*') {
+  if (targetOrigin === '*' || isSameOrigin(guestContents.getURL(), targetOrigin)) {
     const sourceId = event.sender.id
     guestContents.send('ELECTRON_GUEST_WINDOW_POSTMESSAGE', sourceId, message, sourceOrigin)
   }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "electron",
-  "version": "1.6.7",
+  "version": "1.6.8",
   "devDependencies": {
     "asar": "^0.11.0",
     "browserify": "^13.1.0",

spec/api-browser-window-spec.js

@@ -86,6 +86,42 @@ describe('BrowserWindow module', function () {
   })
 
   describe('BrowserWindow.close()', function () {
+    let server
+
+    before(function (done) {
+      server = http.createServer((request, response) => {
+        switch (request.url) {
+          case '/404':
+            response.statusCode = '404'
+            response.end()
+            break
+          case '/301':
+            response.statusCode = '301'
+            response.setHeader('Location', '/200')
+            response.end()
+            break
+          case '/200':
+            response.statusCode = '200'
+            response.end('hello')
+            break
+          case '/title':
+            response.statusCode = '200'
+            response.end('<title>Hello</title>')
+            break
+          default:
+            done('unsupported endpoint')
+        }
+      }).listen(0, '127.0.0.1', () => {
+        server.url = 'http://127.0.0.1:' + server.address().port
+        done()
+      })
+    })
+
+    after(function () {
+      server.close()
+      server = null
+    })
+
     it('should emit unload handler', function (done) {
       w.webContents.on('did-finish-load', function () {
         w.close()
@@ -109,6 +145,38 @@ describe('BrowserWindow module', function () {
       })
       w.loadURL('file://' + path.join(fixtures, 'api', 'beforeunload-false.html'))
     })
+
+    it('should not crash when invoked synchronously inside navigation observer', function (done) {
+      const events = [
+        { name: 'did-start-loading', url: `${server.url}/200` },
+        { name: 'did-get-redirect-request', url: `${server.url}/301` },
+        { name: 'did-get-response-details', url: `${server.url}/200` },
+        { name: 'dom-ready', url: `${server.url}/200` },
+        { name: 'page-title-updated', url: `${server.url}/title` },
+        { name: 'did-stop-loading', url: `${server.url}/200` },
+        { name: 'did-finish-load', url: `${server.url}/200` },
+        { name: 'did-frame-finish-load', url: `${server.url}/200` },
+        { name: 'did-fail-load', url: `${server.url}/404` }
+      ]
+      const responseEvent = 'window-webContents-destroyed'
+
+      function* genNavigationEvent () {
+        let eventOptions = null
+        while ((eventOptions = events.shift()) && events.length) {
+          let w = new BrowserWindow({show: false})
+          eventOptions.id = w.id
+          eventOptions.responseEvent = responseEvent
+          ipcRenderer.send('test-webcontents-navigation-observer', eventOptions)
+          yield 1
+        }
+      }
+
+      let gen = genNavigationEvent()
+      ipcRenderer.on(responseEvent, function () {
+        if (!gen.next().value) done()
+      })
+      gen.next()
+    })
   })
 
   describe('window.close()', function () {

spec/api-debugger-spec.js

@@ -1,4 +1,5 @@
 const assert = require('assert')
+const http = require('http')
 const path = require('path')
 const {closeWindow} = require('./window-helpers')
 const BrowserWindow = require('electron').remote.BrowserWindow
@@ -70,6 +71,15 @@ describe('debugger module', function () {
   })
 
   describe('debugger.sendCommand', function () {
+    let server
+
+    afterEach(function () {
+      if (server != null) {
+        server.close()
+        server = null
+      }
+    })
+
     it('retuns response', function (done) {
       w.webContents.loadURL('about:blank')
       try {
@@ -125,5 +135,33 @@ describe('debugger module', function () {
         done()
       })
     })
+
+    it('handles invalid unicode characters in message', function (done) {
+      try {
+        w.webContents.debugger.attach()
+      } catch (err) {
+        done('unexpected error : ' + err)
+      }
+
+      w.webContents.debugger.on('message', (event, method, params) => {
+        if (method === 'Network.loadingFinished') {
+          w.webContents.debugger.sendCommand('Network.getResponseBody', {
+            requestId: params.requestId
+          }, () => {
+            done()
+          })
+        }
+      })
+
+      server = http.createServer((req, res) => {
+        res.setHeader('Content-Type', 'text/plain; charset=utf-8')
+        res.end('\uFFFF')
+      })
+
+      server.listen(0, '127.0.0.1', () => {
+        w.webContents.debugger.sendCommand('Network.enable')
+        w.loadURL(`http://127.0.0.1:${server.address().port}`)
+      })
+    })
   })
 })

spec/api-web-contents-spec.js

@@ -542,4 +542,70 @@ describe('webContents module', function () {
       })
     })
   })
+
+  describe('destroy()', () => {
+    let server
+
+    before(function (done) {
+      server = http.createServer((request, response) => {
+        switch (request.url) {
+          case '/404':
+            response.statusCode = '404'
+            response.end()
+            break
+          case '/301':
+            response.statusCode = '301'
+            response.setHeader('Location', '/200')
+            response.end()
+            break
+          case '/200':
+            response.statusCode = '200'
+            response.end('hello')
+            break
+          default:
+            done('unsupported endpoint')
+        }
+      }).listen(0, '127.0.0.1', () => {
+        server.url = 'http://127.0.0.1:' + server.address().port
+        done()
+      })
+    })
+
+    after(function () {
+      server.close()
+      server = null
+    })
+
+    it('should not crash when invoked synchronously inside navigation observer', (done) => {
+      const events = [
+        { name: 'did-start-loading', url: `${server.url}/200` },
+        { name: 'did-get-redirect-request', url: `${server.url}/301` },
+        { name: 'did-get-response-details', url: `${server.url}/200` },
+        { name: 'dom-ready', url: `${server.url}/200` },
+        { name: 'did-stop-loading', url: `${server.url}/200` },
+        { name: 'did-finish-load', url: `${server.url}/200` },
+        // FIXME: Multiple Emit calls inside an observer assume that object
+        // will be alive till end of the observer. Synchronous `destroy` api
+        // violates this contract and crashes.
+        // { name: 'did-frame-finish-load', url: `${server.url}/200` },
+        { name: 'did-fail-load', url: `${server.url}/404` }
+      ]
+      const responseEvent = 'webcontents-destroyed'
+
+      function* genNavigationEvent () {
+        let eventOptions = null
+        while ((eventOptions = events.shift()) && events.length) {
+          eventOptions.responseEvent = responseEvent
+          ipcRenderer.send('test-webcontents-navigation-observer', eventOptions)
+          yield 1
+        }
+      }
+
+      let gen = genNavigationEvent()
+      ipcRenderer.on(responseEvent, () => {
+        if (!gen.next().value) done()
+      })
+      gen.next()
+    })
+  })
 })

spec/chromium-spec.js

@@ -1,4 +1,5 @@
 const assert = require('assert')
+const fs = require('fs')
 const http = require('http')
 const path = require('path')
 const ws = require('ws')
@@ -618,6 +619,39 @@ describe('chromium feature', function () {
       })
       document.body.appendChild(webview)
     })
+
+    describe('targetOrigin argument', function () {
+      let serverURL
+      let server
+
+      beforeEach(function (done) {
+        server = http.createServer(function (req, res) {
+          res.writeHead(200)
+          const filePath = path.join(fixtures, 'pages', 'window-opener-targetOrigin.html')
+          res.end(fs.readFileSync(filePath, 'utf8'))
+        })
+        server.listen(0, '127.0.0.1', function () {
+          serverURL = `http://127.0.0.1:${server.address().port}`
+          done()
+        })
+      })
+
+      afterEach(function () {
+        server.close()
+      })
+
+      it('delivers messages that match the origin', function (done) {
+        let b
+        listener = function (event) {
+          window.removeEventListener('message', listener)
+          b.close()
+          assert.equal(event.data, 'deliver')
+          done()
+        }
+        window.addEventListener('message', listener)
+        b = window.open(serverURL, '', 'show=no')
+      })
+    })
   })
 
   describe('creating a Uint8Array under browser side', function () {

spec/fixtures/pages/window-opener-targetOrigin.html

@@ -0,0 +1,24 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  const url = require('url')
+  if (url.parse(window.location.href, true).query.opened != null) {
+    // Ensure origins are properly checked by removing a single character from the end
+    window.opener.postMessage('do not deliver substring origin', window.location.origin.substring(0, window.location.origin.length - 1))
+    window.opener.postMessage('do not deliver file://', 'file://')
+    window.opener.postMessage('do not deliver http without port', 'http://127.0.0.1')
+    window.opener.postMessage('do not deliver atom', 'atom://')
+    window.opener.postMessage('do not deliver null', 'null')
+    window.opener.postMessage('do not deliver \\:/', '\\:/')
+    window.opener.postMessage('do not deliver empty', '')
+    window.opener.postMessage('deliver', window.location.origin)
+  } else {
+    const opened = window.open(`${window.location.href}?opened=true`, '', 'show=no')
+    window.addEventListener('message', function (event) {
+      window.opener.postMessage(event.data, '*')
+      opened.close()
+    })
+  }
+</script>
+</body>
+</html>

spec/static/main.js

@@ -338,6 +338,27 @@ ipcMain.on('crash-service-pid', (event, pid) => {
   event.returnValue = null
 })
 
+ipcMain.on('test-webcontents-navigation-observer', (event, options) => {
+  let contents = null
+  let destroy = () => {}
+  if (options.id) {
+    const w = BrowserWindow.fromId(options.id)
+    contents = w.webContents
+    destroy = () => w.close()
+  } else {
+    contents = webContents.create()
+    destroy = () => contents.destroy()
+  }
+
+  contents.once(options.name, () => destroy())
+
+  contents.once('destroyed', () => {
+    event.sender.send(options.responseEvent)
+  })
+
+  contents.loadURL(options.url)
+})
+
 // Suspend listeners until the next event and then restore them
 const suspendListeners = (emitter, eventName, callback) => {
   const listeners = emitter.listeners(eventName)