From a60be1563f444c8e5bc3bbe8b957adf117cf2816 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 27 Dec 2013 14:32:24 +0800 Subject: [PATCH 1/6] Do not implement window.open. --- browser/native_window.cc | 18 ------------------ browser/native_window.h | 5 ----- 2 files changed, 23 deletions(-) diff --git a/browser/native_window.cc b/browser/native_window.cc index ad7773327e6..2411464a723 100644 --- a/browser/native_window.cc +++ b/browser/native_window.cc @@ -280,24 +280,6 @@ void NativeWindow::NotifyWindowBlur() { FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnWindowBlur()); } -// Window opened by window.open. -void NativeWindow::WebContentsCreated( - content::WebContents* source_contents, - int64 source_frame_id, - const string16& frame_name, - const GURL& target_url, - content::WebContents* new_contents) { - LOG(WARNING) << "Please use node-style Window API to create window, " - "using window.open has very strict constrains."; - - scoped_ptr options(new base::DictionaryValue); - options->SetInteger(switches::kWidth, 800); - options->SetInteger(switches::kHeight, 600); - - NativeWindow* window = Create(new_contents, options.get()); - window->InitFromOptions(options.get()); -} - content::JavaScriptDialogManager* NativeWindow::GetJavaScriptDialogManager() { if (!dialog_manager_) dialog_manager_.reset(new AtomJavaScriptDialogManager); diff --git a/browser/native_window.h b/browser/native_window.h index 5e1346138c9..4978ef9bdb7 100644 --- a/browser/native_window.h +++ b/browser/native_window.h @@ -157,11 +157,6 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, const std::vector& regions) = 0; // Implementations of content::WebContentsDelegate. - virtual void WebContentsCreated(content::WebContents* source_contents, - int64 source_frame_id, - const string16& frame_name, - const GURL& target_url, - content::WebContents* new_contents) OVERRIDE; virtual content::JavaScriptDialogManager* GetJavaScriptDialogManager() OVERRIDE; virtual void BeforeUnloadFired(content::WebContents* tab, From 15a9be6b93122d963b31cc741b35a08c12010e71 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 27 Dec 2013 14:39:23 +0800 Subject: [PATCH 2/6] Implement a simple window.open with pure js. --- renderer/lib/init.coffee | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/renderer/lib/init.coffee b/renderer/lib/init.coffee index 198801fe21d..4d03ab06ff9 100644 --- a/renderer/lib/init.coffee +++ b/renderer/lib/init.coffee @@ -59,3 +59,10 @@ window.onerror = (error) -> true else false + +# Override default window.open. +window.open = (url) -> + BrowserWindow = require('remote').require 'browser-window' + browser = new BrowserWindow width: 800, height: 600 + browser.loadUrl url + browser From 1701f572e2f29b27958bfdb63ea74718ad741532 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 27 Dec 2013 14:47:50 +0800 Subject: [PATCH 3/6] Fix crash when calling method of destroyed object. --- browser/api/atom_api_event_emitter.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/browser/api/atom_api_event_emitter.cc b/browser/api/atom_api_event_emitter.cc index c550cfc9402..15dc5ddedaf 100644 --- a/browser/api/atom_api_event_emitter.cc +++ b/browser/api/atom_api_event_emitter.cc @@ -21,6 +21,10 @@ EventEmitter::EventEmitter(v8::Handle wrapper) { } EventEmitter::~EventEmitter() { + // Clear the aligned pointer, it should have been done by ObjectWrap but + // somehow node v0.11.x changed this behaviour. + v8::HandleScope handle_scope(node_isolate); + handle()->SetAlignedPointerInInternalField(0, NULL); } bool EventEmitter::Emit(const std::string& name) { From 0de40febabce34e09a96ae3a10bf105947892eb5 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 27 Dec 2013 14:57:53 +0800 Subject: [PATCH 4/6] Add basic support for window.open's features. --- renderer/lib/init.coffee | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/renderer/lib/init.coffee b/renderer/lib/init.coffee index 4d03ab06ff9..30e1de4e88e 100644 --- a/renderer/lib/init.coffee +++ b/renderer/lib/init.coffee @@ -61,8 +61,25 @@ window.onerror = (error) -> false # Override default window.open. -window.open = (url) -> +window.open = (url, name, features) -> + options = {} + for feature in features.split ',' + [name, value] = feature.split '=' + options[name] = + if value is 'yes' + true + else if value is 'no' + false + else + value + + options.x ?= options.left + options.y ?= options.top + options.title ?= name + options.width ?= 800 + options.height ?= 600 + BrowserWindow = require('remote').require 'browser-window' - browser = new BrowserWindow width: 800, height: 600 + browser = new BrowserWindow options browser.loadUrl url browser From 6312c1108ae325e2369d9d20271a277ebca58311 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 27 Dec 2013 14:58:13 +0800 Subject: [PATCH 5/6] Add spec for window.open. --- spec/chromium-spec.coffee | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/chromium-spec.coffee b/spec/chromium-spec.coffee index 7e41ac8b6a3..d23294210c2 100644 --- a/spec/chromium-spec.coffee +++ b/spec/chromium-spec.coffee @@ -1,3 +1,5 @@ +assert = require 'assert' + describe 'chromium feature', -> describe 'heap snapshot', -> it 'does not crash', -> @@ -13,3 +15,9 @@ describe 'chromium feature', -> navigator.webkitGetUserMedia audio: true, video: false, -> done() -> done() + + describe 'window.open', -> + it 'returns a BrowserWindow object', -> + b = window.open 'about:blank', 'test', 'show=no' + assert.equal b.constructor.name, 'BrowserWindow' + b.destroy() From f28881e2035326aa9e132e25b1ee8a5994c31239 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 27 Dec 2013 15:41:00 +0800 Subject: [PATCH 6/6] Fix crash caused by BrowserWindow.destroy(). --- browser/api/atom_api_window.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/browser/api/atom_api_window.cc b/browser/api/atom_api_window.cc index b2faf6b4a7e..3f97a757278 100644 --- a/browser/api/atom_api_window.cc +++ b/browser/api/atom_api_window.cc @@ -4,6 +4,7 @@ #include "browser/api/atom_api_window.h" +#include "base/bind.h" #include "base/process/kill.h" #include "browser/native_window.h" #include "common/v8/native_type_conversions.h" @@ -127,11 +128,12 @@ void Window::Destroy(const v8::FunctionCallbackInfo& args) { base::ProcessHandle handle = self->window_->GetRenderProcessHandle(); delete self; - // Check if the render process is terminated, it could happen that the render - // became a zombie. - if (!base::WaitForSingleProcess(handle, - base::TimeDelta::FromMilliseconds(500))) - base::KillProcess(handle, 0, true); + // Make sure the renderer process is terminated, it could happen that the + // renderer process became a zombie. + base::MessageLoop::current()->PostDelayedTask( + FROM_HERE, + base::Bind(base::IgnoreResult(base::KillProcess), 0, false, handle), + base::TimeDelta::FromMilliseconds(5000)); } // static