Merge pull request #146 from atom/window-open

Implement window.open in pure js

browser/api/atom_api_event_emitter.cc

@@ -21,6 +21,10 @@ EventEmitter::EventEmitter(v8::Handle<v8::Object> 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) {

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<v8::Value>& args) {
   base::ProcessHandle handle = self->window_->GetRenderProcessHandle();
   delete self;
 
-  // Check if the render process is terminated, it could happen that the render
+  // Make sure the renderer process is terminated, it could happen that the
-  // became a zombie.
+  // renderer process became a zombie.
-  if (!base::WaitForSingleProcess(handle,
+  base::MessageLoop::current()->PostDelayedTask(
-                                  base::TimeDelta::FromMilliseconds(500)))
+      FROM_HERE,
-    base::KillProcess(handle, 0, true);
+      base::Bind(base::IgnoreResult(base::KillProcess), 0, false, handle),
+      base::TimeDelta::FromMilliseconds(5000));
 }
 
 // static

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<base::DictionaryValue> 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);

browser/native_window.h

@@ -157,11 +157,6 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
       const std::vector<DraggableRegion>& 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,

renderer/lib/init.coffee

@@ -59,3 +59,27 @@ window.onerror = (error) ->
     true
   else
     false
+
+# Override default window.open.
+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 options
+  browser.loadUrl url
+  browser

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()