BrowserWindow.focus() should not make window become visible, fixed #106.

If we delete the window immediately other observers may get a invalid
window and cause random crashes.

atom.gyp

@@ -79,8 +79,6 @@
       'browser/atom_browser_main_parts.cc',
       'browser/atom_browser_main_parts.h',
       'browser/atom_browser_main_parts_mac.mm',
-      'browser/atom_event_processing_window.h',
-      'browser/atom_event_processing_window.mm',
       'browser/atom_javascript_dialog_manager.cc',
       'browser/atom_javascript_dialog_manager.h',
       'browser/browser.cc',
@@ -110,10 +108,10 @@
       'browser/ui/accelerator_util.h',
       'browser/ui/accelerator_util_mac.mm',
       'browser/ui/accelerator_util_win.cc',
+      'browser/ui/atom_event_processing_window.h',
+      'browser/ui/atom_event_processing_window.mm',
       'browser/ui/atom_menu_controller_mac.h',
       'browser/ui/atom_menu_controller_mac.mm',
-      'browser/ui/cocoa/custom_frame_view.h',
-      'browser/ui/cocoa/custom_frame_view.mm',
       'browser/ui/file_dialog.h',
       'browser/ui/file_dialog_mac.mm',
       'browser/ui/file_dialog_win.cc',
@@ -157,6 +155,7 @@
       'common/platform_util.h',
       'common/platform_util_mac.mm',
       'common/platform_util_win.cc',
+      'common/v8_conversions.h',
       'common/v8_value_converter_impl.cc',
       'common/v8_value_converter_impl.h',
       'renderer/api/atom_api_renderer_ipc.cc',

browser/api/atom_api_app.cc

@@ -7,6 +7,7 @@
 #include "base/values.h"
 #include "base/command_line.h"
 #include "browser/browser.h"
+#include "common/v8_conversions.h"
 #include "vendor/node/src/node.h"
 
 namespace atom {
@@ -111,14 +112,14 @@ v8::Handle<v8::Value> App::GetVersion(const v8::Arguments &args) {
 v8::Handle<v8::Value> App::AppendSwitch(const v8::Arguments &args) {
   v8::HandleScope scope;
 
-  if (!args[0]->IsString())
+  std::string switch_string;
+  if (!FromV8Arguments(args, &switch_string))
     return node::ThrowError("Bad argument");
 
-  std::string switch_string(*v8::String::Utf8Value(args[0]));
   if (args.Length() == 1) {
     CommandLine::ForCurrentProcess()->AppendSwitch(switch_string);
   } else {
-    std::string value(*v8::String::Utf8Value(args[1]));
+    std::string value = FromV8Value(args[1]);
     CommandLine::ForCurrentProcess()->AppendSwitchASCII(
         switch_string, value);
   }
@@ -130,10 +131,10 @@ v8::Handle<v8::Value> App::AppendSwitch(const v8::Arguments &args) {
 v8::Handle<v8::Value> App::AppendArgument(const v8::Arguments &args) {
   v8::HandleScope scope;
 
-  if (!args[0]->IsString())
+  std::string value;
+  if (!FromV8Arguments(args, &value))
     return node::ThrowError("Bad argument");
 
-  std::string value(*v8::String::Utf8Value(args[0]));
   CommandLine::ForCurrentProcess()->AppendArg(value);
 
   return v8::Undefined();
@@ -143,7 +144,7 @@ v8::Handle<v8::Value> App::AppendArgument(const v8::Arguments &args) {
 
 // static
 v8::Handle<v8::Value> App::DockBounce(const v8::Arguments& args) {
-  std::string type(*v8::String::Utf8Value(args[0]));
+  std::string type = FromV8Value(args[0]);
   int request_id = -1;
 
   if (type == "critical")
@@ -158,21 +159,20 @@ v8::Handle<v8::Value> App::DockBounce(const v8::Arguments& args) {
 
 // static
 v8::Handle<v8::Value> App::DockCancelBounce(const v8::Arguments& args) {
-  Browser::Get()->DockCancelBounce(args[0]->IntegerValue());
+  Browser::Get()->DockCancelBounce(FromV8Value(args[0]));
   return v8::Undefined();
 }
 
 // static
 v8::Handle<v8::Value> App::DockSetBadgeText(const v8::Arguments& args) {
-  std::string label(*v8::String::Utf8Value(args[0]));
-  Browser::Get()->DockSetBadgeText(label);
+  Browser::Get()->DockSetBadgeText(FromV8Value(args[0]));
   return v8::Undefined();
 }
 
 // static
 v8::Handle<v8::Value> App::DockGetBadgeText(const v8::Arguments& args) {
   std::string text(Browser::Get()->DockGetBadgeText());
-  return v8::String::New(text.data(), text.size());
+  return ToV8Value(text);
 }
 
 #endif  // defined(OS_MACOSX)

browser/api/atom_api_auto_updater.cc

@@ -6,6 +6,7 @@
 
 #include "base/values.h"
 #include "browser/auto_updater.h"
+#include "common/v8_conversions.h"
 
 namespace atom {
 
@@ -56,7 +57,7 @@ v8::Handle<v8::Value> AutoUpdater::New(const v8::Arguments &args) {
 
 // static
 v8::Handle<v8::Value> AutoUpdater::SetFeedURL(const v8::Arguments &args) {
-  auto_updater::AutoUpdater::SetFeedURL(*v8::String::Utf8Value(args[0]));
+  auto_updater::AutoUpdater::SetFeedURL(FromV8Value(args[0]));
   return v8::Undefined();
 }
 

browser/api/atom_api_browser_ipc.cc

@@ -6,6 +6,7 @@
 
 #include "base/values.h"
 #include "common/api/api_messages.h"
+#include "common/v8_conversions.h"
 #include "common/v8_value_converter_impl.h"
 #include "content/public/browser/render_view_host.h"
 #include "vendor/node/src/node.h"
@@ -22,13 +23,11 @@ namespace api {
 v8::Handle<v8::Value> BrowserIPC::Send(const v8::Arguments &args) {
   v8::HandleScope scope;
 
-  if (!args[0]->IsString() || !args[1]->IsNumber() || !args[2]->IsNumber())
+  string16 channel;
+  int process_id, routing_id;
+  if (!FromV8Arguments(args, &channel, &process_id, &routing_id))
     return node::ThrowTypeError("Bad argument");
 
-  std::string channel(*v8::String::Utf8Value(args[0]));
-  int process_id = args[1]->IntegerValue();
-  int routing_id = args[2]->IntegerValue();
-
   RenderViewHost* render_view_host(RenderViewHost::FromID(
       process_id, routing_id));
   if (!render_view_host)

browser/api/atom_api_crash_reporter.cc

@@ -5,6 +5,7 @@
 #include "browser/api/atom_api_crash_reporter.h"
 
 #include "browser/crash_reporter.h"
+#include "common/v8_conversions.h"
 #include "vendor/node/src/node.h"
 #include "vendor/node/src/node_internals.h"
 
@@ -14,22 +15,20 @@ namespace api {
 
 // static
 v8::Handle<v8::Value> CrashReporter::SetCompanyName(const v8::Arguments &args) {
-  std::string name(*v8::String::Utf8Value(args[0]));
-  crash_reporter::CrashReporter::SetCompanyName(name);
+  crash_reporter::CrashReporter::SetCompanyName(FromV8Value(args[0]));
   return v8::Undefined();
 }
 
 // static
 v8::Handle<v8::Value> CrashReporter::SetSubmissionURL(
     const v8::Arguments &args) {
-  std::string url(*v8::String::Utf8Value(args[0]));
-  crash_reporter::CrashReporter::SetSubmissionURL(url);
+  crash_reporter::CrashReporter::SetSubmissionURL(FromV8Value(args[0]));
   return v8::Undefined();
 }
 
 // static
 v8::Handle<v8::Value> CrashReporter::SetAutoSubmit(const v8::Arguments &args) {
-  crash_reporter::CrashReporter::SetAutoSubmit(args[0]->BooleanValue());
+  crash_reporter::CrashReporter::SetAutoSubmit(FromV8Value(args[0]));
   return v8::Undefined();
 }
 

browser/api/atom_api_dialog.cc

@@ -4,14 +4,14 @@
 
 #include "browser/api/atom_api_dialog.h"
 
-#include <string>
-
-#include "base/utf_string_conversions.h"
-#include "base/values.h"
-#include "browser/api/atom_api_window.h"
+#include "base/bind.h"
 #include "browser/native_window.h"
 #include "browser/ui/file_dialog.h"
 #include "browser/ui/message_box.h"
+#include "common/v8_conversions.h"
+#include "vendor/node/src/node_internals.h"
+
+using node::node_isolate;
 
 namespace atom {
 
@@ -19,14 +19,24 @@ namespace api {
 
 namespace {
 
-base::FilePath V8ValueToFilePath(v8::Handle<v8::Value> path) {
-  std::string path_string(*v8::String::Utf8Value(path));
-  return base::FilePath::FromUTF8Unsafe(path_string);
+template<typename T>
+void CallV8Function(v8::Persistent<v8::Function> callback, T arg) {
+  DCHECK(!callback.IsEmpty());
+
+  v8::HandleScope scope;
+  v8::Handle<v8::Value> value = ToV8Value(arg);
+  callback->Call(callback, 1, &value);
+  callback.Dispose(node_isolate);
 }
 
-v8::Handle<v8::Value> FilePathToV8Value(const base::FilePath path) {
-  std::string path_string(path.AsUTF8Unsafe());
-  return v8::String::New(path_string.data(), path_string.size());
+template<typename T>
+void CallV8Function2(v8::Persistent<v8::Function> callback,
+                     bool result,
+                     T arg) {
+  if (result)
+    return CallV8Function<T>(callback, arg);
+  else
+    return CallV8Function<void*>(callback, NULL);
 }
 
 void Initialize(v8::Handle<v8::Object> target) {
@@ -42,84 +52,103 @@ void Initialize(v8::Handle<v8::Object> target) {
 v8::Handle<v8::Value> ShowMessageBox(const v8::Arguments &args) {
   v8::HandleScope scope;
 
-  if (!args[0]->IsNumber() ||  // type
-      !args[1]->IsArray() ||   // buttons
-      !args[2]->IsString() ||  // title
-      !args[3]->IsString() ||  // message
-      !args[4]->IsString())    // detail
+  int type;
+  std::vector<std::string> buttons;
+  std::string title, message, detail;
+  if (!FromV8Arguments(args, &type, &buttons, &title, &message, &detail))
     return node::ThrowTypeError("Bad argument");
 
-  NativeWindow* native_window = NULL;
-  if (args[5]->IsObject()) {
-    Window* window = Window::Unwrap<Window>(args[5]->ToObject());
-    if (!window || !window->window())
-      return node::ThrowError("Invalid window");
+  NativeWindow* native_window = FromV8Value(args[5]);
+  v8::Persistent<v8::Function> callback = FromV8Value(args[6]);
 
-    native_window = window->window();
+  if (callback.IsEmpty()) {
+    int chosen = atom::ShowMessageBox(
+        native_window,
+        (MessageBoxType)type,
+        buttons,
+        title,
+        message,
+        detail);
+    return scope.Close(v8::Integer::New(chosen));
+  } else {
+    atom::ShowMessageBox(
+        native_window,
+        (MessageBoxType)type,
+        buttons,
+        title,
+        message,
+        detail,
+        base::Bind(&CallV8Function<int>, callback));
+    return v8::Undefined();
   }
-
-  MessageBoxType type = (MessageBoxType)(args[0]->IntegerValue());
-
-  std::vector<std::string> buttons;
-  v8::Handle<v8::Array> v8_buttons = v8::Handle<v8::Array>::Cast(args[1]);
-  for (uint32_t i = 0; i < v8_buttons->Length(); ++i)
-    buttons.push_back(*v8::String::Utf8Value(v8_buttons->Get(i)));
-
-  std::string title(*v8::String::Utf8Value(args[2]));
-  std::string message(*v8::String::Utf8Value(args[3]));
-  std::string detail(*v8::String::Utf8Value(args[4]));
-
-  int chosen = atom::ShowMessageBox(
-      native_window, type, buttons, title, message, detail);
-  return scope.Close(v8::Integer::New(chosen));
 }
 
 v8::Handle<v8::Value> ShowOpenDialog(const v8::Arguments &args) {
   v8::HandleScope scope;
 
-  if (!args[0]->IsString() ||  // title
-      !args[1]->IsString() ||  // default_path
-      !args[2]->IsNumber())    // properties
+  std::string title;
+  base::FilePath default_path;
+  int properties;
+  if (!FromV8Arguments(args, &title, &default_path, &properties))
     return node::ThrowTypeError("Bad argument");
 
-  std::string title(*v8::String::Utf8Value(args[0]));
-  base::FilePath default_path(V8ValueToFilePath(args[1]));
-  int properties = args[2]->IntegerValue();
+  NativeWindow* native_window = FromV8Value(args[3]);
+  v8::Persistent<v8::Function> callback = FromV8Value(args[4]);
 
-  std::vector<base::FilePath> paths;
-  if (!file_dialog::ShowOpenDialog(title, default_path, properties, &paths))
+  if (callback.IsEmpty()) {
+    std::vector<base::FilePath> paths;
+    if (!file_dialog::ShowOpenDialog(native_window,
+                                     title,
+                                     default_path,
+                                     properties,
+                                     &paths))
+      return v8::Undefined();
+
+    v8::Handle<v8::Array> result = v8::Array::New(paths.size());
+    for (size_t i = 0; i < paths.size(); ++i)
+      result->Set(i, ToV8Value(paths[i]));
+
+    return scope.Close(result);
+  } else {
+    file_dialog::ShowOpenDialog(
+        native_window,
+        title,
+        default_path,
+        properties,
+        base::Bind(&CallV8Function2<const std::vector<base::FilePath>&>,
+                   callback));
     return v8::Undefined();
-
-  v8::Handle<v8::Array> result = v8::Array::New(paths.size());
-  for (size_t i = 0; i < paths.size(); ++i)
-    result->Set(i, FilePathToV8Value(paths[i]));
-
-  return scope.Close(result);
+  }
 }
 
 v8::Handle<v8::Value> ShowSaveDialog(const v8::Arguments &args) {
   v8::HandleScope scope;
 
-  if (!args[0]->IsObject() ||  // window
-      !args[1]->IsString() ||  // title
-      !args[2]->IsString())    // default_path
+  std::string title;
+  base::FilePath default_path;
+  if (!FromV8Arguments(args, &title, &default_path))
     return node::ThrowTypeError("Bad argument");
 
-  Window* window = Window::Unwrap<Window>(args[0]->ToObject());
-  if (!window || !window->window())
-    return node::ThrowError("Invalid window");
+  NativeWindow* native_window = FromV8Value(args[2]);
+  v8::Persistent<v8::Function> callback = FromV8Value(args[3]);
 
-  std::string title(*v8::String::Utf8Value(args[1]));
-  base::FilePath default_path(V8ValueToFilePath(args[2]));
+  if (callback.IsEmpty()) {
+    base::FilePath path;
+    if (!file_dialog::ShowSaveDialog(native_window,
+                                     title,
+                                     default_path,
+                                     &path))
+      return v8::Undefined();
 
-  base::FilePath path;
-  if (!file_dialog::ShowSaveDialog(window->window(),
-                                   title,
-                                   default_path,
-                                   &path))
+    return scope.Close(ToV8Value(path));
+  } else {
+    file_dialog::ShowSaveDialog(
+        native_window,
+        title,
+        default_path,
+        base::Bind(&CallV8Function2<const base::FilePath&>, callback));
     return v8::Undefined();
-
-  return scope.Close(FilePathToV8Value(path));
+  }
 }
 
 }  // namespace api

browser/api/atom_api_event.cc

@@ -4,6 +4,10 @@
 
 #include "browser/api/atom_api_event.h"
 
+#include "browser/native_window.h"
+#include "common/api/api_messages.h"
+#include "common/v8_conversions.h"
+
 using node::node_isolate;
 
 namespace atom {
@@ -13,10 +17,14 @@ namespace api {
 v8::Persistent<v8::FunctionTemplate> Event::constructor_template_;
 
 Event::Event()
-    : prevent_default_(false) {
+    : sender_(NULL),
+      message_(NULL),
+      prevent_default_(false) {
 }
 
 Event::~Event() {
+  if (sender_ != NULL)
+    sender_->RemoveObserver(this);
 }
 
 // static
@@ -31,6 +39,8 @@ v8::Handle<v8::Object> Event::CreateV8Object() {
     constructor_template_->SetClassName(v8::String::NewSymbol("Event"));
 
     NODE_SET_PROTOTYPE_METHOD(t, "preventDefault", PreventDefault);
+    NODE_SET_PROTOTYPE_METHOD(t, "sendReply", SendReply);
+    NODE_SET_PROTOTYPE_METHOD(t, "destroy", Destroy);
   }
 
   v8::Handle<v8::Object> v8_event =
@@ -39,14 +49,30 @@ v8::Handle<v8::Object> Event::CreateV8Object() {
   return scope.Close(v8_event);
 }
 
-v8::Handle<v8::Value> Event::New(const v8::Arguments &args) {
+void Event::SetSenderAndMessage(NativeWindow* sender, IPC::Message* message) {
+  DCHECK(!sender_);
+  DCHECK(!message_);
+  sender_ = sender;
+  message_ = message;
+
+  sender_->AddObserver(this);
+}
+
+void Event::OnWindowClosed() {
+  sender_ = NULL;
+  message_ = NULL;
+}
+
+// static
+v8::Handle<v8::Value> Event::New(const v8::Arguments& args) {
   Event* event = new Event;
   event->Wrap(args.This());
 
   return args.This();
 }
 
-v8::Handle<v8::Value> Event::PreventDefault(const v8::Arguments &args) {
+// static
+v8::Handle<v8::Value> Event::PreventDefault(const v8::Arguments& args) {
   Event* event = Unwrap<Event>(args.This());
   if (event == NULL)
     return node::ThrowError("Event is already destroyed");
@@ -56,6 +82,31 @@ v8::Handle<v8::Value> Event::PreventDefault(const v8::Arguments &args) {
   return v8::Undefined();
 }
 
+// static
+v8::Handle<v8::Value> Event::SendReply(const v8::Arguments& args) {
+  Event* event = Unwrap<Event>(args.This());
+  if (event == NULL)
+    return node::ThrowError("Event is already destroyed");
+
+  if (event->message_ == NULL || event->sender_ == NULL)
+    return node::ThrowError("Can only send reply to synchronous events");
+
+  string16 json = FromV8Value(args[0]);
+
+  AtomViewHostMsg_Message_Sync::WriteReplyParams(event->message_, json);
+  event->sender_->Send(event->message_);
+
+  delete event;
+
+  return v8::Undefined();
+}
+
+// static
+v8::Handle<v8::Value> Event::Destroy(const v8::Arguments& args) {
+  delete Unwrap<Event>(args.This());
+  return v8::Undefined();
+}
+
 }  // namespace api
 
 }  // namespace atom

browser/api/atom_api_event.h

@@ -6,19 +6,32 @@
 #define ATOM_BROWSER_ATOM_API_EVENT_H_
 
 #include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/string16.h"
+#include "browser/native_window_observer.h"
 #include "vendor/node/src/node_object_wrap.h"
 
+namespace IPC {
+class Message;
+}
+
 namespace atom {
 
+class NativeWindow;
+
 namespace api {
 
-class Event : public node::ObjectWrap {
+class Event : public node::ObjectWrap,
+              public NativeWindowObserver {
  public:
   virtual ~Event();
 
   // Create a V8 Event object.
   static v8::Handle<v8::Object> CreateV8Object();
 
+  // Pass the sender and message to be replied.
+  void SetSenderAndMessage(NativeWindow* sender, IPC::Message* message);
+
   // Accessor to return handle_, this follows Google C++ Style.
   v8::Persistent<v8::Object>& handle() { return handle_; }
 
@@ -28,12 +41,22 @@ class Event : public node::ObjectWrap {
  protected:
   Event();
 
+  // NativeWindowObserver implementations:
+  virtual void OnWindowClosed() OVERRIDE;
+
  private:
-  static v8::Handle<v8::Value> New(const v8::Arguments &args);
-  static v8::Handle<v8::Value> PreventDefault(const v8::Arguments &args);
+  static v8::Handle<v8::Value> New(const v8::Arguments& args);
+
+  static v8::Handle<v8::Value> PreventDefault(const v8::Arguments& args);
+  static v8::Handle<v8::Value> SendReply(const v8::Arguments& args);
+  static v8::Handle<v8::Value> Destroy(const v8::Arguments& args);
 
   static v8::Persistent<v8::FunctionTemplate> constructor_template_;
 
+  // Replyer for the synchronous messages.
+  NativeWindow* sender_;
+  IPC::Message* message_;
+
   bool prevent_default_;
 
   DISALLOW_COPY_AND_ASSIGN(Event);

browser/api/atom_api_menu.cc

@@ -4,8 +4,8 @@
 
 #include "browser/api/atom_api_menu.h"
 
-#include "browser/api/atom_api_window.h"
 #include "browser/ui/accelerator_util.h"
+#include "common/v8_conversions.h"
 
 #define UNWRAP_MEMNU_AND_CHECK \
   Menu* self = ObjectWrap::Unwrap<Menu>(args.This()); \
@@ -18,17 +18,6 @@ namespace api {
 
 namespace {
 
-// Converts a V8 value to a string16.
-string16 V8ValueToUTF16(v8::Handle<v8::Value> value) {
-  v8::String::Value s(value);
-  return string16(reinterpret_cast<const char16*>(*s), s.length());
-}
-
-// Converts string16 to V8 String.
-v8::Handle<v8::Value> UTF16ToV8Value(const string16& s) {
-  return v8::String::New(reinterpret_cast<const uint16_t*>(s.data()), s.size());
-}
-
 // Call method of delegate object.
 v8::Handle<v8::Value> CallDelegate(v8::Handle<v8::Value> default_value,
                                    v8::Handle<v8::Object> menu,
@@ -93,7 +82,7 @@ bool Menu::GetAcceleratorForCommandId(int command_id,
                                                 "getAcceleratorForCommandId",
                                                 command_id);
   if (shortcut->IsString()) {
-    std::string shortcut_str(*v8::String::Utf8Value(shortcut));
+    std::string shortcut_str = FromV8Value(shortcut);
     return accelerator_util::StringToAccelerator(shortcut_str, accelerator);
   }
 
@@ -110,18 +99,18 @@ bool Menu::IsItemForCommandIdDynamic(int command_id) const {
 
 string16 Menu::GetLabelForCommandId(int command_id) const {
   v8::HandleScope scope;
-  return V8ValueToUTF16(CallDelegate(v8::False(),
-                                     handle(),
-                                     "getLabelForCommandId",
-                                     command_id));
+  return FromV8Value(CallDelegate(v8::False(),
+                                  handle(),
+                                  "getLabelForCommandId",
+                                  command_id));
 }
 
 string16 Menu::GetSublabelForCommandId(int command_id) const {
   v8::HandleScope scope;
-  return V8ValueToUTF16(CallDelegate(v8::False(),
-                                     handle(),
-                                     "getSubLabelForCommandId",
-                                     command_id));
+  return FromV8Value(CallDelegate(v8::False(),
+                                  handle(),
+                                  "getSubLabelForCommandId",
+                                  command_id));
 }
 
 void Menu::ExecuteCommand(int command_id, int event_flags) {
@@ -145,16 +134,15 @@ v8::Handle<v8::Value> Menu::New(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::InsertItem(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
 
-  if (!args[0]->IsNumber() || !args[1]->IsNumber() || !args[2]->IsString())
+  int index, command_id;
+  string16 label;
+  if (!FromV8Arguments(args, &index, &command_id, &label))
     return node::ThrowTypeError("Bad argument");
 
-  int index = args[0]->IntegerValue();
-
   if (index < 0)
-    self->model_->AddItem(args[1]->IntegerValue(), V8ValueToUTF16(args[2]));
+    self->model_->AddItem(command_id, label);
   else
-    self->model_->InsertItemAt(
-        index, args[1]->IntegerValue(), V8ValueToUTF16(args[2]));
+    self->model_->InsertItemAt(index, command_id, label);
 
   return v8::Undefined();
 }
@@ -163,16 +151,15 @@ v8::Handle<v8::Value> Menu::InsertItem(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::InsertCheckItem(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
 
-  if (!args[0]->IsNumber() || !args[1]->IsNumber() || !args[2]->IsString())
+  int index, command_id;
+  string16 label;
+  if (!FromV8Arguments(args, &index, &command_id, &label))
     return node::ThrowTypeError("Bad argument");
 
-  int index = args[0]->IntegerValue();
-  int command_id = args[1]->IntegerValue();
-
   if (index < 0)
-    self->model_->AddCheckItem(command_id, V8ValueToUTF16(args[2]));
+    self->model_->AddCheckItem(command_id, label);
   else
-    self->model_->InsertCheckItemAt(index, command_id, V8ValueToUTF16(args[2]));
+    self->model_->InsertCheckItemAt(index, command_id, label);
 
   return v8::Undefined();
 }
@@ -181,21 +168,15 @@ v8::Handle<v8::Value> Menu::InsertCheckItem(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::InsertRadioItem(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
 
-  if (!args[0]->IsNumber() ||
-      !args[1]->IsNumber() ||
-      !args[2]->IsString() ||
-      !args[3]->IsNumber())
+  int index, command_id, group_id;
+  string16 label;
+  if (!FromV8Arguments(args, &index, &command_id, &label, &group_id))
     return node::ThrowTypeError("Bad argument");
 
-  int index = args[0]->IntegerValue();
-  int command_id = args[1]->IntegerValue();
-  int group_id = args[3]->IntegerValue();
-
   if (index < 0)
-    self->model_->AddRadioItem(command_id, V8ValueToUTF16(args[2]), group_id);
+    self->model_->AddRadioItem(command_id, label, group_id);
   else
-    self->model_->InsertRadioItemAt(
-        index, command_id, V8ValueToUTF16(args[2]), group_id);
+    self->model_->InsertRadioItemAt(index, command_id, label, group_id);
 
   return v8::Undefined();
 }
@@ -204,11 +185,10 @@ v8::Handle<v8::Value> Menu::InsertRadioItem(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::InsertSeparator(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
 
-  if (!args[0]->IsNumber())
+  int index;
+  if (!FromV8Arguments(args, &index))
     return node::ThrowTypeError("Bad argument");
 
-  int index = args[0]->IntegerValue();
-
   if (index < 0)
     self->model_->AddSeparator(ui::NORMAL_SEPARATOR);
   else
@@ -221,25 +201,20 @@ v8::Handle<v8::Value> Menu::InsertSeparator(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::InsertSubMenu(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
 
-  if (!args[0]->IsNumber() ||
-      !args[1]->IsNumber() ||
-      !args[2]->IsString() ||
-      !args[3]->IsObject())
+  int index, command_id;
+  string16 label;
+  if (!FromV8Arguments(args, &index, &command_id, &label))
     return node::ThrowTypeError("Bad argument");
 
   Menu* submenu = ObjectWrap::Unwrap<Menu>(args[3]->ToObject());
   if (!submenu)
     return node::ThrowTypeError("The submenu is already destroyed");
 
-  int index = args[0]->IntegerValue();
-  int command_id = args[1]->IntegerValue();
-
   if (index < 0)
-    self->model_->AddSubMenu(
-        command_id, V8ValueToUTF16(args[2]), submenu->model_.get());
+    self->model_->AddSubMenu(command_id, label, submenu->model_.get());
   else
     self->model_->InsertSubMenuAt(
-        index, command_id, V8ValueToUTF16(args[2]), submenu->model_.get());
+        index, command_id, label, submenu->model_.get());
 
   return v8::Undefined();
 }
@@ -248,7 +223,9 @@ v8::Handle<v8::Value> Menu::InsertSubMenu(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::SetIcon(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
 
-  if (!args[0]->IsNumber() || !args[1]->IsString())
+  int index;
+  base::FilePath path;
+  if (!FromV8Arguments(args, &index, &path))
     return node::ThrowTypeError("Bad argument");
 
   // FIXME use webkit_glue's image decoder here.
@@ -260,10 +237,12 @@ v8::Handle<v8::Value> Menu::SetIcon(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::SetSublabel(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
 
-  if (!args[0]->IsNumber() || !args[1]->IsString())
+  int index;
+  string16 label;
+  if (!FromV8Arguments(args, &index, &label))
     return node::ThrowTypeError("Bad argument");
 
-  self->model_->SetSublabel(args[0]->IntegerValue(), V8ValueToUTF16(args[1]));
+  self->model_->SetSublabel(index, label);
 
   return v8::Undefined();
 }
@@ -301,14 +280,14 @@ v8::Handle<v8::Value> Menu::GetCommandIdAt(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::GetLabelAt(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
   int index = args[0]->IntegerValue();
-  return UTF16ToV8Value(self->model_->GetLabelAt(index));
+  return ToV8Value(self->model_->GetLabelAt(index));
 }
 
 // static
 v8::Handle<v8::Value> Menu::GetSublabelAt(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
   int index = args[0]->IntegerValue();
-  return UTF16ToV8Value(self->model_->GetSublabelAt(index));
+  return ToV8Value(self->model_->GetSublabelAt(index));
 }
 
 // static
@@ -334,11 +313,11 @@ v8::Handle<v8::Value> Menu::IsVisibleAt(const v8::Arguments &args) {
 v8::Handle<v8::Value> Menu::Popup(const v8::Arguments &args) {
   UNWRAP_MEMNU_AND_CHECK;
 
-  Window* window = Window::Unwrap<Window>(args[0]->ToObject());
-  if (!window)
-    return node::ThrowTypeError("Invalid window");
+  atom::NativeWindow* window;
+  if (!FromV8Arguments(args, &window))
+    return node::ThrowTypeError("Bad argument");
 
-  self->Popup(window->window());
+  self->Popup(window);
   return v8::Undefined();
 }
 

browser/api/atom_api_menu_mac.mm

@@ -8,6 +8,7 @@
 #include "base/mac/scoped_sending_event.h"
 #include "base/strings/sys_string_conversions.h"
 #include "browser/native_window.h"
+#include "common/v8_conversions.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 
@@ -95,10 +96,10 @@ v8::Handle<v8::Value> Menu::SendActionToFirstResponder(
     const v8::Arguments &args) {
   v8::HandleScope scope;
 
-  if (!args[0]->IsString())
+  std::string action;
+  if (!FromV8Arguments(args, &action))
     return node::ThrowTypeError("Bad argument");
 
-  std::string action(*v8::String::Utf8Value(args[0]));
   MenuMac::SendActionToFirstResponder(action);
 
   return v8::Undefined();

browser/api/atom_api_protocol.cc

@@ -9,6 +9,7 @@
 #include "browser/net/adapter_request_job.h"
 #include "browser/net/atom_url_request_context_getter.h"
 #include "browser/net/atom_url_request_job_factory.h"
+#include "common/v8_conversions.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_request_context.h"
 #include "vendor/node/src/node.h"
@@ -29,13 +30,16 @@ v8::Persistent<v8::Object> g_protocol_object;
 typedef std::map<std::string, v8::Persistent<v8::Function>> HandlersMap;
 static HandlersMap g_handlers;
 
+static const char* kEarlyUseProtocolError = "This method can only be used"
+    "after the application has finished launching.";
+
 // Emit an event for the protocol module.
 void EmitEventInUI(const std::string& event, const std::string& parameter) {
   v8::HandleScope scope;
 
-  v8::Local<v8::Value> argv[] = {
-    v8::String::New(event.data(), event.size()),
-    v8::String::New(parameter.data(), parameter.size()),
+  v8::Handle<v8::Value> argv[] = {
+    ToV8Value(event),
+    ToV8Value(parameter),
   };
   node::MakeCallback(g_protocol_object, "emit", arraysize(argv), argv);
 }
@@ -80,7 +84,7 @@ class CustomProtocolRequestJob : public AdapterRequestJob {
 
     // Determine the type of the job we are going to create.
     if (result->IsString()) {
-      std::string data = *v8::String::Utf8Value(result);
+      std::string data = FromV8Value(result);
       content::BrowserThread::PostTask(
           content::BrowserThread::IO,
           FROM_HERE,
@@ -92,14 +96,12 @@ class CustomProtocolRequestJob : public AdapterRequestJob {
       return;
     } else if (result->IsObject()) {
       v8::Handle<v8::Object> obj = result->ToObject();
-      std::string name = *v8::String::Utf8Value(obj->GetConstructorName());
+      std::string name = FromV8Value(obj->GetConstructorName());
       if (name == "RequestStringJob") {
-        std::string mime_type = *v8::String::Utf8Value(obj->Get(
+        std::string mime_type = FromV8Value(obj->Get(
             v8::String::New("mimeType")));
-        std::string charset = *v8::String::Utf8Value(obj->Get(
-            v8::String::New("charset")));
-        std::string data = *v8::String::Utf8Value(obj->Get(
-            v8::String::New("data")));
+        std::string charset = FromV8Value(obj->Get(v8::String::New("charset")));
+        std::string data = FromV8Value(obj->Get(v8::String::New("data")));
 
         content::BrowserThread::PostTask(
             content::BrowserThread::IO,
@@ -111,8 +113,7 @@ class CustomProtocolRequestJob : public AdapterRequestJob {
                        data));
         return;
       } else if (name == "RequestFileJob") {
-        base::FilePath path = base::FilePath::FromUTF8Unsafe(
-            *v8::String::Utf8Value(obj->Get(v8::String::New("path"))));
+        base::FilePath path = FromV8Value(obj->Get(v8::String::New("path")));
 
         content::BrowserThread::PostTask(
             content::BrowserThread::IO,
@@ -180,16 +181,20 @@ class CustomProtocolHandler : public ProtocolHandler {
 
 // static
 v8::Handle<v8::Value> Protocol::RegisterProtocol(const v8::Arguments& args) {
-  std::string scheme(*v8::String::Utf8Value(args[0]));
+  std::string scheme;
+  v8::Persistent<v8::Function> callback;
+  if (!FromV8Arguments(args, &scheme, &callback))
+    return node::ThrowTypeError("Bad argument");
+
   if (g_handlers.find(scheme) != g_handlers.end() ||
       net::URLRequest::IsHandledProtocol(scheme))
     return node::ThrowError("The scheme is already registered");
 
+  if (AtomBrowserContext::Get()->url_request_context_getter() == NULL)
+    return node::ThrowError(kEarlyUseProtocolError);
+
   // Store the handler in a map.
-  if (!args[1]->IsFunction())
-    return node::ThrowError("Handler must be a function");
-  g_handlers[scheme] = v8::Persistent<v8::Function>::New(
-      node::node_isolate, v8::Handle<v8::Function>::Cast(args[1]));
+  g_handlers[scheme] = callback;
 
   content::BrowserThread::PostTask(content::BrowserThread::IO,
                                    FROM_HERE,
@@ -200,7 +205,12 @@ v8::Handle<v8::Value> Protocol::RegisterProtocol(const v8::Arguments& args) {
 
 // static
 v8::Handle<v8::Value> Protocol::UnregisterProtocol(const v8::Arguments& args) {
-  std::string scheme(*v8::String::Utf8Value(args[0]));
+  std::string scheme;
+  if (!FromV8Arguments(args, &scheme))
+    return node::ThrowTypeError("Bad argument");
+
+  if (AtomBrowserContext::Get()->url_request_context_getter() == NULL)
+    return node::ThrowError(kEarlyUseProtocolError);
 
   // Erase the handler from map.
   HandlersMap::iterator it(g_handlers.find(scheme));
@@ -217,24 +227,27 @@ v8::Handle<v8::Value> Protocol::UnregisterProtocol(const v8::Arguments& args) {
 
 // static
 v8::Handle<v8::Value> Protocol::IsHandledProtocol(const v8::Arguments& args) {
-  return v8::Boolean::New(net::URLRequest::IsHandledProtocol(
-      *v8::String::Utf8Value(args[0])));
+  return ToV8Value(net::URLRequest::IsHandledProtocol(FromV8Value(args[0])));
 }
 
 // static
 v8::Handle<v8::Value> Protocol::InterceptProtocol(const v8::Arguments& args) {
-  std::string scheme(*v8::String::Utf8Value(args[0]));
+  std::string scheme;
+  v8::Persistent<v8::Function> callback;
+  if (!FromV8Arguments(args, &scheme, &callback))
+    return node::ThrowTypeError("Bad argument");
+
   if (!GetRequestJobFactory()->HasProtocolHandler(scheme))
     return node::ThrowError("Cannot intercept procotol");
 
   if (ContainsKey(g_handlers, scheme))
     return node::ThrowError("Cannot intercept custom procotols");
 
+  if (AtomBrowserContext::Get()->url_request_context_getter() == NULL)
+    return node::ThrowError(kEarlyUseProtocolError);
+
   // Store the handler in a map.
-  if (!args[1]->IsFunction())
-    return node::ThrowError("Handler must be a function");
-  g_handlers[scheme] = v8::Persistent<v8::Function>::New(
-      node::node_isolate, v8::Handle<v8::Function>::Cast(args[1]));
+  g_handlers[scheme] = callback;
 
   content::BrowserThread::PostTask(content::BrowserThread::IO,
                                    FROM_HERE,
@@ -244,7 +257,12 @@ v8::Handle<v8::Value> Protocol::InterceptProtocol(const v8::Arguments& args) {
 
 // static
 v8::Handle<v8::Value> Protocol::UninterceptProtocol(const v8::Arguments& args) {
-  std::string scheme(*v8::String::Utf8Value(args[0]));
+  std::string scheme;
+  if (!FromV8Arguments(args, &scheme))
+    return node::ThrowTypeError("Bad argument");
+
+  if (AtomBrowserContext::Get()->url_request_context_getter() == NULL)
+    return node::ThrowError(kEarlyUseProtocolError);
 
   // Erase the handler from map.
   HandlersMap::iterator it(g_handlers.find(scheme));

browser/api/atom_api_window.cc

@@ -6,6 +6,7 @@
 
 #include "base/values.h"
 #include "browser/native_window.h"
+#include "common/v8_conversions.h"
 #include "common/v8_value_converter_impl.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
@@ -27,15 +28,6 @@ namespace atom {
 
 namespace api {
 
-namespace {
-
-// Converts string16 to V8 String.
-v8::Handle<v8::String> UTF16ToV8String(const string16& s) {
-  return v8::String::New(reinterpret_cast<const uint16_t*>(s.data()), s.size());
-}
-
-}  // namespace
-
 Window::Window(v8::Handle<v8::Object> wrapper, base::DictionaryValue* options)
     : EventEmitter(wrapper),
       window_(NativeWindow::Create(options)) {
@@ -69,8 +61,9 @@ void Window::WillCloseWindow(bool* prevent_default) {
 void Window::OnWindowClosed() {
   Emit("closed");
 
-  // Free memory immediately when window is closed.
-  delete this;
+  // Free memory when native window is closed, the delete is delayed so other
+  // observers would not get a invalid pointer of NativeWindow.
+  base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
 
 void Window::OnWindowBlur() {
@@ -141,7 +134,7 @@ v8::Handle<v8::Value> Window::Focus(const v8::Arguments &args) {
 // static
 v8::Handle<v8::Value> Window::IsFocused(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
-  return v8::Boolean::New(self->window_->IsFocused());
+  return ToV8Value(self->window_->IsFocused());
 }
 
 // static
@@ -162,6 +155,12 @@ v8::Handle<v8::Value> Window::Hide(const v8::Arguments &args) {
   return v8::Undefined();
 }
 
+// static
+v8::Handle<v8::Value> Window::IsVisible(const v8::Arguments& args) {
+  UNWRAP_WINDOW_AND_CHECK;
+  return ToV8Value(self->window_->IsVisible());
+}
+
 // static
 v8::Handle<v8::Value> Window::Maximize(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
@@ -202,10 +201,11 @@ v8::Handle<v8::Value> Window::Restore(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::SetFullscreen(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1 || !args[0]->IsBoolean())
+  bool fs;
+  if (!FromV8Arguments(args, &fs))
     return node::ThrowTypeError("Bad argument");
-  self->window_->SetFullscreen(args[0]->BooleanValue());
 
+  self->window_->SetFullscreen(fs);
   return v8::Undefined();
 }
 
@@ -213,18 +213,18 @@ v8::Handle<v8::Value> Window::SetFullscreen(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::IsFullscreen(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Boolean::New(self->window_->IsFullscreen());
+  return ToV8Value(self->window_->IsFullscreen());
 }
 
 // static
 v8::Handle<v8::Value> Window::SetSize(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 2)
+  int width, height;
+  if (!FromV8Arguments(args, &width, &height))
     return node::ThrowTypeError("Bad argument");
-  self->window_->SetSize(
-      gfx::Size(args[0]->IntegerValue(), args[1]->IntegerValue()));
 
+  self->window_->SetSize(gfx::Size(width, height));
   return v8::Undefined();
 }
 
@@ -234,8 +234,8 @@ v8::Handle<v8::Value> Window::GetSize(const v8::Arguments &args) {
 
   gfx::Size size = self->window_->GetSize();
   v8::Handle<v8::Array> ret = v8::Array::New(2);
-  ret->Set(0, v8::Integer::New(size.width()));
-  ret->Set(1, v8::Integer::New(size.height()));
+  ret->Set(0, ToV8Value(size.width()));
+  ret->Set(1, ToV8Value(size.height()));
 
   return ret;
 }
@@ -244,11 +244,11 @@ v8::Handle<v8::Value> Window::GetSize(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::SetMinimumSize(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 2)
+  int width, height;
+  if (!FromV8Arguments(args, &width, &height))
     return node::ThrowTypeError("Bad argument");
-  self->window_->SetMinimumSize(
-      gfx::Size(args[0]->IntegerValue(), args[1]->IntegerValue()));
 
+  self->window_->SetMinimumSize(gfx::Size(width, height));
   return v8::Undefined();
 }
 
@@ -258,8 +258,8 @@ v8::Handle<v8::Value> Window::GetMinimumSize(const v8::Arguments &args) {
 
   gfx::Size size = self->window_->GetMinimumSize();
   v8::Handle<v8::Array> ret = v8::Array::New(2);
-  ret->Set(0, v8::Integer::New(size.width()));
-  ret->Set(1, v8::Integer::New(size.height()));
+  ret->Set(0, ToV8Value(size.width()));
+  ret->Set(1, ToV8Value(size.height()));
 
   return ret;
 }
@@ -268,11 +268,12 @@ v8::Handle<v8::Value> Window::GetMinimumSize(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::SetMaximumSize(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 2)
-    return node::ThrowTypeError("Bad argument");
-  self->window_->SetMaximumSize(
-      gfx::Size(args[0]->IntegerValue(), args[1]->IntegerValue()));
 
+  int width, height;
+  if (!FromV8Arguments(args, &width, &height))
+    return node::ThrowTypeError("Bad argument");
+
+  self->window_->SetMaximumSize(gfx::Size(width, height));
   return v8::Undefined();
 }
 
@@ -282,8 +283,8 @@ v8::Handle<v8::Value> Window::GetMaximumSize(const v8::Arguments &args) {
 
   gfx::Size size = self->window_->GetMaximumSize();
   v8::Handle<v8::Array> ret = v8::Array::New(2);
-  ret->Set(0, v8::Integer::New(size.width()));
-  ret->Set(1, v8::Integer::New(size.height()));
+  ret->Set(0, ToV8Value(size.width()));
+  ret->Set(1, ToV8Value(size.height()));
 
   return ret;
 }
@@ -292,10 +293,11 @@ v8::Handle<v8::Value> Window::GetMaximumSize(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::SetResizable(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1 || !args[0]->IsBoolean())
+  bool resizable;
+  if (!FromV8Arguments(args, &resizable))
     return node::ThrowTypeError("Bad argument");
-  self->window_->SetResizable(args[0]->BooleanValue());
 
+  self->window_->SetResizable(resizable);
   return v8::Undefined();
 }
 
@@ -303,17 +305,18 @@ v8::Handle<v8::Value> Window::SetResizable(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::IsResizable(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Boolean::New(self->window_->IsResizable());
+  return ToV8Value(self->window_->IsResizable());
 }
 
 // static
 v8::Handle<v8::Value> Window::SetAlwaysOnTop(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1 || !args[0]->IsBoolean())
+  bool top;
+  if (!FromV8Arguments(args, &top))
     return node::ThrowTypeError("Bad argument");
-  self->window_->SetAlwaysOnTop(args[0]->BooleanValue());
 
+  self->window_->SetAlwaysOnTop(top);
   return v8::Undefined();
 }
 
@@ -321,7 +324,7 @@ v8::Handle<v8::Value> Window::SetAlwaysOnTop(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::IsAlwaysOnTop(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Boolean::New(self->window_->IsAlwaysOnTop());
+  return ToV8Value(self->window_->IsAlwaysOnTop());
 }
 
 // static
@@ -337,11 +340,11 @@ v8::Handle<v8::Value> Window::Center(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::SetPosition(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 2)
+  int x, y;
+  if (!FromV8Arguments(args, &x, &y))
     return node::ThrowTypeError("Bad argument");
-  self->window_->SetPosition(
-      gfx::Point(args[0]->IntegerValue(), args[1]->IntegerValue()));
 
+  self->window_->SetPosition(gfx::Point(x, y));
   return v8::Undefined();
 }
 
@@ -351,8 +354,8 @@ v8::Handle<v8::Value> Window::GetPosition(const v8::Arguments &args) {
 
   gfx::Point pos = self->window_->GetPosition();
   v8::Handle<v8::Array> ret = v8::Array::New(2);
-  ret->Set(0, v8::Integer::New(pos.x()));
-  ret->Set(1, v8::Integer::New(pos.y()));
+  ret->Set(0, ToV8Value(pos.x()));
+  ret->Set(1, ToV8Value(pos.y()));
 
   return ret;
 }
@@ -361,20 +364,18 @@ v8::Handle<v8::Value> Window::GetPosition(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::SetTitle(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1 || !args[0]->IsString())
+  std::string title;
+  if (!FromV8Arguments(args, &title))
     return node::ThrowTypeError("Bad argument");
-  self->window_->SetTitle(*v8::String::Utf8Value(args[0]));
 
+  self->window_->SetTitle(title);
   return v8::Undefined();
 }
 
 // static
 v8::Handle<v8::Value> Window::GetTitle(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
-
-  std::string title = self->window_->GetTitle();
-
-  return v8::String::New(title.c_str(), title.size());
+  return ToV8Value(self->window_->GetTitle());
 }
 
 // static
@@ -391,10 +392,11 @@ v8::Handle<v8::Value> Window::FlashFrame(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::SetKiosk(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1 || !args[0]->IsBoolean())
+  bool kiosk;
+  if (!FromV8Arguments(args, &kiosk))
     return node::ThrowTypeError("Bad argument");
-  self->window_->SetKiosk(args[0]->BooleanValue());
 
+  self->window_->SetKiosk(kiosk);
   return v8::Undefined();
 }
 
@@ -402,7 +404,7 @@ v8::Handle<v8::Value> Window::SetKiosk(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::IsKiosk(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Boolean::New(self->window_->IsKiosk());
+  return ToV8Value(self->window_->IsKiosk());
 }
 
 // static
@@ -427,9 +429,11 @@ v8::Handle<v8::Value> Window::CloseDevTools(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::InspectElement(const v8::Arguments& args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  self->window_->InspectElement(args[0]->IntegerValue(),
-                                args[1]->IntegerValue());
+  int x, y;
+  if (!FromV8Arguments(args, &x, &y))
+    return node::ThrowTypeError("Bad argument");
 
+  self->window_->InspectElement(x, y);
   return v8::Undefined();
 }
 
@@ -454,7 +458,7 @@ v8::Handle<v8::Value> Window::BlurWebView(const v8::Arguments &args) {
 // static
 v8::Handle<v8::Value> Window::IsWebViewFocused(const v8::Arguments& args) {
   UNWRAP_WINDOW_AND_CHECK;
-  return v8::Boolean::New(self->window_->IsWebViewFocused());
+  return ToV8Value(self->window_->IsWebViewFocused());
 }
 
 // static
@@ -471,24 +475,21 @@ v8::Handle<v8::Value> Window::RestartHangMonitorTimeout(
 v8::Handle<v8::Value> Window::GetPageTitle(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  string16 title = self->window_->GetWebContents()->GetTitle();
-
-  return UTF16ToV8String(title);
+  return ToV8Value(self->window_->GetWebContents()->GetTitle());
 }
 
 // static
 v8::Handle<v8::Value> Window::IsLoading(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Boolean::New(self->window_->GetWebContents()->IsLoading());
+  return ToV8Value(self->window_->GetWebContents()->IsLoading());
 }
 
 // static
 v8::Handle<v8::Value> Window::IsWaitingForResponse(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Boolean::New(
-      self->window_->GetWebContents()->IsWaitingForResponse());
+  return ToV8Value(self->window_->GetWebContents()->IsWaitingForResponse());
 }
 
 // static
@@ -504,14 +505,14 @@ v8::Handle<v8::Value> Window::Stop(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::GetRoutingID(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Integer::New(self->window_->GetWebContents()->GetRoutingID());
+  return ToV8Value(self->window_->GetWebContents()->GetRoutingID());
 }
 
 // static
 v8::Handle<v8::Value> Window::GetProcessID(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Integer::New(
+  return ToV8Value(
       self->window_->GetWebContents()->GetRenderProcessHost()->GetID());
 }
 
@@ -519,19 +520,20 @@ v8::Handle<v8::Value> Window::GetProcessID(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::IsCrashed(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  return v8::Boolean::New(self->window_->GetWebContents()->IsCrashed());
+  return ToV8Value(self->window_->GetWebContents()->IsCrashed());
 }
 
 // static
 v8::Handle<v8::Value> Window::LoadURL(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1 || !args[0]->IsString())
+  std::string url;
+  if (!FromV8Arguments(args, &url))
     return node::ThrowTypeError("Bad argument");
 
   NavigationController& controller =
       self->window_->GetWebContents()->GetController();
-  controller.LoadURL(GURL(*v8::String::Utf8Value(args[0])),
+  controller.LoadURL(GURL(url),
                      content::Referrer(),
                      content::PAGE_TRANSITION_AUTO_TOPLEVEL,
                      std::string());
@@ -549,7 +551,7 @@ v8::Handle<v8::Value> Window::GetURL(const v8::Arguments &args) {
   if (controller.GetActiveEntry())
     url = controller.GetActiveEntry()->GetVirtualURL().spec();
 
-  return v8::String::New(url.c_str(), url.size());
+  return ToV8Value(url);
 }
 
 // static
@@ -559,7 +561,7 @@ v8::Handle<v8::Value> Window::CanGoBack(const v8::Arguments &args) {
   NavigationController& controller =
       self->window_->GetWebContents()->GetController();
 
-  return v8::Boolean::New(controller.CanGoBack());
+  return ToV8Value(controller.CanGoBack());
 }
 
 // static
@@ -569,21 +571,21 @@ v8::Handle<v8::Value> Window::CanGoForward(const v8::Arguments &args) {
   NavigationController& controller =
       self->window_->GetWebContents()->GetController();
 
-  return v8::Boolean::New(controller.CanGoForward());
+  return ToV8Value(controller.CanGoForward());
 }
 
 // static
 v8::Handle<v8::Value> Window::CanGoToOffset(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1)
+  int offset;
+  if (!FromV8Arguments(args, &offset))
     return node::ThrowTypeError("Bad argument");
 
   NavigationController& controller =
       self->window_->GetWebContents()->GetController();
-  int offset = args[0]->IntegerValue();
 
-  return v8::Boolean::New(controller.CanGoToOffset(offset));
+  return ToV8Value(controller.CanGoToOffset(offset));
 }
 
 // static
@@ -612,12 +614,13 @@ v8::Handle<v8::Value> Window::GoForward(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::GoToIndex(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1)
+  int index;
+  if (!FromV8Arguments(args, &index))
     return node::ThrowTypeError("Bad argument");
 
   NavigationController& controller =
       self->window_->GetWebContents()->GetController();
-  controller.GoToIndex(args[0]->IntegerValue());
+  controller.GoToIndex(index);
 
   return v8::Undefined();
 }
@@ -626,12 +629,13 @@ v8::Handle<v8::Value> Window::GoToIndex(const v8::Arguments &args) {
 v8::Handle<v8::Value> Window::GoToOffset(const v8::Arguments &args) {
   UNWRAP_WINDOW_AND_CHECK;
 
-  if (args.Length() < 1)
+  int offset;
+  if (!FromV8Arguments(args, &offset))
     return node::ThrowTypeError("Bad argument");
 
   NavigationController& controller =
       self->window_->GetWebContents()->GetController();
-  controller.GoToOffset(args[0]->IntegerValue());
+  controller.GoToOffset(offset);
 
   return v8::Undefined();
 }
@@ -673,6 +677,7 @@ void Window::Initialize(v8::Handle<v8::Object> target) {
   NODE_SET_PROTOTYPE_METHOD(t, "isFocused", IsFocused);
   NODE_SET_PROTOTYPE_METHOD(t, "show", Show);
   NODE_SET_PROTOTYPE_METHOD(t, "hide", Hide);
+  NODE_SET_PROTOTYPE_METHOD(t, "isVisible", IsVisible);
   NODE_SET_PROTOTYPE_METHOD(t, "maximize", Maximize);
   NODE_SET_PROTOTYPE_METHOD(t, "unmaximize", Unmaximize);
   NODE_SET_PROTOTYPE_METHOD(t, "minimize", Minimize);

browser/api/atom_api_window.h

@@ -53,6 +53,7 @@ class Window : public EventEmitter,
   static v8::Handle<v8::Value> IsFocused(const v8::Arguments &args);
   static v8::Handle<v8::Value> Show(const v8::Arguments &args);
   static v8::Handle<v8::Value> Hide(const v8::Arguments &args);
+  static v8::Handle<v8::Value> IsVisible(const v8::Arguments &args);
   static v8::Handle<v8::Value> Maximize(const v8::Arguments &args);
   static v8::Handle<v8::Value> Unmaximize(const v8::Arguments &args);
   static v8::Handle<v8::Value> Minimize(const v8::Arguments &args);

browser/api/atom_browser_bindings.cc

@@ -8,6 +8,8 @@
 
 #include "base/logging.h"
 #include "base/values.h"
+#include "browser/api/atom_api_event.h"
+#include "common/v8_conversions.h"
 #include "common/v8_value_converter_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "vendor/node/src/node.h"
@@ -42,7 +44,7 @@ void AtomBrowserBindings::AfterLoad() {
 
 void AtomBrowserBindings::OnRendererMessage(int process_id,
                                             int routing_id,
-                                            const std::string& channel,
+                                            const string16& channel,
                                             const base::ListValue& args) {
   v8::HandleScope scope;
 
@@ -53,7 +55,7 @@ void AtomBrowserBindings::OnRendererMessage(int process_id,
   // process.emit(channel, 'message', process_id, routing_id);
   std::vector<v8::Handle<v8::Value>> arguments;
   arguments.reserve(3 + args.GetSize());
-  arguments.push_back(v8::String::New(channel.c_str(), channel.size()));
+  arguments.push_back(ToV8Value(channel));
   const base::Value* value;
   if (args.Get(0, &value))
     arguments.push_back(converter->ToV8Value(value, context));
@@ -72,21 +74,24 @@ void AtomBrowserBindings::OnRendererMessage(int process_id,
 void AtomBrowserBindings::OnRendererMessageSync(
     int process_id,
     int routing_id,
-    const std::string& channel,
+    const string16& channel,
     const base::ListValue& args,
-    base::DictionaryValue* result) {
+    NativeWindow* sender,
+    IPC::Message* message) {
   v8::HandleScope scope;
 
   v8::Handle<v8::Context> context = v8::Context::GetCurrent();
 
   scoped_ptr<V8ValueConverter> converter(new V8ValueConverterImpl());
 
-  v8::Handle<v8::Object> event = v8::Object::New();
+  // Create the event object.
+  v8::Handle<v8::Object> event = api::Event::CreateV8Object();
+  api::Event::Unwrap<api::Event>(event)->SetSenderAndMessage(sender, message);
 
   // process.emit(channel, 'sync-message', event, process_id, routing_id);
   std::vector<v8::Handle<v8::Value>> arguments;
   arguments.reserve(3 + args.GetSize());
-  arguments.push_back(v8::String::New(channel.c_str(), channel.size()));
+  arguments.push_back(ToV8Value(channel));
   const base::Value* value;
   if (args.Get(0, &value))
     arguments.push_back(converter->ToV8Value(value, context));
@@ -101,11 +106,6 @@ void AtomBrowserBindings::OnRendererMessageSync(
   }
 
   node::MakeCallback(node::process, "emit", arguments.size(), &arguments[0]);
-
-  scoped_ptr<base::Value> base_event(converter->FromV8Value(event, context));
-  DCHECK(base_event && base_event->IsType(base::Value::TYPE_DICTIONARY));
-
-  result->Swap(static_cast<base::DictionaryValue*>(base_event.get()));
 }
 
 }  // namespace atom

browser/api/atom_browser_bindings.h

@@ -5,17 +5,21 @@
 #ifndef ATOM_BROWSER_API_ATOM_BROWSER_BINDINGS_
 #define ATOM_BROWSER_API_ATOM_BROWSER_BINDINGS_
 
-#include <string>
-
+#include "base/string16.h"
 #include "common/api/atom_bindings.h"
 
 namespace base {
-class DictionaryValue;
 class ListValue;
 }
 
+namespace IPC {
+class Message;
+}
+
 namespace atom {
 
+class NativeWindow;
+
 class AtomBrowserBindings : public AtomBindings {
  public:
   AtomBrowserBindings();
@@ -27,15 +31,16 @@ class AtomBrowserBindings : public AtomBindings {
   // Called when received a message from renderer.
   void OnRendererMessage(int process_id,
                          int routing_id,
-                         const std::string& channel,
+                         const string16& channel,
                          const base::ListValue& args);
 
   // Called when received a synchronous message from renderer.
   void OnRendererMessageSync(int process_id,
                              int routing_id,
-                             const std::string& channel,
+                             const string16& channel,
                              const base::ListValue& args,
-                             base::DictionaryValue* result);
+                             NativeWindow* sender,
+                             IPC::Message* message);
 
   // The require('atom').browserMainParts object.
   v8::Handle<v8::Object> browser_main_parts() {

browser/api/lib/dialog.coffee

@@ -1,4 +1,5 @@
 binding = process.atomBinding 'dialog'
+v8Util = process.atomBinding 'v8_util'
 BrowserWindow = require 'browser-window'
 
 fileDialogProperties =
@@ -7,48 +8,71 @@ fileDialogProperties =
 messageBoxTypes = ['none', 'info', 'warning']
 
 module.exports =
-  showOpenDialog: (options) ->
-    options = title: 'Open', properties: ['openFile'] unless options?
-    options.properties = options.properties ? ['openFile']
+  showOpenDialog: (window, options, callback) ->
+    unless window?.constructor is BrowserWindow
+      # Shift.
+      callback = options
+      options = window
+      window = null
+
+    options ?= title: 'Open', properties: ['openFile']
+    options.properties ?= ['openFile']
     throw new TypeError('Properties need to be array') unless Array.isArray options.properties
 
     properties = 0
     for prop, value of fileDialogProperties
       properties |= value if prop in options.properties
 
-    options.title = options.title ? ''
-    options.defaultPath = options.defaultPath ? ''
+    options.title ?= ''
+    options.defaultPath ?= ''
 
-    binding.showOpenDialog options.title, options.defaultPath, properties
+    binding.showOpenDialog String(options.title),
+                           String(options.defaultPath),
+                           properties,
+                           window,
+                           callback
 
-  showSaveDialog: (window, options) ->
-    throw new TypeError('Invalid window') unless window?.constructor is BrowserWindow
-    options = title: 'Save' unless options?
-
-    options.title = options.title ? ''
-    options.defaultPath = options.defaultPath ? ''
-
-    binding.showSaveDialog window, options.title, options.defaultPath
-
-  showMessageBox: (window, options) ->
-    if window? and window.constructor isnt BrowserWindow
+  showSaveDialog: (window, options, callback) ->
+    unless window?.constructor is BrowserWindow
+      # Shift.
+      callback = options
       options = window
       window = null
 
-    options = type: 'none' unless options?
-    options.type = options.type ? 'none'
+    options ?= title: 'Save'
+    options.title ?= ''
+    options.defaultPath ?= ''
+
+    binding.showSaveDialog String(options.title),
+                           String(options.defaultPath),
+                           window,
+                           callback
+
+  showMessageBox: (window, options, callback) ->
+    unless window?.constructor is BrowserWindow
+      # Shift.
+      callback = options
+      options = window
+      window = null
+
+    options ?= type: 'none'
+    options.type ?= 'none'
     options.type = messageBoxTypes.indexOf options.type
     throw new TypeError('Invalid message box type') unless options.type > -1
 
     throw new TypeError('Buttons need to be array') unless Array.isArray options.buttons
 
-    options.title = options.title ? ''
-    options.message = options.message ? ''
-    options.detail = options.detail ? ''
+    options.title ?= ''
+    options.message ?= ''
+    options.detail ?= ''
 
     binding.showMessageBox options.type,
                            options.buttons,
                            String(options.title),
                            String(options.message),
                            String(options.detail),
-                           window
+                           window,
+                           callback
+
+# Mark standard asynchronous functions.
+v8Util.setHiddenValue f, 'asynchronous', true for k, f of module.exports

browser/api/lib/ipc.coffee

@@ -14,8 +14,13 @@ class Ipc extends EventEmitter
   constructor: ->
     process.on 'ATOM_INTERNAL_MESSAGE', (args...) =>
       @emit(args...)
-    process.on 'ATOM_INTERNAL_MESSAGE_SYNC', (args...) =>
-      @emit(args...)
+    process.on 'ATOM_INTERNAL_MESSAGE_SYNC', (channel, event, args...) =>
+      set = (value) -> event.sendReply JSON.stringify(value)
+
+      Object.defineProperty event, 'returnValue', {set}
+      Object.defineProperty event, 'result', {set}
+
+      @emit(channel, event, args...)
 
   send: (processId, routingId, args...) ->
     @sendChannel(processId, routingId, 'message', args...)

browser/atom/atom.coffee

@@ -26,7 +26,7 @@ atom.browserMainParts =
 global.__atom = atom
 
 # Add browser/api/lib to require's search paths,
-# which contains javascript part of Atom's built-in libraries.  
+# which contains javascript part of Atom's built-in libraries.
 globalPaths = require('module').globalPaths
 globalPaths.push path.join process.resourcesPath, 'browser', 'api', 'lib'
 
@@ -38,11 +38,11 @@ process.on 'uncaughtException', (error) ->
   # Show error in GUI.
   message = error.stack ? "#{error.name}: #{error.message}"
   require('dialog').showMessageBox
-      type: 'warning'
-      title: 'An javascript error occured in the browser'
-      message: 'uncaughtException'
-      detail: message
-      buttons: ['OK']
+    type: 'warning'
+    title: 'An javascript error occured in the browser'
+    message: 'uncaughtException'
+    detail: message
+    buttons: ['OK']
 
 # Load the RPC server.
 require './rpc-server.js'

browser/atom/rpc-server.coffee

@@ -61,28 +61,40 @@ unwrapArgs = (processId, routingId, args) ->
 
   args.map metaToValue
 
+# Call a function and send reply asynchronously if it's a an asynchronous
+# style function and the caller didn't pass a callback.
+callFunction = (event, processId, routingId, func, caller, args) ->
+  if v8Util.getHiddenValue(func, 'asynchronous') and typeof args[args.length - 1] isnt 'function'
+    args.push (ret) ->
+      event.returnValue = valueToMeta processId, routingId, ret
+    func.apply caller, args
+  else
+    ret = func.apply caller, args
+    event.returnValue = valueToMeta processId, routingId, ret
+
 ipc.on 'ATOM_BROWSER_REQUIRE', (event, processId, routingId, module) ->
   try
-    event.result = valueToMeta processId, routingId, require(module)
+    event.returnValue = valueToMeta processId, routingId, require(module)
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_GLOBAL', (event, processId, routingId, name) ->
   try
-    event.result = valueToMeta processId, routingId, global[name]
+    event.returnValue = valueToMeta processId, routingId, global[name]
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_RELEASE_RENDER_VIEW', (event, processId, routingId) ->
   objectsRegistry.clear processId, routingId
+  event.returnValue = null
 
 ipc.on 'ATOM_BROWSER_CURRENT_WINDOW', (event, processId, routingId) ->
   try
     BrowserWindow = require 'browser-window'
     window = BrowserWindow.fromProcessIdAndRoutingId processId, routingId
-    event.result = valueToMeta processId, routingId, window
+    event.returnValue = valueToMeta processId, routingId, window
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_CONSTRUCTOR', (event, processId, routingId, id, args) ->
   try
@@ -91,18 +103,17 @@ ipc.on 'ATOM_BROWSER_CONSTRUCTOR', (event, processId, routingId, id, args) ->
     # Call new with array of arguments.
     # http://stackoverflow.com/questions/1606797/use-of-apply-with-new-operator-is-this-possible
     obj = new (Function::bind.apply(constructor, [null].concat(args)))
-    event.result = valueToMeta processId, routingId, obj
+    event.returnValue = valueToMeta processId, routingId, obj
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_FUNCTION_CALL', (event, processId, routingId, id, args) ->
   try
     args = unwrapArgs processId, routingId, args
     func = objectsRegistry.get id
-    ret = func.apply global, args
-    event.result = valueToMeta processId, routingId, ret
+    callFunction event, processId, routingId, func, global, args
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_MEMBER_CONSTRUCTOR', (event, processId, routingId, id, method, args) ->
   try
@@ -110,32 +121,32 @@ ipc.on 'ATOM_BROWSER_MEMBER_CONSTRUCTOR', (event, processId, routingId, id, meth
     constructor = objectsRegistry.get(id)[method]
     # Call new with array of arguments.
     obj = new (Function::bind.apply(constructor, [null].concat(args)))
-    event.result = valueToMeta processId, routingId, obj
+    event.returnValue = valueToMeta processId, routingId, obj
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_MEMBER_CALL', (event, processId, routingId, id, method, args) ->
   try
     args = unwrapArgs processId, routingId, args
     obj = objectsRegistry.get id
-    ret = obj[method].apply(obj, args)
-    event.result = valueToMeta processId, routingId, ret
+    callFunction event, processId, routingId, obj[method], obj, args
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_MEMBER_SET', (event, processId, routingId, id, name, value) ->
   try
     obj = objectsRegistry.get id
     obj[name] = value
+    event.returnValue = null
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_MEMBER_GET', (event, processId, routingId, id, name) ->
   try
     obj = objectsRegistry.get id
-    event.result = valueToMeta processId, routingId, obj[name]
+    event.returnValue = valueToMeta processId, routingId, obj[name]
   catch e
-    event.result = errorToMeta e
+    event.returnValue = errorToMeta e
 
 ipc.on 'ATOM_BROWSER_DEREFERENCE', (processId, routingId, storeId) ->
   objectsRegistry.remove processId, routingId, storeId

browser/atom_browser_main_parts.cc

@@ -10,6 +10,7 @@
 #include "browser/atom_browser_context.h"
 #include "browser/browser.h"
 #include "common/node_bindings.h"
+#include "net/proxy/proxy_resolver_v8.h"
 #include "vendor/node/src/node.h"
 #include "vendor/node/src/node_internals.h"
 
@@ -75,6 +76,11 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
 
   node_bindings_->RunMessageLoop();
 
+  // Make sure the url request job factory is created before the
+  // will-finish-launching event.
+  static_cast<content::BrowserContext*>(AtomBrowserContext::Get())->
+      GetRequestContext();
+
 #if !defined(OS_MACOSX)
   // The corresponding call in OS X is in AtomApplicationDelegate.
   Browser::Get()->WillFinishLaunching();
@@ -82,4 +88,10 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
 #endif
 }
 
+int AtomBrowserMainParts::PreCreateThreads() {
+  // TODO(zcbenz): Calling CreateIsolate() on Windows when updated to Chrome30.
+  net::ProxyResolverV8::RememberDefaultIsolate();
+  return 0;
+}
+
 }  // namespace atom

browser/atom_browser_main_parts.h

@@ -30,6 +30,7 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
   // Implementations of content::BrowserMainParts.
   virtual void PostEarlyInitialization() OVERRIDE;
   virtual void PreMainMessageLoopRun() OVERRIDE;
+  virtual int PreCreateThreads() OVERRIDE;
 #if defined(OS_MACOSX)
   virtual void PreMainMessageLoopStart() OVERRIDE;
   virtual void PostDestroyThreads() OVERRIDE;

browser/native_window.cc

@@ -300,7 +300,8 @@ bool NativeWindow::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(NativeWindow, message)
     IPC_MESSAGE_HANDLER(AtomViewHostMsg_Message, OnRendererMessage)
-    IPC_MESSAGE_HANDLER(AtomViewHostMsg_Message_Sync, OnRendererMessageSync)
+    IPC_MESSAGE_HANDLER_DELAY_REPLY(AtomViewHostMsg_Message_Sync,
+                                    OnRendererMessageSync)
     IPC_MESSAGE_HANDLER(AtomViewHostMsg_UpdateDraggableRegions,
                         UpdateDraggableRegions)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -340,7 +341,7 @@ void NativeWindow::Observe(int type,
   }
 }
 
-void NativeWindow::OnRendererMessage(const std::string& channel,
+void NativeWindow::OnRendererMessage(const string16& channel,
                                      const base::ListValue& args) {
   AtomBrowserMainParts::Get()->atom_bindings()->OnRendererMessage(
       GetWebContents()->GetRenderProcessHost()->GetID(),
@@ -349,15 +350,16 @@ void NativeWindow::OnRendererMessage(const std::string& channel,
       args);
 }
 
-void NativeWindow::OnRendererMessageSync(const std::string& channel,
+void NativeWindow::OnRendererMessageSync(const string16& channel,
                                          const base::ListValue& args,
-                                         base::DictionaryValue* result) {
+                                         IPC::Message* reply_msg) {
   AtomBrowserMainParts::Get()->atom_bindings()->OnRendererMessageSync(
       GetWebContents()->GetRenderProcessHost()->GetID(),
       GetWebContents()->GetRoutingID(),
       channel,
       args,
-      result);
+      this,
+      reply_msg);
 }
 
 }  // namespace atom

browser/native_window.h

@@ -35,6 +35,10 @@ class Rect;
 class Size;
 }
 
+namespace IPC {
+class Message;
+}
+
 namespace atom {
 
 class AtomJavaScriptDialogManager;
@@ -65,6 +69,7 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
   virtual bool IsFocused() = 0;
   virtual void Show() = 0;
   virtual void Hide() = 0;
+  virtual bool IsVisible() = 0;
   virtual void Maximize() = 0;
   virtual void Unmaximize() = 0;
   virtual void Minimize() = 0;
@@ -173,12 +178,12 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate,
  private:
   void RendererUnresponsiveDelayed();
 
-  void OnRendererMessage(const std::string& channel,
+  void OnRendererMessage(const string16& channel,
                          const base::ListValue& args);
 
-  void OnRendererMessageSync(const std::string& channel,
+  void OnRendererMessageSync(const string16& channel,
                              const base::ListValue& args,
-                             base::DictionaryValue* result);
+                             IPC::Message* reply_msg);
 
   // Notification manager.
   content::NotificationRegistrar registrar_;

browser/native_window_mac.h

@@ -28,6 +28,7 @@ class NativeWindowMac : public NativeWindow {
   virtual bool IsFocused() OVERRIDE;
   virtual void Show() OVERRIDE;
   virtual void Hide() OVERRIDE;
+  virtual bool IsVisible() OVERRIDE;
   virtual void Maximize() OVERRIDE;
   virtual void Unmaximize() OVERRIDE;
   virtual void Minimize() OVERRIDE;

browser/native_window_mac.mm

@@ -13,8 +13,7 @@
 #include "base/mac/mac_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/values.h"
-#import "browser/atom_event_processing_window.h"
-#import "browser/ui/cocoa/custom_frame_view.h"
+#import "browser/ui/atom_event_processing_window.h"
 #include "brightray/browser/inspectable_web_contents.h"
 #include "brightray/browser/inspectable_web_contents_view.h"
 #include "common/draggable_region.h"
@@ -158,8 +157,8 @@ NativeWindowMac::NativeWindowMac(content::WebContents* web_contents,
   atomWindow = [[AtomNSWindow alloc]
       initWithContentRect:cocoa_bounds
                 styleMask:NSTitledWindowMask | NSClosableWindowMask |
-                            NSMiniaturizableWindowMask | NSResizableWindowMask |
-                            NSTexturedBackgroundWindowMask
+                          NSMiniaturizableWindowMask | NSResizableWindowMask |
+                          NSTexturedBackgroundWindowMask
                   backing:NSBackingStoreBuffered
                     defer:YES];
 
@@ -210,7 +209,10 @@ void NativeWindowMac::Move(const gfx::Rect& pos) {
 }
 
 void NativeWindowMac::Focus(bool focus) {
-  if (focus && [window() isVisible]) {
+  if (!IsVisible())
+    return;
+
+  if (focus) {
     [[NSApplication sharedApplication] activateIgnoringOtherApps:YES];
     [window() makeKeyAndOrderFront:nil];
   } else {
@@ -230,6 +232,10 @@ void NativeWindowMac::Hide() {
   [window() orderOut:nil];
 }
 
+bool NativeWindowMac::IsVisible() {
+  return [window() isVisible];
+}
+
 void NativeWindowMac::Maximize() {
   [window() zoom:nil];
 }

browser/native_window_win.cc

@@ -231,6 +231,10 @@ void NativeWindowWin::Unmaximize() {
   window_->Restore();
 }
 
+bool NativeWindowWin::IsVisible() {
+  return window_->IsVisible();
+}
+
 void NativeWindowWin::Minimize() {
   window_->Minimize();
 }

browser/native_window_win.h

@@ -34,6 +34,7 @@ class NativeWindowWin : public NativeWindow,
   virtual bool IsFocused() OVERRIDE;
   virtual void Show() OVERRIDE;
   virtual void Hide() OVERRIDE;
+  virtual bool IsVisible() OVERRIDE;
   virtual void Maximize() OVERRIDE;
   virtual void Unmaximize() OVERRIDE;
   virtual void Minimize() OVERRIDE;

browser/net/atom_url_request_context_getter.cc

@@ -15,7 +15,11 @@
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_cache.h"
 #include "net/http/http_server_properties_impl.h"
+#include "net/proxy/dhcp_proxy_script_fetcher_factory.h"
+#include "net/proxy/proxy_config_service.h"
+#include "net/proxy/proxy_script_fetcher_impl.h"
 #include "net/proxy/proxy_service.h"
+#include "net/proxy/proxy_service_v8.h"
 #include "net/ssl/default_server_bound_cert_store.h"
 #include "net/ssl/server_bound_cert_service.h"
 #include "net/ssl/ssl_config_service_defaults.h"
@@ -43,7 +47,12 @@ AtomURLRequestContextGetter::AtomURLRequestContextGetter(
       network_delegate_(network_delegate.Pass()) {
   // Must first be created on the UI thread.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   std::swap(protocol_handlers_, *protocol_handlers);
+
+  // We must create the proxy config service on the UI loop on Linux because it
+  // must synchronously run on the glib message loop. This will be passed to
+  // the URLRequestContextStorage on the IO thread in GetURLRequestContext().
   proxy_config_service_.reset(
       net::ProxyService::CreateSystemProxyConfigService(
           io_loop_->message_loop_proxy(),
@@ -75,14 +84,17 @@ net::URLRequestContext* AtomURLRequestContextGetter::GetURLRequestContext() {
 
     scoped_ptr<net::HostResolver> host_resolver(
         net::HostResolver::CreateDefaultResolver(NULL));
+    net::DhcpProxyScriptFetcherFactory dhcp_factory;
 
     storage_->set_cert_verifier(net::CertVerifier::CreateDefault());
-    // TODO(jam): use v8 if possible, look at chrome code.
     storage_->set_proxy_service(
-        net::ProxyService::CreateUsingSystemProxyResolver(
-        proxy_config_service_.release(),
-        0,
-        NULL));
+        net::CreateProxyServiceUsingV8ProxyResolver(
+            proxy_config_service_.release(),
+            new net::ProxyScriptFetcherImpl(url_request_context_.get()),
+            dhcp_factory.Create(url_request_context_.get()),
+            host_resolver.get(),
+            NULL,
+            url_request_context_->network_delegate()));
     storage_->set_ssl_config_service(new net::SSLConfigServiceDefaults);
     storage_->set_http_auth_handler_factory(
         net::HttpAuthHandlerFactory::CreateDefault(host_resolver.get()));

browser/ui/atom_event_processing_window.h

@@ -2,18 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef ATOM_BROWSER_ATOM_EVENT_PROCESSING_WINDOW_H_
-#define ATOM_BROWSER_ATOM_EVENT_PROCESSING_WINDOW_H_
+#ifndef ATOM_BROWSER_UI_ATOM_EVENT_PROCESSING_WINDOW_H_
+#define ATOM_BROWSER_UI_ATOM_EVENT_PROCESSING_WINDOW_H_
 
 #import <Cocoa/Cocoa.h>
 
 #include "base/memory/scoped_nsobject.h"
-#import "ui/base/cocoa/underlay_opengl_hosting_window.h"
 
 // Override NSWindow to access unhandled keyboard events (for command
 // processing); subclassing NSWindow is the only method to do
 // this.
-@interface AtomEventProcessingWindow : UnderlayOpenGLHostingWindow {
+@interface AtomEventProcessingWindow : NSWindow {
  @private
   BOOL redispatchingEvent_;
   BOOL eventHandled_;
@@ -30,4 +29,4 @@
 - (BOOL)performKeyEquivalent:(NSEvent*)theEvent;
 @end
 
-#endif  // ATOM_BROWSER_ATOM_EVENT_PROCESSING_WINDOW_H_
+#endif  // ATOM_BROWSER_UI_ATOM_EVENT_PROCESSING_WINDOW_H_

browser/ui/atom_event_processing_window.mm

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import "browser/atom_event_processing_window.h"
+#import "browser/ui/atom_event_processing_window.h"
 
 #include "base/logging.h"
 #import "content/public/browser/render_widget_host_view_mac_base.h"

browser/ui/cocoa/custom_frame_view.h

@@ -1,56 +0,0 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#import <Cocoa/Cocoa.h>
-
-// CustomFrameView is a class whose methods we swizzle into NSGrayFrame
-// on 10.7 and below, or NSThemeFrame on 10.8 and above, so that we can
-// support custom frame drawing. This is used with a textured window so that
-// AppKit does not draw a title bar.
-// This class is never to be instantiated on its own.
-// We explored a variety of ways to support custom frame drawing and custom
-// window widgets.
-// Our requirements were:
-// a) that we could fall back on standard system drawing at any time for the
-//    "default theme"
-// b) We needed to be able to draw both a background pattern, and an overlay
-//    graphic, and we need to be able to set the pattern phase of our background
-//    window.
-// c) We had to be able to support "transparent" themes, so that you could see
-//    through to the underlying windows in places without the system theme
-//    getting in the way.
-//
-// Since we want "A" we couldn't just do a transparent borderless window. At
-// least I couldn't find the right combination of HITheme calls to make it draw
-// nicely, and I don't trust that the HITheme calls are going to exist in future
-// system versions.
-// "C" precluded us from inserting a view between the system frame and the
-// the content frame in Z order. To get the transparency we actually need to
-// replace the drawing of the system frame.
-// "B" precluded us from just setting a background color on the window.
-//
-// Originally we tried overriding the private API +frameViewForStyleMask: to
-// add our own subclass of NSGrayView to our window. Turns out that if you
-// subclass NSGrayView it does not draw correctly when you call NSGrayView's
-// drawRect. It appears that NSGrayView's drawRect: method (and that of its
-// superclasses) do lots of "isMemberOfClass/isKindOfClass" calls, and if your
-// class is NOT an instance of NSGrayView (as opposed to a subclass of
-// NSGrayView) then the system drawing will not work correctly.
-//
-// Given all of the above, we found swizzling drawRect in _load to be the
-// easiest and safest method of achieving our goals. We do the best we can to
-// check that everything is safe, and attempt to fallback gracefully if it is
-// not.
-
-@interface NSWindow (CustomFrameView)
-
-// To define custom window drawing behaviour, override this method on an
-// NSWindow subclass. Call the default method (on super) to draw the
-// default frame.
-// NOTE: Always call the default implementation (even if you just immediately
-// draw over it), otherwise the top-left and top-right corners of the window
-// won't be drawn correctly.
-- (void)drawCustomFrameRect:(NSRect)rect forView:(NSView*)view;
-
-@end

browser/ui/cocoa/custom_frame_view.mm

@@ -1,138 +0,0 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#import "browser/ui/cocoa/custom_frame_view.h"
-
-#import <objc/runtime.h>
-#import <Carbon/Carbon.h>
-
-#include "base/logging.h"
-#include "base/mac/mac_util.h"
-#include "base/mac/scoped_nsautorelease_pool.h"
-
-namespace {
-BOOL gCanDrawTitle = NO;
-BOOL gCanGetCornerRadius = NO;
-}  // namespace
-
-@interface NSView (Swizzles)
-- (void)drawRectOriginal:(NSRect)rect;
-- (NSPoint)_fullScreenButtonOriginOriginal;
-@end
-
-@interface NSWindow (FramedBrowserWindow)
-- (NSPoint)fullScreenButtonOriginAdjustment;
-@end
-
-@implementation NSWindow (CustomFrameView)
-- (void)drawCustomFrameRect:(NSRect)rect forView:(NSView*)view {
-  [view drawRectOriginal:rect];
-}
-@end
-
-@interface CustomFrameView : NSView
-
-@end
-
-@implementation CustomFrameView
-
-// This is where we swizzle drawRect, and add in two methods that we
-// need. If any of these fail it shouldn't affect the functionality of the
-// others. If they all fail, we will lose window frame theming and
-// roll overs for our close widgets, but things should still function
-// correctly.
-+ (void)load {
-  base::mac::ScopedNSAutoreleasePool pool;
-
-  // On 10.8+ the background for textured windows are no longer drawn by
-  // NSGrayFrame, and NSThemeFrame is used instead <http://crbug.com/114745>.
-  Class borderViewClass = NSClassFromString(
-      base::mac::IsOSMountainLionOrLater() ? @"NSThemeFrame" : @"NSGrayFrame");
-  DCHECK(borderViewClass);
-  if (!borderViewClass) return;
-
-  // Exchange draw rect.
-  Method m0 = class_getInstanceMethod([self class], @selector(drawRect:));
-  DCHECK(m0);
-  if (m0) {
-    BOOL didAdd = class_addMethod(borderViewClass,
-                                  @selector(drawRectOriginal:),
-                                  method_getImplementation(m0),
-                                  method_getTypeEncoding(m0));
-    DCHECK(didAdd);
-    if (didAdd) {
-      Method m1 = class_getInstanceMethod(borderViewClass,
-                                          @selector(drawRect:));
-      Method m2 = class_getInstanceMethod(borderViewClass,
-                                          @selector(drawRectOriginal:));
-      DCHECK(m1 && m2);
-      if (m1 && m2) {
-        method_exchangeImplementations(m1, m2);
-      }
-    }
-  }
-
-  // Swizzle the method that sets the origin for the Lion fullscreen button. Do
-  // nothing if it cannot be found.
-  m0 = class_getInstanceMethod([self class],
-                               @selector(_fullScreenButtonOrigin));
-  if (m0) {
-    BOOL didAdd = class_addMethod(borderViewClass,
-                                  @selector(_fullScreenButtonOriginOriginal),
-                                  method_getImplementation(m0),
-                                  method_getTypeEncoding(m0));
-    if (didAdd) {
-      Method m1 = class_getInstanceMethod(borderViewClass,
-                                          @selector(_fullScreenButtonOrigin));
-      Method m2 = class_getInstanceMethod(borderViewClass,
-          @selector(_fullScreenButtonOriginOriginal));
-      if (m1 && m2) {
-        method_exchangeImplementations(m1, m2);
-      }
-    }
-  }
-}
-
-+ (BOOL)canDrawTitle {
-  return gCanDrawTitle;
-}
-
-+ (BOOL)canGetCornerRadius {
-  return gCanGetCornerRadius;
-}
-
-- (id)initWithFrame:(NSRect)frame {
-  // This class is not for instantiating.
-  [self doesNotRecognizeSelector:_cmd];
-  return nil;
-}
-
-- (id)initWithCoder:(NSCoder*)coder {
-  // This class is not for instantiating.
-  [self doesNotRecognizeSelector:_cmd];
-  return nil;
-}
-
-// Here is our custom drawing for our frame.
-- (void)drawRect:(NSRect)rect {
-  // Delegate drawing to the window, whose default implementation (above) is to
-  // call into the original implementation.
-  [[self window] drawCustomFrameRect:rect forView:self];
-}
-
-// Override to move the fullscreen button to the left of the profile avatar.
-- (NSPoint)_fullScreenButtonOrigin {
-  NSWindow* window = [self window];
-  NSPoint offset = NSZeroPoint;
-
-  if ([window respondsToSelector:@selector(fullScreenButtonOriginAdjustment)])
-    offset = [window fullScreenButtonOriginAdjustment];
-
-  NSPoint origin = [self _fullScreenButtonOriginOriginal];
-  origin.x += offset.x;
-  origin.y += offset.y;
-  return origin;
-}
-
-@end

browser/ui/file_dialog.h

@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_forward.h"
 #include "base/files/file_path.h"
 
 namespace atom {
@@ -23,16 +24,34 @@ enum FileDialogProperty {
   FILE_DIALOG_CREATE_DIRECTORY = 8,
 };
 
-bool ShowOpenDialog(const std::string& title,
+typedef base::Callback<void(
+    bool result, const std::vector<base::FilePath>& paths)> OpenDialogCallback;
+
+typedef base::Callback<void(
+    bool result, const base::FilePath& path)> SaveDialogCallback;
+
+bool ShowOpenDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
                     const base::FilePath& default_path,
                     int properties,
                     std::vector<base::FilePath>* paths);
 
-bool ShowSaveDialog(atom::NativeWindow* window,
+void ShowOpenDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
+                    const base::FilePath& default_path,
+                    int properties,
+                    const OpenDialogCallback& callback);
+
+bool ShowSaveDialog(atom::NativeWindow* parent_window,
                     const std::string& title,
                     const base::FilePath& default_path,
                     base::FilePath* path);
 
+void ShowSaveDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
+                    const base::FilePath& default_path,
+                    const SaveDialogCallback& callback);
+
 }  // namespace file_dialog
 
 #endif  // BROWSER_UI_FILE_DIALOG_H_

browser/ui/file_dialog_mac.mm

@@ -38,20 +38,11 @@ void SetupDialog(NSSavePanel* dialog,
   if (default_filename)
     [dialog setNameFieldStringValue:default_filename];
 
+  [dialog setCanSelectHiddenExtension:YES];
   [dialog setAllowsOtherFileTypes:YES];
 }
 
-}  // namespace
-
-bool ShowOpenDialog(const std::string& title,
-                    const base::FilePath& default_path,
-                    int properties,
-                    std::vector<base::FilePath>* paths) {
-  DCHECK(paths);
-  NSOpenPanel* dialog = [NSOpenPanel openPanel];
-
-  SetupDialog(dialog, title, default_path);
-
+void SetupDialogForProperties(NSOpenPanel* dialog, int properties) {
   [dialog setCanChooseFiles:(properties & FILE_DIALOG_OPEN_FILE)];
   if (properties & FILE_DIALOG_OPEN_DIRECTORY)
     [dialog setCanChooseDirectories:YES];
@@ -59,49 +50,119 @@ bool ShowOpenDialog(const std::string& title,
     [dialog setCanCreateDirectories:YES];
   if (properties & FILE_DIALOG_MULTI_SELECTIONS)
     [dialog setAllowsMultipleSelection:YES];
+}
 
-  if ([dialog runModal] == NSFileHandlingPanelCancelButton)
-    return false;
+// Run modal dialog with parent window and return user's choice.
+int RunModalDialog(NSSavePanel* dialog, atom::NativeWindow* parent_window) {
+  __block int chosen = NSFileHandlingPanelCancelButton;
+  if (parent_window == NULL) {
+    chosen = [dialog runModal];
+  } else {
+    NSWindow* window = parent_window->GetNativeWindow();
 
+    [dialog beginSheetModalForWindow:window
+                   completionHandler:^(NSInteger c) {
+      chosen = c;
+      [NSApp stopModal];
+    }];
+    [NSApp runModalForWindow:window];
+  }
+
+  return chosen;
+}
+
+void ReadDialogPaths(NSOpenPanel* dialog, std::vector<base::FilePath>* paths) {
   NSArray* urls = [dialog URLs];
   for (NSURL* url in urls)
     if ([url isFileURL])
       paths->push_back(base::FilePath(base::SysNSStringToUTF8([url path])));
+}
 
+}  // namespace
+
+bool ShowOpenDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
+                    const base::FilePath& default_path,
+                    int properties,
+                    std::vector<base::FilePath>* paths) {
+  DCHECK(paths);
+  NSOpenPanel* dialog = [NSOpenPanel openPanel];
+
+  SetupDialog(dialog, title, default_path);
+  SetupDialogForProperties(dialog, properties);
+
+  int chosen = RunModalDialog(dialog, parent_window);
+  if (chosen == NSFileHandlingPanelCancelButton)
+    return false;
+
+  ReadDialogPaths(dialog, paths);
   return true;
 }
 
-bool ShowSaveDialog(atom::NativeWindow* window,
+void ShowOpenDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
+                    const base::FilePath& default_path,
+                    int properties,
+                    const OpenDialogCallback& c) {
+  NSOpenPanel* dialog = [NSOpenPanel openPanel];
+
+  SetupDialog(dialog, title, default_path);
+  SetupDialogForProperties(dialog, properties);
+
+  // Duplicate the callback object here since c is a reference and gcd would
+  // only store the pointer, by duplication we can force gcd to store a copy.
+  __block OpenDialogCallback callback = c;
+
+  NSWindow* window = parent_window ? parent_window->GetNativeWindow() : NULL;
+  [dialog beginSheetModalForWindow:window
+                 completionHandler:^(NSInteger chosen) {
+    if (chosen == NSFileHandlingPanelCancelButton) {
+      callback.Run(false, std::vector<base::FilePath>());
+    } else {
+      std::vector<base::FilePath> paths;
+      ReadDialogPaths(dialog, &paths);
+      callback.Run(true, paths);
+    }
+  }];
+}
+
+bool ShowSaveDialog(atom::NativeWindow* parent_window,
                     const std::string& title,
                     const base::FilePath& default_path,
                     base::FilePath* path) {
-  DCHECK(window);
   DCHECK(path);
   NSSavePanel* dialog = [NSSavePanel savePanel];
 
   SetupDialog(dialog, title, default_path);
 
-  [dialog setCanSelectHiddenExtension:YES];
+  int chosen = RunModalDialog(dialog, parent_window);
+  if (chosen == NSFileHandlingPanelCancelButton || ![[dialog URL] isFileURL])
+    return false;
 
-  __block bool result = false;
-  __block base::FilePath ret_path;
-  [dialog beginSheetModalForWindow:window->GetNativeWindow()
+  *path = base::FilePath(base::SysNSStringToUTF8([[dialog URL] path]));
+  return true;
+}
+
+void ShowSaveDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
+                    const base::FilePath& default_path,
+                    const SaveDialogCallback& c) {
+  NSSavePanel* dialog = [NSSavePanel savePanel];
+
+  SetupDialog(dialog, title, default_path);
+
+  __block SaveDialogCallback callback = c;
+
+  NSWindow* window = parent_window ? parent_window->GetNativeWindow() : NULL;
+  [dialog beginSheetModalForWindow:window
                  completionHandler:^(NSInteger chosen) {
-    if (chosen == NSFileHandlingPanelCancelButton ||
-        ![[dialog URL] isFileURL]) {
-      result = false;
+    if (chosen == NSFileHandlingPanelCancelButton) {
+      callback.Run(false, base::FilePath());
     } else {
-      result = true;
-      ret_path = base::FilePath(base::SysNSStringToUTF8([[dialog URL] path]));
+      std::string path = base::SysNSStringToUTF8([[dialog URL] path]);
+      callback.Run(true, base::FilePath(path));
     }
-
-    [NSApp stopModal];
   }];
-
-  [NSApp runModalForWindow:window->GetNativeWindow()];
-
-  *path = ret_path;
-  return result;
 }
 
 }  // namespace file_dialog

browser/ui/file_dialog_win.cc

@@ -4,6 +4,7 @@
 
 #include "browser/ui/file_dialog.h"
 
+#include <atlbase.h>
 #include <windows.h>
 #include <commdlg.h>
 #include <shlobj.h>
@@ -14,19 +15,14 @@
 #include "base/strings/string_split.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/registry.h"
-#include "base/win/windows_version.h"
 #include "browser/native_window.h"
+#include "third_party/wtl/include/atlapp.h"
+#include "third_party/wtl/include/atldlgs.h"
 
 namespace file_dialog {
 
 namespace {
 
-// Given |extension|, if it's not empty, then remove the leading dot.
-std::wstring GetExtensionWithoutLeadingDot(const std::wstring& extension) {
-  DCHECK(extension.empty() || extension[0] == L'.');
-  return extension.empty() ? extension : extension.substr(1);
-}
-
 // Distinguish directories from regular files.
 bool IsDirectory(const base::FilePath& path) {
   base::PlatformFileInfo file_info;
@@ -67,26 +63,21 @@ static bool GetRegistryDescriptionFromExtension(const std::wstring& file_ext,
 // If a description is not provided for a file extension, it will be retrieved
 // from the registry. If the file extension does not exist in the registry, it
 // will be omitted from the filter, as it is likely a bogus extension.
-std::wstring FormatFilterForExtensions(
-    const std::vector<std::wstring>& file_ext,
-    const std::vector<std::wstring>& ext_desc,
-    bool include_all_files) {
-  const std::wstring all_ext = L"*.*";
-  // TODO(zcbenz): Should be localized.
-  const std::wstring all_desc = L"All Files";
+void FormatFilterForExtensions(
+    std::vector<std::wstring>* file_ext,
+    std::vector<std::wstring>* ext_desc,
+    bool include_all_files,
+    std::vector<COMDLG_FILTERSPEC>* file_types) {
+  DCHECK(file_ext->size() >= ext_desc->size());
 
-  DCHECK(file_ext.size() >= ext_desc.size());
-
-  if (file_ext.empty())
+  if (file_ext->empty())
     include_all_files = true;
 
-  std::wstring result;
-
-  for (size_t i = 0; i < file_ext.size(); ++i) {
-    std::wstring ext = file_ext[i];
+  for (size_t i = 0; i < file_ext->size(); ++i) {
+    std::wstring ext = (*file_ext)[i];
     std::wstring desc;
-    if (i < ext_desc.size())
-      desc = ext_desc[i];
+    if (i < ext_desc->size())
+      desc = (*ext_desc)[i];
 
     if (ext.empty()) {
       // Force something reasonable to appear in the dialog box if there is no
@@ -114,231 +105,214 @@ std::wstring FormatFilterForExtensions(
         // the we create a description "QQQ File (.qqq)").
         include_all_files = true;
         // TODO(zcbenz): should be localized.
-        desc = base::i18n::ToUpper(WideToUTF16(ext_name)) + L" File (."
-                                                          + ext_name
-                                                          + L")";
+        desc = base::i18n::ToUpper(WideToUTF16(ext_name)) + L" File";
       }
-      if (desc.empty())
-        desc = L"*." + ext_name;
+      desc += L" (*." + ext_name + L")";
+
+      // Store the description.
+      ext_desc->push_back(desc);
     }
 
-    result.append(desc.c_str(), desc.size() + 1);  // Append NULL too.
-    result.append(ext.c_str(), ext.size() + 1);
+    COMDLG_FILTERSPEC spec = { (*ext_desc)[i].c_str(), (*file_ext)[i].c_str() };
+    file_types->push_back(spec);
   }
 
   if (include_all_files) {
-    result.append(all_desc.c_str(), all_desc.size() + 1);
-    result.append(all_ext.c_str(), all_ext.size() + 1);
-  }
+    // TODO(zcbenz): Should be localized.
+    ext_desc->push_back(L"All Files (*.*)");
+    file_ext->push_back(L"*.*");
 
-  result.append(1, '\0');  // Double NULL required.
-  return result;
+    COMDLG_FILTERSPEC spec = {
+      (*ext_desc)[ext_desc->size() - 1].c_str(),
+      (*file_ext)[file_ext->size() - 1].c_str(),
+    };
+    file_types->push_back(spec);
+  }
 }
 
-// This function takes the output of a SaveAs dialog: a filename, a filter and
-// the extension originally suggested to the user (shown in the dialog box) and
-// returns back the filename with the appropriate extension tacked on. If the
-// user requests an unknown extension and is not using the 'All files' filter,
-// the suggested extension will be appended, otherwise we will leave the
-// filename unmodified. |filename| should contain the filename selected in the
-// SaveAs dialog box and may include the path, |filter_selected| should be
-// '*.something', for example '*.*' or it can be blank (which is treated as
-// *.*). |suggested_ext| should contain the extension without the dot (.) in
-// front, for example 'jpg'.
-std::wstring AppendExtensionIfNeeded(
-    const std::wstring& filename,
-    const std::wstring& filter_selected,
-    const std::wstring& suggested_ext) {
-  DCHECK(!filename.empty());
-  std::wstring return_value = filename;
+// Generic class to delegate common open/save dialog's behaviours, users need to
+// call interface methods via GetPtr().
+template <typename T>
+class FileDialog {
+ public:
+  FileDialog(const base::FilePath& default_path,
+             const std::string title,
+             int options,
+             const std::vector<std::wstring>& file_ext,
+             const std::vector<std::wstring>& desc_ext)
+      : file_ext_(file_ext),
+        desc_ext_(desc_ext) {
+    std::vector<COMDLG_FILTERSPEC> filters;
+    FormatFilterForExtensions(&file_ext_, &desc_ext_, true, &filters);
 
-  // If we wanted a specific extension, but the user's filename deleted it or
-  // changed it to something that the system doesn't understand, re-append.
-  // Careful: Checking net::GetMimeTypeFromExtension() will only find
-  // extensions with a known MIME type, which many "known" extensions on Windows
-  // don't have.  So we check directly for the "known extension" registry key.
-  std::wstring file_extension(
-      GetExtensionWithoutLeadingDot(base::FilePath(filename).Extension()));
-  std::wstring key(L"." + file_extension);
-  if (!(filter_selected.empty() || filter_selected == L"*.*") &&
-      !base::win::RegKey(HKEY_CLASSES_ROOT, key.c_str(), KEY_READ).Valid() &&
-      file_extension != suggested_ext) {
-    if (return_value[return_value.length() - 1] != L'.')
-      return_value.append(L".");
-    return_value.append(suggested_ext);
+    std::wstring file_part;
+    if (!IsDirectory(default_path))
+      file_part = default_path.BaseName().value();
+
+    dialog_.reset(new T(
+        file_part.c_str(),
+        options,
+        NULL,
+        filters.data(),
+        filters.size()));
+
+    if (!title.empty())
+      GetPtr()->SetTitle(UTF8ToUTF16(title).c_str());
+
+    SetDefaultFolder(default_path);
   }
 
-  // Strip any trailing dots, which Windows doesn't allow.
-  size_t index = return_value.find_last_not_of(L'.');
-  if (index < return_value.size() - 1)
-    return_value.resize(index + 1);
-
-  return return_value;
-}
-
-// Enforce visible dialog box.
-UINT_PTR CALLBACK SaveAsDialogHook(HWND dialog, UINT message,
-                                   WPARAM wparam, LPARAM lparam) {
-  static const UINT kPrivateMessage = 0x2F3F;
-  switch (message) {
-    case WM_INITDIALOG: {
-      // Do nothing here. Just post a message to defer actual processing.
-      PostMessage(dialog, kPrivateMessage, 0, 0);
-      return TRUE;
-    }
-    case kPrivateMessage: {
-      // The dialog box is the parent of the current handle.
-      HWND real_dialog = GetParent(dialog);
-
-      // Retrieve the final size.
-      RECT dialog_rect;
-      GetWindowRect(real_dialog, &dialog_rect);
-
-      // Verify that the upper left corner is visible.
-      POINT point = { dialog_rect.left, dialog_rect.top };
-      HMONITOR monitor1 = MonitorFromPoint(point, MONITOR_DEFAULTTONULL);
-      point.x = dialog_rect.right;
-      point.y = dialog_rect.bottom;
-
-      // Verify that the lower right corner is visible.
-      HMONITOR monitor2 = MonitorFromPoint(point, MONITOR_DEFAULTTONULL);
-      if (monitor1 && monitor2)
-        return 0;
-
-      // Some part of the dialog box is not visible, fix it by moving is to the
-      // client rect position of the browser window.
-      HWND parent_window = GetParent(real_dialog);
-      if (!parent_window)
-        return 0;
-      WINDOWINFO parent_info;
-      parent_info.cbSize = sizeof(WINDOWINFO);
-      GetWindowInfo(parent_window, &parent_info);
-      SetWindowPos(real_dialog, NULL,
-                   parent_info.rcClient.left,
-                   parent_info.rcClient.top,
-                   0, 0,  // Size.
-                   SWP_NOACTIVATE | SWP_NOOWNERZORDER | SWP_NOSIZE |
-                   SWP_NOZORDER);
-
-      return 0;
-    }
+  bool Show(atom::NativeWindow* parent_window) {
+    HWND window = parent_window ? parent_window->GetNativeWindow() : NULL;
+    return dialog_->DoModal(window) == IDOK;
   }
-  return 0;
-}
+
+  T* GetDialog() { return dialog_.get(); }
+
+  IFileDialog* GetPtr() const { return dialog_->GetPtr(); }
+
+  const std::vector<std::wstring> file_ext() const { return file_ext_; }
+
+ private:
+  // Set up the initial directory for the dialog.
+  void SetDefaultFolder(const base::FilePath file_path) {
+    std::wstring directory = IsDirectory(file_path) ?
+        file_path.value() :
+        file_path.DirName().value();
+
+    ATL::CComPtr<IShellItem> folder_item;
+    HRESULT hr = SHCreateItemFromParsingName(directory.c_str(),
+                                             NULL,
+                                             IID_PPV_ARGS(&folder_item));
+    if (SUCCEEDED(hr))
+      GetPtr()->SetDefaultFolder(folder_item);
+  }
+
+  scoped_ptr<T> dialog_;
+
+  std::vector<std::wstring> file_ext_;
+  std::vector<std::wstring> desc_ext_;
+  std::vector<COMDLG_FILTERSPEC> filters_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileDialog);
+};
 
 }  // namespace
 
-bool ShowOpenDialog(const std::string& title,
+bool ShowOpenDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
                     const base::FilePath& default_path,
                     int properties,
                     std::vector<base::FilePath>* paths) {
-  return false;
-}
+  int options = FOS_FORCEFILESYSTEM | FOS_FILEMUSTEXIST;
+  if (properties & FILE_DIALOG_OPEN_DIRECTORY)
+    options |= FOS_PICKFOLDERS;
+  if (properties & FILE_DIALOG_MULTI_SELECTIONS)
+    options |= FOS_ALLOWMULTISELECT;
 
-bool ShowSaveDialog(atom::NativeWindow* window,
-                    const std::string& title,
-                    const base::FilePath& default_path,
-                    base::FilePath* path) {
-  std::wstring file_ext = default_path.Extension().insert(0, L"*");
-  std::wstring filter = FormatFilterForExtensions(
-      std::vector<std::wstring>(1, file_ext),
+  FileDialog<CShellFileOpenDialog> open_dialog(
+      default_path,
+      title,
+      options,
       std::vector<std::wstring>(),
-      true);
-
-  std::wstring file_part = default_path.BaseName().value();
-  // If the default_path is a root directory, file_part will be '\', and the
-  // call to GetSaveFileName below will fail.
-  if (file_part.size() == 1 && file_part[0] == L'\\')
-    file_part.clear();
-
-  // The size of the in/out buffer in number of characters we pass to win32
-  // GetSaveFileName.  From MSDN "The buffer must be large enough to store the
-  // path and file name string or strings, including the terminating NULL
-  // character.  ... The buffer should be at least 256 characters long.".
-  // _IsValidPathComDlg does a copy expecting at most MAX_PATH, otherwise will
-  // result in an error of FNERR_INVALIDFILENAME.  So we should only pass the
-  // API a buffer of at most MAX_PATH.
-  wchar_t file_name[MAX_PATH];
-  base::wcslcpy(file_name, file_part.c_str(), arraysize(file_name));
-
-  OPENFILENAME save_as;
-  // We must do this otherwise the ofn's FlagsEx may be initialized to random
-  // junk in release builds which can cause the Places Bar not to show up!
-  ZeroMemory(&save_as, sizeof(save_as));
-  save_as.lStructSize = sizeof(OPENFILENAME);
-  save_as.hwndOwner = window->GetNativeWindow();
-  save_as.hInstance = NULL;
-
-  save_as.lpstrFilter = filter.empty() ? NULL : filter.c_str();
-
-  save_as.lpstrCustomFilter = NULL;
-  save_as.nMaxCustFilter = 0;
-  save_as.nFilterIndex = 1;
-  save_as.lpstrFile = file_name;
-  save_as.nMaxFile = arraysize(file_name);
-  save_as.lpstrFileTitle = NULL;
-  save_as.nMaxFileTitle = 0;
-
-  // Set up the initial directory for the dialog.
-  std::wstring directory;
-  if (IsDirectory(default_path)) {
-    directory = default_path.value();
-    file_part.clear();
-  } else {
-    directory = default_path.DirName().value();
-  }
-
-  save_as.lpstrInitialDir = directory.c_str();
-  save_as.lpstrTitle = NULL;
-  save_as.Flags = OFN_OVERWRITEPROMPT | OFN_EXPLORER | OFN_ENABLESIZING |
-                  OFN_NOCHANGEDIR | OFN_PATHMUSTEXIST;
-  save_as.lpstrDefExt = NULL;  // default extension, ignored for now.
-  save_as.lCustData = NULL;
-
-  if (base::win::GetVersion() < base::win::VERSION_VISTA) {
-    // The save as on Windows XP remembers its last position,
-    // and if the screen resolution changed, it will be off screen.
-    save_as.Flags |= OFN_ENABLEHOOK;
-    save_as.lpfnHook = &SaveAsDialogHook;
-  }
-
-  // Must be NULL or 0.
-  save_as.pvReserved = NULL;
-  save_as.dwReserved = 0;
-
-  if (!GetSaveFileName(&save_as)) {
-    // Zero means the dialog was closed, otherwise we had an error.
-    DWORD error_code = CommDlgExtendedError();
-    if (error_code != 0) {
-      NOTREACHED() << "GetSaveFileName failed with code: " << error_code;
-    }
+      std::vector<std::wstring>());
+  if (!open_dialog.Show(parent_window))
     return false;
+
+  ATL::CComPtr<IShellItemArray> items;
+  HRESULT hr = static_cast<IFileOpenDialog*>(open_dialog.GetPtr())->GetResults(
+      &items);
+  if (FAILED(hr))
+    return false;
+
+  ATL::CComPtr<IShellItem> item;
+  DWORD count = 0;
+  hr = items->GetCount(&count);
+  if (FAILED(hr))
+    return false;
+
+  paths->reserve(count);
+  for (DWORD i = 0; i < count; ++i) {
+    hr = items->GetItemAt(i, &item);
+    if (FAILED(hr))
+      return false;
+
+    wchar_t file_name[MAX_PATH];
+    hr = CShellFileOpenDialog::GetFileNameFromShellItem(
+        item, SIGDN_FILESYSPATH, file_name, MAX_PATH);
+    if (FAILED(hr))
+      return false;
+
+    paths->push_back(base::FilePath(file_name));
   }
 
-  // Return the user's choice.
-  *path = base::FilePath();
-
-  // Figure out what filter got selected from the vector with embedded nulls.
-  // NOTE: The filter contains a string with embedded nulls, such as:
-  // JPG Image\0*.jpg\0All files\0*.*\0\0
-  // The filter index is 1-based index for which pair got selected. So, using
-  // the example above, if the first index was selected we need to skip 1
-  // instance of null to get to "*.jpg".
-  std::vector<std::wstring> filters;
-  if (!filter.empty() && save_as.nFilterIndex > 0)
-    base::SplitString(filter, '\0', &filters);
-  std::wstring filter_selected;
-  if (!filters.empty())
-    filter_selected = filters[(2 * (save_as.nFilterIndex - 1)) + 1];
-
-  // Get the extension that was suggested to the user (when the Save As dialog
-  // was opened).
-  std::wstring suggested_ext =
-    GetExtensionWithoutLeadingDot(default_path.Extension());
-
-  *path = base::FilePath(AppendExtensionIfNeeded(
-        save_as.lpstrFile, filter_selected, suggested_ext));
   return true;
 }
 
+void ShowOpenDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
+                    const base::FilePath& default_path,
+                    int properties,
+                    const OpenDialogCallback& callback) {
+  std::vector<base::FilePath> paths;
+  bool result = ShowOpenDialog(parent_window,
+                               title,
+                               default_path,
+                               properties,
+                               &paths);
+  callback.Run(result, paths);
+}
+
+bool ShowSaveDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
+                    const base::FilePath& default_path,
+                    base::FilePath* path) {
+  // TODO(zcbenz): Accept custom filters from caller.
+  std::vector<std::wstring> file_ext;
+  std::wstring extension = default_path.Extension();
+  if (!extension.empty())
+    file_ext.push_back(extension.insert(0, L"*"));
+
+  FileDialog<CShellFileSaveDialog> save_dialog(
+      default_path,
+      title,
+      FOS_FORCEFILESYSTEM | FOS_PATHMUSTEXIST | FOS_OVERWRITEPROMPT,
+      file_ext,
+      std::vector<std::wstring>());
+  if (!save_dialog.Show(parent_window))
+    return false;
+
+  wchar_t file_name[MAX_PATH];
+  HRESULT hr = save_dialog.GetDialog()->GetFilePath(file_name, MAX_PATH);
+  if (FAILED(hr))
+    return false;
+
+  // Append extension according to selected filter.
+  UINT filter_index = 1;
+  save_dialog.GetPtr()->GetFileTypeIndex(&filter_index);
+  std::wstring selected_filter = save_dialog.file_ext()[filter_index - 1];
+  if (selected_filter != L"*.*") {
+    std::wstring result = file_name;
+    if (!EndsWith(result, selected_filter.substr(1), false)) {
+      if (result[result.length() - 1] != L'.')
+        result.push_back(L'.');
+      result.append(selected_filter.substr(2));
+      *path = base::FilePath(result);
+      return true;
+    }
+  }
+
+  *path = base::FilePath(file_name);
+  return true;
+}
+
+void ShowSaveDialog(atom::NativeWindow* parent_window,
+                    const std::string& title,
+                    const base::FilePath& default_path,
+                    const SaveDialogCallback& callback) {
+  base::FilePath path;
+  bool result = ShowSaveDialog(parent_window, title, default_path, &path);
+  callback.Run(result, path);
+}
+
 }  // namespace file_dialog

browser/ui/message_box.h

@@ -8,6 +8,8 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_forward.h"
+
 namespace atom {
 
 class NativeWindow;
@@ -18,6 +20,8 @@ enum MessageBoxType {
   MESSAGE_BOX_TYPE_WARNING
 };
 
+typedef base::Callback<void(int code)> MessageBoxCallback;
+
 int ShowMessageBox(NativeWindow* parent_window,
                    MessageBoxType type,
                    const std::vector<std::string>& buttons,
@@ -25,6 +29,14 @@ int ShowMessageBox(NativeWindow* parent_window,
                    const std::string& message,
                    const std::string& detail);
 
+void ShowMessageBox(NativeWindow* parent_window,
+                    MessageBoxType type,
+                    const std::vector<std::string>& buttons,
+                    const std::string& title,
+                    const std::string& message,
+                    const std::string& detail,
+                    const MessageBoxCallback& callback);
+
 }  // namespace atom
 
 #endif  // BROWSER_UI_MESSAGE_BOX_H_

browser/ui/message_box_mac.mm

@@ -6,20 +6,53 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include "base/callback.h"
 #include "base/strings/sys_string_conversions.h"
 #include "browser/native_window.h"
 #include "browser/ui/nsalert_synchronous_sheet_mac.h"
 
+@interface ModalDelegate : NSObject {
+ @private
+  atom::MessageBoxCallback callback_;
+  NSAlert* alert_;
+}
+- (id)initWithCallback:(const atom::MessageBoxCallback&)callback
+              andAlert:(NSAlert*)alert;
+@end
+
+@implementation ModalDelegate
+
+- (id)initWithCallback:(const atom::MessageBoxCallback&)callback
+              andAlert:(NSAlert*)alert {
+  if ((self = [super init])) {
+    callback_ = callback;
+    alert_ = alert;
+  }
+  return self;
+}
+
+- (void)alertDidEnd:(NSAlert*)alert
+         returnCode:(NSInteger)returnCode
+        contextInfo:(void*)contextInfo {
+  callback_.Run(returnCode);
+  [alert_ release];
+  [self release];
+}
+
+@end
+
 namespace atom {
 
-int ShowMessageBox(NativeWindow* parent_window,
-                   MessageBoxType type,
-                   const std::vector<std::string>& buttons,
-                   const std::string& title,
-                   const std::string& message,
-                   const std::string& detail) {
+namespace {
+
+NSAlert* CreateNSAlert(NativeWindow* parent_window,
+                       MessageBoxType type,
+                       const std::vector<std::string>& buttons,
+                       const std::string& title,
+                       const std::string& message,
+                       const std::string& detail) {
   // Ignore the title; it's the window title on other platforms and ignorable.
-  NSAlert* alert = [[[NSAlert alloc] init] autorelease];
+  NSAlert* alert = [[NSAlert alloc] init];
   [alert setMessageText:base::SysUTF8ToNSString(message)];
   [alert setInformativeText:base::SysUTF8ToNSString(detail)];
 
@@ -40,10 +73,44 @@ int ShowMessageBox(NativeWindow* parent_window,
     [button setTag:i];
   }
 
+  return alert;
+}
+
+}  // namespace
+
+int ShowMessageBox(NativeWindow* parent_window,
+                   MessageBoxType type,
+                   const std::vector<std::string>& buttons,
+                   const std::string& title,
+                   const std::string& message,
+                   const std::string& detail) {
+  NSAlert* alert = CreateNSAlert(
+      parent_window, type, buttons, title, message, detail);
+  [alert autorelease];
+
   if (parent_window)
     return [alert runModalSheetForWindow:parent_window->GetNativeWindow()];
   else
     return [alert runModal];
 }
 
+void ShowMessageBox(NativeWindow* parent_window,
+                    MessageBoxType type,
+                    const std::vector<std::string>& buttons,
+                    const std::string& title,
+                    const std::string& message,
+                    const std::string& detail,
+                    const MessageBoxCallback& callback) {
+  NSAlert* alert = CreateNSAlert(
+      parent_window, type, buttons, title, message, detail);
+  ModalDelegate* delegate = [[ModalDelegate alloc] initWithCallback:callback
+                                                           andAlert:alert];
+
+  NSWindow* window = parent_window ? parent_window->GetNativeWindow() : nil;
+  [alert beginSheetModalForWindow:window
+                    modalDelegate:delegate
+                   didEndSelector:@selector(alertDidEnd:returnCode:contextInfo:)
+                      contextInfo:nil];
+}
+
 }  // namespace atom

browser/ui/message_box_win.cc

@@ -4,6 +4,7 @@
 
 #include "browser/ui/message_box.h"
 
+#include "base/callback.h"
 #include "base/message_loop.h"
 #include "base/run_loop.h"
 #include "base/string_util.h"
@@ -38,7 +39,14 @@ class MessageDialog : public base::MessageLoop::Dispatcher,
                 const std::string& detail);
   virtual ~MessageDialog();
 
-  int result() const { return result_; }
+  void Show();
+
+  int GetResult() const;
+
+  void set_callback(const MessageBoxCallback& callback) {
+    delete_on_close_ = true;
+    callback_ = callback;
+  }
 
  private:
   // Overridden from MessageLoop::Dispatcher:
@@ -63,11 +71,13 @@ class MessageDialog : public base::MessageLoop::Dispatcher,
                              const ui::Event& event) OVERRIDE;
 
   bool should_close_;
+  bool delete_on_close_;
   int result_;
   string16 title_;
   views::Widget* widget_;
   views::MessageBoxView* message_box_view_;
   std::vector<views::LabelButton*> buttons_;
+  MessageBoxCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(MessageDialog);
 };
@@ -82,6 +92,7 @@ MessageDialog::MessageDialog(NativeWindow* parent_window,
                              const std::string& message,
                              const std::string& detail)
     : should_close_(false),
+      delete_on_close_(false),
       result_(-1),
       title_(UTF8ToUTF16(title)),
       widget_(NULL),
@@ -124,12 +135,30 @@ MessageDialog::MessageDialog(NativeWindow* parent_window,
 
   set_background(views::Background::CreateSolidBackground(
         skia::COLORREFToSkColor(GetSysColor(COLOR_WINDOW))));
-  widget_->Show();
 }
 
 MessageDialog::~MessageDialog() {
 }
 
+void MessageDialog::Show() {
+  widget_->Show();
+}
+
+int MessageDialog::GetResult() const {
+  // When the dialog is closed without choosing anything, we think the user
+  // chose 'Cancel', otherwise we think the default behavior is chosen.
+  if (result_ == -1) {
+    for (size_t i = 0; i < buttons_.size(); ++i)
+      if (LowerCaseEqualsASCII(buttons_[i]->GetText(), "cancel")) {
+        return i;
+      }
+
+    return 0;
+  } else {
+    return result_;
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // MessageDialog, private:
 
@@ -145,6 +174,11 @@ string16 MessageDialog::GetWindowTitle() const {
 
 void MessageDialog::WindowClosing() {
   should_close_ = true;
+
+  if (delete_on_close_) {
+    callback_.Run(GetResult());
+    base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+  }
 }
 
 views::Widget* MessageDialog::GetWidget() {
@@ -232,6 +266,7 @@ int ShowMessageBox(NativeWindow* parent_window,
                    const std::string& message,
                    const std::string& detail) {
   MessageDialog dialog(parent_window, type, buttons, title, message, detail);
+  dialog.Show();
   {
     base::MessageLoop::ScopedNestableTaskAllower allow(
         base::MessageLoopForUI::current());
@@ -239,18 +274,21 @@ int ShowMessageBox(NativeWindow* parent_window,
     run_loop.Run();
   }
 
-  // When the dialog is closed without choosing anything, we think the user
-  // chose 'Cancel', otherwise we think the default behavior is chosen.
-  if (dialog.result() == -1) {
-    for (size_t i = 0; i < buttons.size(); ++i)
-      if (LowerCaseEqualsASCII(buttons[i], "cancel")) {
-        return i;
-      }
+  return dialog.GetResult();
+}
 
-    return 0;
-  } else {
-    return dialog.result();
-  }
+void ShowMessageBox(NativeWindow* parent_window,
+                    MessageBoxType type,
+                    const std::vector<std::string>& buttons,
+                    const std::string& title,
+                    const std::string& message,
+                    const std::string& detail,
+                    const MessageBoxCallback& callback) {
+  // The dialog would be deleted when the dialog is closed.
+  MessageDialog* dialog = new MessageDialog(
+      parent_window, type, buttons, title, message, detail);
+  dialog->set_callback(callback);
+  dialog->Show();
 }
 
 }  // namespace atom

common/api/api_messages.h

@@ -4,8 +4,7 @@
 
 // Multiply-included file, no traditional include guard.
 
-#include <string>
-
+#include "base/string16.h"
 #include "base/values.h"
 #include "common/draggable_region.h"
 #include "content/public/common/common_param_traits.h"
@@ -22,16 +21,16 @@ IPC_STRUCT_TRAITS_BEGIN(atom::DraggableRegion)
 IPC_STRUCT_TRAITS_END()
 
 IPC_MESSAGE_ROUTED2(AtomViewHostMsg_Message,
-                    std::string /* channel */,
+                    string16 /* channel */,
                     ListValue /* arguments */)
 
 IPC_SYNC_MESSAGE_ROUTED2_1(AtomViewHostMsg_Message_Sync,
-                           std::string /* channel */,
+                           string16 /* channel */,
                            ListValue /* arguments */,
-                           DictionaryValue /* result */)
+                           string16 /* result (in JSON) */)
 
 IPC_MESSAGE_ROUTED2(AtomViewMsg_Message,
-                    std::string /* channel */,
+                    string16 /* channel */,
                     ListValue /* arguments */)
 
 // Sent by the renderer when the draggable regions are updated.

common/api/atom_bindings.cc

@@ -7,17 +7,35 @@
 #include "base/debug/debugger.h"
 #include "base/logging.h"
 #include "common/atom_version.h"
+#include "common/v8_conversions.h"
 #include "vendor/node/src/node.h"
 
 namespace atom {
 
 namespace {
 
+static int kMaxCallStackSize = 200;  // Same with WebKit.
+
 static uv_async_t dummy_uv_handle;
 
 void UvNoOp(uv_async_t* handle, int status) {
 }
 
+v8::Handle<v8::Object> DumpStackFrame(v8::Handle<v8::StackFrame> stack_frame) {
+  v8::Local<v8::Object> result = v8::Object::New();
+  result->Set(ToV8Value("line"), ToV8Value(stack_frame->GetLineNumber()));
+  result->Set(ToV8Value("column"), ToV8Value(stack_frame->GetColumn()));
+
+  v8::Handle<v8::String> script = stack_frame->GetScriptName();
+  if (!script.IsEmpty())
+    result->Set(ToV8Value("script"), script);
+
+  v8::Handle<v8::String> function = stack_frame->GetScriptNameOrSourceURL();
+  if (!function.IsEmpty())
+    result->Set(ToV8Value("function"), function);
+  return result;
+}
+
 }  // namespace
 
 // Defined in atom_extensions.cc.
@@ -37,6 +55,7 @@ void AtomBindings::BindTo(v8::Handle<v8::Object> process) {
   node::SetMethod(process, "crash", Crash);
   node::SetMethod(process, "activateUvLoop", ActivateUVLoop);
   node::SetMethod(process, "log", Log);
+  node::SetMethod(process, "getCurrentStackTrace", GetCurrentStackTrace);
 
   process->Get(v8::String::New("versions"))->ToObject()->
     Set(v8::String::New("atom-shell"), v8::String::New(ATOM_VERSION_STRING));
@@ -111,4 +130,23 @@ v8::Handle<v8::Value> AtomBindings::Log(const v8::Arguments& args) {
   return v8::Undefined();
 }
 
+// static
+v8::Handle<v8::Value> AtomBindings::GetCurrentStackTrace(
+    const v8::Arguments& args) {
+  v8::HandleScope scope;
+
+  int stack_limit = kMaxCallStackSize;
+  FromV8Arguments(args, &stack_limit);
+
+  v8::Local<v8::StackTrace> stack_trace = v8::StackTrace::CurrentStackTrace(
+      stack_limit, v8::StackTrace::kDetailed);
+
+  int frame_count = stack_trace->GetFrameCount();
+  v8::Local<v8::Array> result = v8::Array::New(frame_count);
+  for (int i = 0; i < frame_count; ++i)
+    result->Set(i, DumpStackFrame(stack_trace->GetFrame(i)));
+
+  return scope.Close(result);
+}
+
 }  // namespace atom

common/api/atom_bindings.h

@@ -24,6 +24,7 @@ class AtomBindings {
   static v8::Handle<v8::Value> Crash(const v8::Arguments& args);
   static v8::Handle<v8::Value> ActivateUVLoop(const v8::Arguments& args);
   static v8::Handle<v8::Value> Log(const v8::Arguments& args);
+  static v8::Handle<v8::Value> GetCurrentStackTrace(const v8::Arguments& args);
 
   DISALLOW_COPY_AND_ASSIGN(AtomBindings);
 };

common/atom_version.h

@@ -6,8 +6,8 @@
 #define ATOM_VERSION_H
 
 #define ATOM_MAJOR_VERSION 0
-#define ATOM_MINOR_VERSION 4
-#define ATOM_PATCH_VERSION 8
+#define ATOM_MINOR_VERSION 5
+#define ATOM_PATCH_VERSION 3
 
 #define ATOM_VERSION_IS_RELEASE 1
 

common/v8_conversions.h

@@ -0,0 +1,232 @@
+// Copyright (c) 2013 GitHub, Inc. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMMON_V8_CONVERSIONS_H_
+#define COMMON_V8_CONVERSIONS_H_
+
+#include <string>
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "base/string16.h"
+#include "browser/api/atom_api_window.h"
+#include "v8/include/v8.h"
+
+// Convert V8 value to arbitrary supported types.
+struct FromV8Value {
+  explicit FromV8Value(v8::Handle<v8::Value> value) : value_(value) {}
+
+  operator int() {
+    return value_->IntegerValue();
+  }
+
+  operator bool() {
+    return value_->BooleanValue();
+  }
+
+  operator std::string() {
+    return *v8::String::Utf8Value(value_);
+  }
+
+  operator string16() {
+    v8::String::Value s(value_);
+    return string16(reinterpret_cast<const char16*>(*s), s.length());
+  }
+
+  operator base::FilePath() {
+    return base::FilePath::FromUTF8Unsafe(FromV8Value(value_));
+  }
+
+  operator std::vector<std::string>() {
+    std::vector<std::string> array;
+    v8::Handle<v8::Array> v8_array = v8::Handle<v8::Array>::Cast(value_);
+    for (uint32_t i = 0; i < v8_array->Length(); ++i)
+      array.push_back(FromV8Value(v8_array->Get(i)));
+
+    return array;
+  }
+
+  operator atom::NativeWindow*() {
+    using atom::api::Window;
+    if (value_->IsObject()) {
+      Window* window = Window::Unwrap<Window>(value_->ToObject());
+      if (window && window->window())
+        return window->window();
+    }
+    return NULL;
+  }
+
+  operator v8::Persistent<v8::Function>() {
+    return value_->IsFunction() ?
+      v8::Persistent<v8::Function>::New(
+          node::node_isolate,
+          v8::Handle<v8::Function>::Cast(value_)) :
+      v8::Persistent<v8::Function>();
+  }
+
+  v8::Handle<v8::Value> value_;
+};
+
+// Convert arbitrary supported native type to V8 value.
+inline v8::Handle<v8::Value> ToV8Value(int i) {
+  return v8::Integer::New(i);
+}
+
+inline v8::Handle<v8::Value> ToV8Value(bool b) {
+  return v8::Boolean::New(b);
+}
+
+inline v8::Handle<v8::Value> ToV8Value(const char* s) {
+  return v8::String::New(s);
+}
+
+inline v8::Handle<v8::Value> ToV8Value(const std::string& s) {
+  return v8::String::New(s.data(), s.size());
+}
+
+inline v8::Handle<v8::Value> ToV8Value(const string16& s) {
+  return v8::String::New(reinterpret_cast<const uint16_t*>(s.data()), s.size());
+}
+
+inline v8::Handle<v8::Value> ToV8Value(const base::FilePath& path) {
+  std::string path_string(path.AsUTF8Unsafe());
+  return v8::String::New(path_string.data(), path_string.size());
+}
+
+inline v8::Handle<v8::Value> ToV8Value(void* whatever) {
+  return v8::Undefined();
+}
+
+inline
+v8::Handle<v8::Value> ToV8Value(const std::vector<base::FilePath>& paths) {
+  v8::Handle<v8::Array> result = v8::Array::New(paths.size());
+  for (size_t i = 0; i < paths.size(); ++i)
+    result->Set(i, ToV8Value(paths[i]));
+  return result;
+}
+
+// Check if a V8 Value is of specified type.
+template<class T> inline
+bool V8ValueCanBeConvertedTo(v8::Handle<v8::Value> value) {
+  return false;
+}
+
+template<> inline
+bool V8ValueCanBeConvertedTo<int>(v8::Handle<v8::Value> value) {
+  return value->IsNumber();
+}
+
+template<> inline
+bool V8ValueCanBeConvertedTo<bool>(v8::Handle<v8::Value> value) {
+  return value->IsBoolean();
+}
+
+template<> inline
+bool V8ValueCanBeConvertedTo<std::string>(v8::Handle<v8::Value> value) {
+  return value->IsString();
+}
+
+template<> inline
+bool V8ValueCanBeConvertedTo<string16>(v8::Handle<v8::Value> value) {
+  return V8ValueCanBeConvertedTo<std::string>(value);
+}
+
+template<> inline
+bool V8ValueCanBeConvertedTo<base::FilePath>(v8::Handle<v8::Value> value) {
+  return V8ValueCanBeConvertedTo<std::string>(value);
+}
+
+template<> inline
+bool V8ValueCanBeConvertedTo<std::vector<std::string>>(
+    v8::Handle<v8::Value> value) {
+  return value->IsArray();
+}
+
+template<> inline
+bool V8ValueCanBeConvertedTo<atom::NativeWindow*>(v8::Handle<v8::Value> value) {
+  using atom::api::Window;
+  if (value->IsObject()) {
+    Window* window = Window::Unwrap<Window>(value->ToObject());
+    if (window && window->window())
+      return true;
+  }
+  return false;
+}
+
+template<> inline
+bool V8ValueCanBeConvertedTo<v8::Persistent<v8::Function>>(
+    v8::Handle<v8::Value> value) {
+  return value->IsFunction();
+}
+
+// Check and convert V8's Arguments to native types.
+template<typename T1> inline
+bool FromV8Arguments(const v8::Arguments& args, T1* value, int index = 0) {
+  if (!V8ValueCanBeConvertedTo<T1>(args[index]))
+    return false;
+  *value = static_cast<const T1&>(FromV8Value(args[index]));
+  return true;
+}
+
+template<typename T1, typename T2> inline
+bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2) {
+  return FromV8Arguments<T1>(args, a1) && FromV8Arguments<T2>(args, a2, 1);
+}
+
+template<typename T1, typename T2, typename T3> inline
+bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2, T3* a3) {
+  return FromV8Arguments<T1, T2>(args, a1, a2) &&
+         FromV8Arguments<T3>(args, a3, 2);
+}
+
+template<typename T1, typename T2, typename T3, typename T4> inline
+bool FromV8Arguments(const v8::Arguments& args,
+                     T1* a1,
+                     T2* a2,
+                     T3* a3,
+                     T4* a4) {
+  return FromV8Arguments<T1, T2, T3>(args, a1, a2, a3) &&
+         FromV8Arguments<T4>(args, a4, 3);
+}
+
+template<typename T1, typename T2, typename T3, typename T4, typename T5> inline
+bool FromV8Arguments(const v8::Arguments& args,
+                     T1* a1,
+                     T2* a2,
+                     T3* a3,
+                     T4* a4,
+                     T5* a5) {
+  return FromV8Arguments<T1, T2, T3, T4>(args, a1, a2, a3, a4) &&
+         FromV8Arguments<T5>(args, a5, 4);
+}
+
+template<typename T1, typename T2, typename T3, typename T4, typename T5,
+    typename T6> inline
+bool FromV8Arguments(const v8::Arguments& args,
+                     T1* a1,
+                     T2* a2,
+                     T3* a3,
+                     T4* a4,
+                     T5* a5,
+                     T6* a6) {
+  return FromV8Arguments<T1, T2, T3, T4, T5>(args, a1, a2, a3, a4, a5) &&
+         FromV8Arguments<T6>(args, a6, 5);
+}
+
+template<typename T1, typename T2, typename T3, typename T4, typename T5,
+    typename T6, typename T7> inline
+bool FromV8Arguments(const v8::Arguments& args,
+                     T1* a1,
+                     T2* a2,
+                     T3* a3,
+                     T4* a4,
+                     T5* a5,
+                     T6* a6,
+                     T7* a7) {
+  return
+      FromV8Arguments<T1, T2, T3, T4, T5, T6>(args, a1, a2, a3, a4, a5, a6) &&
+      FromV8Arguments<T7>(args, a7, 6);
+}
+
+#endif  // COMMON_V8_CONVERSIONS_H_

docs/api/browser/browser-window.md

@@ -141,6 +141,10 @@ Shows the window.
 
 Hides the window.
 
+### BrowserWindow.isVisible()
+
+Returns whether the window is visible to the user.
+
 ### BrowserWindow.maximize()
 
 Maximizes the window.

docs/api/browser/ipc-browser.md

@@ -18,7 +18,7 @@ Emitted when renderer sent a message to the browser.
 * `routingId` Integer
 
 Emitted when renderer sent a synchronous message to the browser. The receiver
-should store the result in `event.result`.
+should store the result in `event.returnValue`.
 
 **Note:** Due to the limitation of `EventEmitter`, returning value in the
 event handler has no effect, so we have to store the result by using the

docs/api/browser/protocol.md

@@ -14,6 +14,9 @@ protocol.registerProtocol('atom', function(request) {
 });
 ```
 
+**Note:** This module can only be used after the `will-finish-launching` event
+was emitted.
+
 ## protocol.registerProtocol(scheme, handler)
 
 * `scheme` String

docs/api/renderer/ipc-renderer.md

@@ -30,7 +30,7 @@ An example of sending synchronous message from renderer to browser:
 // In browser:
 var ipc = require('ipc');
 ipc.on('browser-data-request', function(event, processId, routingId, message) {
-  event.result = 'THIS SOME DATA FROM THE BROWSER';
+  event.returnValue = 'THIS SOME DATA FROM THE BROWSER';
 });
 ```
 

package.json

@@ -9,7 +9,8 @@
     "walkdir": "*",
     "unzip": "*",
     "d3": "*",
-    "int64-native": "*"
+    "int64-native": "*",
+    "coffeelint": "*"
   },
 
   "private": true,

renderer/api/atom_api_renderer_ipc.cc

@@ -6,6 +6,7 @@
 
 #include "base/values.h"
 #include "common/api/api_messages.h"
+#include "common/v8_conversions.h"
 #include "content/public/renderer/render_view.h"
 #include "content/public/renderer/v8_value_converter.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
@@ -47,8 +48,7 @@ v8::Handle<v8::Value> RendererIPC::Send(const v8::Arguments &args) {
   if (!args[0]->IsString())
     return node::ThrowTypeError("Bad argument");
 
-  std::string channel(*v8::String::Utf8Value(args[0]));
-
+  string16 channel = FromV8Value(args[0]);
   RenderView* render_view = GetCurrentRenderView();
 
   // Convert Arguments to Array, so we can use V8ValueConverter to convert it
@@ -82,7 +82,7 @@ v8::Handle<v8::Value> RendererIPC::SendSync(const v8::Arguments &args) {
     return node::ThrowTypeError("Bad argument");
 
   v8::Handle<v8::Context> context = v8::Context::GetCurrent();
-  std::string channel(*v8::String::Utf8Value(args[0]));
+  string16 channel = FromV8Value(args[0]);
 
   // Convert Arguments to Array, so we can use V8ValueConverter to convert it
   // to ListValue.
@@ -97,12 +97,12 @@ v8::Handle<v8::Value> RendererIPC::SendSync(const v8::Arguments &args) {
 
   RenderView* render_view = GetCurrentRenderView();
 
-  base::DictionaryValue result;
+  string16 json;
   IPC::SyncMessage* message = new AtomViewHostMsg_Message_Sync(
       render_view->GetRoutingID(),
       channel,
       *static_cast<base::ListValue*>(arguments.get()),
-      &result);
+      &json);
   // Enable the UI thread in browser to receive messages.
   message->EnableMessagePumping();
   bool success = render_view->Send(message);
@@ -110,7 +110,7 @@ v8::Handle<v8::Value> RendererIPC::SendSync(const v8::Arguments &args) {
   if (!success)
     return node::ThrowError("Unable to send AtomViewHostMsg_Message_Sync");
 
-  return scope.Close(converter->ToV8Value(&result, context));
+  return scope.Close(ToV8Value(json));
 }
 
 // static

renderer/api/atom_renderer_bindings.cc

@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/values.h"
+#include "common/v8_conversions.h"
 #include "content/public/renderer/render_view.h"
 #include "content/public/renderer/v8_value_converter.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
@@ -51,7 +52,7 @@ void AtomRendererBindings::BindToFrame(WebFrame* frame) {
   AtomBindings::BindTo(GetProcessObject(context));
 }
 
-void AtomRendererBindings::OnBrowserMessage(const std::string& channel,
+void AtomRendererBindings::OnBrowserMessage(const string16& channel,
                                             const base::ListValue& args) {
   if (!render_view_->GetWebView())
     return;
@@ -70,7 +71,7 @@ void AtomRendererBindings::OnBrowserMessage(const std::string& channel,
 
   std::vector<v8::Handle<v8::Value>> arguments;
   arguments.reserve(1 + args.GetSize());
-  arguments.push_back(v8::String::New(channel.c_str(), channel.size()));
+  arguments.push_back(ToV8Value(channel));
 
   for (size_t i = 0; i < args.GetSize(); i++) {
     const base::Value* value;

renderer/api/atom_renderer_bindings.h

@@ -5,10 +5,10 @@
 #ifndef ATOM_RENDERER_API_ATOM_RENDERER_BINDINGS_H_
 #define ATOM_RENDERER_API_ATOM_RENDERER_BINDINGS_H_
 
-#include <string>
-
 #include "common/api/atom_bindings.h"
 
+#include "base/string16.h"
+
 namespace base {
 class ListValue;
 }
@@ -32,7 +32,7 @@ class AtomRendererBindings : public AtomBindings {
   void BindToFrame(WebKit::WebFrame* frame);
 
   // Dispatch messages from browser.
-  void OnBrowserMessage(const std::string& channel,
+  void OnBrowserMessage(const string16& channel,
                         const base::ListValue& args);
 
  private:

renderer/api/lib/ipc.coffee

@@ -16,9 +16,11 @@ class Ipc extends EventEmitter
     ipc.send('ATOM_INTERNAL_MESSAGE', args...)
 
   sendSync: (args...) ->
-    ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', 'sync-message', args...).result
+    msg = ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', 'sync-message', args...)
+    JSON.parse(msg)
 
   sendChannelSync: (args...) ->
-    ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', args...).result
+    msg = ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', args...)
+    JSON.parse(msg)
 
 module.exports = new Ipc

renderer/api/lib/remote.coffee

@@ -72,6 +72,7 @@ metaToValue = (meta) ->
             ret.__defineSetter__ member.name, (value) ->
               # Set member data.
               ipc.sendChannelSync 'ATOM_BROWSER_MEMBER_SET', meta.id, member.name, value
+              value
 
             ret.__defineGetter__ member.name, ->
               # Get member data.

renderer/atom_render_view_observer.cc

@@ -101,7 +101,7 @@ bool AtomRenderViewObserver::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
-void AtomRenderViewObserver::OnBrowserMessage(const std::string& channel,
+void AtomRenderViewObserver::OnBrowserMessage(const string16& channel,
                                               const base::ListValue& args) {
   atom_bindings()->OnBrowserMessage(channel, args);
 }

renderer/atom_render_view_observer.h

@@ -35,7 +35,7 @@ class AtomRenderViewObserver : content::RenderViewObserver {
   virtual void DraggableRegionsChanged(WebKit::WebFrame* frame) OVERRIDE;
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
-  void OnBrowserMessage(const std::string& channel,
+  void OnBrowserMessage(const string16& channel,
                         const base::ListValue& args);
 
   scoped_ptr<AtomRendererBindings> atom_bindings_;

script/bootstrap.py

@@ -1,12 +1,11 @@
 #!/usr/bin/env python
 
 import argparse
-import errno
 import os
 import subprocess
 import sys
 
-from lib.util import *
+from lib.util import scoped_cwd
 
 
 SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__)))
@@ -68,7 +67,7 @@ def update_apm():
 
 def update_node_modules():
   for dirname in ['.', 'browser/default_app', 'spec']:
-    update_node_modules_for_dir(dirname);
+    update_node_modules_for_dir(dirname)
 
 
 def update_node_modules_for_dir(dirname):

script/cibuild

@@ -11,6 +11,8 @@ SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__)))
 def main():
   run_script('bootstrap.py')
   run_script('cpplint.py')
+  run_script('pylint.py')
+  run_script('coffeelint.py')
   run_script('build.py')
   run_script('test.py', ['--ci'])
   run_script('create-dist.py')

script/coffeelint.json

@@ -0,0 +1,6 @@
+{
+    "max_line_length": {
+        "value": 80,
+        "level": "ignore"
+    }
+}

script/coffeelint.py

@@ -0,0 +1,31 @@
+#!/usr/bin/env python
+
+import glob
+import os
+import subprocess
+import sys
+
+
+SOURCE_ROOT = os.path.dirname(os.path.dirname(__file__))
+
+
+def main():
+  os.chdir(SOURCE_ROOT)
+
+  coffeelint = os.path.join(SOURCE_ROOT, 'node_modules', 'coffeelint', 'bin',
+                            'coffeelint')
+  settings = ['--quiet', '-f', os.path.join('script', 'coffeelint.json')]
+  files = glob.glob('browser/api/lib/*.coffee') + \
+          glob.glob('renderer/api/lib/*.coffee') + \
+          glob.glob('common/api/lib/*.coffee') + \
+          glob.glob('browser/atom/*.coffee')
+
+  if sys.platform in ['win32', 'cygwin']:
+    subprocess.check_call(['node', coffeelint] + settings + files,
+                          executable='C:/Program Files/nodejs/node.exe')
+  else:
+    subprocess.check_call(['node', coffeelint] + settings + files)
+
+
+if __name__ == '__main__':
+  sys.exit(main())

script/compile-coffee.py

@@ -4,8 +4,6 @@ import os
 import subprocess
 import sys
 
-from lib.util import *
-
 
 SOURCE_ROOT = os.path.dirname(os.path.dirname(__file__))
 
@@ -22,5 +20,6 @@ def main():
   else:
     subprocess.check_call(['node', coffee, '-c', '-o', output_dir, input_file])
 
+
 if __name__ == '__main__':
   sys.exit(main())

script/cpplint.py

@@ -7,12 +7,11 @@ import sys
 
 IGNORE_FILES = [
   'app/win/resource.h',
-  'browser/atom_event_processing_window.h',
   'browser/atom_application_mac.h',
   'browser/atom_application_delegate_mac.h',
   'browser/native_window_mac.h',
+  'browser/ui/atom_event_processing_window.h',
   'browser/ui/atom_menu_controller_mac.h',
-  'browser/ui/cocoa/custom_frame_view.h',
   'browser/ui/nsalert_synchronous_sheet_mac.h',
   'common/api/api_messages.cc',
   'common/api/api_messages.h',
@@ -32,7 +31,7 @@ def main():
 def list_files(directories, filters):
   matches = []
   for directory in directories:
-    for root, dirs, filenames, in os.walk(directory):
+    for root, _, filenames, in os.walk(directory):
       for f in filters:
         for filename in fnmatch.filter(filenames, f):
           matches.append(os.path.join(root, filename))

script/create-dist.py

@@ -1,14 +1,13 @@
 #!/usr/bin/env python
 
-import errno
-import glob
 import os
 import shutil
 import subprocess
 import sys
 import tarfile
 
-from lib.util import *
+from lib.util import scoped_cwd, rm_rf, get_atom_shell_version, make_zip, \
+                     safe_mkdir
 
 
 ATOM_SHELL_VRESION = get_atom_shell_version()
@@ -79,7 +78,7 @@ def main():
 
 def force_build():
   build = os.path.join(SOURCE_ROOT, 'script', 'build.py')
-  subprocess.check_call([sys.executable, build, '-c', 'Release']);
+  subprocess.check_call([sys.executable, build, '-c', 'Release'])
 
 
 def copy_binaries():
@@ -99,7 +98,7 @@ def copy_headers():
   # Copy standard node headers from node. repository.
   for include_path in HEADERS_DIRS:
     abs_path = os.path.join(NODE_DIR, include_path)
-    for dirpath, dirnames, filenames in os.walk(abs_path):
+    for dirpath, _, filenames in os.walk(abs_path):
       for filename in filenames:
         extension = os.path.splitext(filename)[1]
         if extension not in HEADERS_SUFFIX:
@@ -111,7 +110,7 @@ def copy_headers():
   # Copy V8 headers from chromium's repository.
   src = os.path.join(SOURCE_ROOT, 'vendor', 'brightray', 'vendor', 'download',
                     'libchromiumcontent', 'src')
-  for dirpath, dirnames, filenames in os.walk(os.path.join(src, 'v8')):
+  for dirpath, _, filenames in os.walk(os.path.join(src, 'v8')):
     for filename in filenames:
       extension = os.path.splitext(filename)[1]
       if extension not in HEADERS_SUFFIX:
@@ -122,8 +121,7 @@ def copy_headers():
 
 
 def copy_license():
-  license = os.path.join(SOURCE_ROOT, 'LICENSE')
-  shutil.copy2(license, DIST_DIR)
+  shutil.copy2(os.path.join(SOURCE_ROOT, 'LICENSE'), DIST_DIR)
 
 
 def create_version():

script/lib/github.py

@@ -0,0 +1,70 @@
+#!/usr/bin/env python
+
+import json
+import re
+import requests
+
+GITHUB_URL = 'https://api.github.com'
+GITHUB_UPLOAD_ASSET_URL = 'https://uploads.github.com'
+
+class GitHub:
+  def __init__(self, access_token):
+    self._authorization = 'token %s' % access_token
+
+    pattern = '^/repos/{0}/{0}/releases/{1}/assets$'.format('[^/]+', '[0-9]+')
+    self._releases_upload_api_pattern = re.compile(pattern)
+
+  def __getattr__(self, attr):
+    return _Callable(self, '/%s' % attr)
+
+  def send(self, method, path, **kw):
+    if not 'headers' in kw:
+      kw['headers'] = dict()
+    headers = kw['headers']
+    headers['Authorization'] = self._authorization
+    headers['Accept'] = 'application/vnd.github.manifold-preview'
+
+    # Data are sent in JSON format.
+    if 'data' in kw:
+      kw['data'] = json.dumps(kw['data'])
+
+    # Switch to a different domain for the releases uploading API.
+    if self._releases_upload_api_pattern.match(path):
+      url = '%s%s' % (GITHUB_UPLOAD_ASSET_URL, path)
+    else:
+      url = '%s%s' % (GITHUB_URL, path)
+
+    r = getattr(requests, method)(url, **kw).json()
+    if 'message' in r:
+      raise Exception(json.dumps(r, indent=2, separators=(',', ': ')))
+    return r
+
+
+class _Executable:
+  def __init__(self, gh, method, path):
+    self._gh = gh
+    self._method = method
+    self._path = path
+
+  def __call__(self, **kw):
+    return self._gh.send(self._method, self._path, **kw)
+
+
+class _Callable(object):
+  def __init__(self, gh, name):
+    self._gh = gh
+    self._name = name
+
+  def __call__(self, *args):
+    if len(args) == 0:
+      return self
+
+    name = '%s/%s' % (self._name, '/'.join([str(arg) for arg in args]))
+    return _Callable(self._gh, name)
+
+  def __getattr__(self, attr):
+    if attr in ['get', 'put', 'post', 'patch', 'delete']:
+      return _Executable(self._gh, attr, self._name)
+
+    name = '%s/%s' % (self._name, attr)
+    return _Callable(self._gh, name)

script/pylint.py

@@ -0,0 +1,22 @@
+#!/usr/bin/env python
+
+import glob
+import os
+import subprocess
+import sys
+
+SOURCE_ROOT = os.path.dirname(os.path.dirname(__file__))
+
+
+def main():
+  os.chdir(SOURCE_ROOT)
+
+  pylint = os.path.join(SOURCE_ROOT, 'vendor', 'depot_tools', 'pylint.py')
+  settings = ['--rcfile=vendor/depot_tools/pylintrc']
+  pys = glob.glob('script/*.py')
+  subprocess.check_call([sys.executable, pylint] + settings + pys,
+                        env=dict(PYTHONPATH='script'))
+
+
+if __name__ == '__main__':
+  sys.exit(main())

script/update-frameworks.py

@@ -3,11 +3,11 @@
 import sys
 import os
 
-from lib.util import *
+from lib.util import safe_mkdir, extract_zip, tempdir, download
 
 
 SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__)))
-FRAMEWORKS_URL='https://gh-contractor-zcbenz.s3.amazonaws.com/frameworks'
+FRAMEWORKS_URL = 'https://gh-contractor-zcbenz.s3.amazonaws.com/frameworks'
 
 
 def main():

script/update.py

@@ -1,10 +1,9 @@
 #!/usr/bin/env python
 
+import os
 import subprocess
 import sys
 
-from lib.util import *
-
 
 SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__)))
 

script/upload.py

@@ -8,7 +8,8 @@ import subprocess
 import sys
 import tempfile
 
-from lib.util import *
+from lib.util import get_atom_shell_version, scoped_cwd, safe_mkdir
+from lib.github import GitHub
 
 
 TARGET_PLATFORM = {
@@ -18,6 +19,7 @@ TARGET_PLATFORM = {
   'win32': 'win32',
 }[sys.platform]
 
+ATOM_SHELL_REPO = 'atom/atom-shell'
 ATOM_SHELL_VRESION = get_atom_shell_version()
 NODE_VERSION = 'v0.10.18'
 
@@ -32,18 +34,26 @@ def main():
 
   if not dist_newer_than_head():
     create_dist = os.path.join(SOURCE_ROOT, 'script', 'create-dist.py')
-    subprocess.check_call([sys.executable, create_dist])
+    subprocess.check_output([sys.executable, create_dist])
 
+  # Upload atom-shell with GitHub Releases API.
+  github = GitHub(auth_token())
+  release_id = create_or_get_release_draft(github, args.version)
+  upload_atom_shell(github, release_id, os.path.join(DIST_DIR, DIST_NAME))
+  if not args.no_publish_release:
+    publish_release(github, release_id)
+
+  # Upload node's headers to S3.
   bucket, access_key, secret_key = s3_config()
-  upload(bucket, access_key, secret_key)
-  if not args.no_update_version:
-    update_version(bucket, access_key, secret_key)
+  upload_node(bucket, access_key, secret_key, NODE_VERSION)
 
 
 def parse_args():
   parser = argparse.ArgumentParser(description='upload distribution file')
-  parser.add_argument('-n', '--no-update-version',
-                      help='Do not update the latest version file',
+  parser.add_argument('-v', '--version', help='Specify the version',
+                      default=ATOM_SHELL_VRESION)
+  parser.add_argument('-n', '--no-publish-release',
+                      help='Do not publish the release',
                       action='store_true')
   return parser.parse_args()
 
@@ -62,36 +72,94 @@ def dist_newer_than_head():
   return dist_time > int(head_time)
 
 
-def upload(bucket, access_key, secret_key, version=ATOM_SHELL_VRESION):
+def get_text_with_editor(name):
+  editor = os.environ.get('EDITOR', 'nano')
+  initial_message = '\n# Please enter the body of your release note for %s.' \
+                    % name
+
+  t = tempfile.NamedTemporaryFile(suffix='.tmp', delete=False)
+  t.write(initial_message)
+  t.close()
+  subprocess.call([editor, t.name])
+
+  text = ''
+  for line in open(t.name, 'r'):
+    if len(line) == 0 or line[0] != '#':
+      text += line
+
+  os.unlink(t.name)
+  return text
+
+def create_or_get_release_draft(github, tag):
+  name = 'atom-shell %s' % tag
+  releases = github.repos(ATOM_SHELL_REPO).releases.get()
+  for release in releases:
+    # The untagged commit doesn't have a matching tag_name, so also check name.
+    if release['tag_name'] == tag or release['name'] == name:
+      return release['id']
+
+  return create_release_draft(github, tag)
+
+
+def create_release_draft(github, tag):
+  name = 'atom-shell %s' % tag
+  body = get_text_with_editor(name)
+  if body == '':
+    sys.stderr.write('Quit due to empty release note.\n')
+    sys.exit(0)
+
+  data = dict(tag_name=tag, name=name, body=body, draft=True)
+  r = github.repos(ATOM_SHELL_REPO).releases.post(data=data)
+  return r['id']
+
+
+def upload_atom_shell(github, release_id, file_path):
+  params = {'name': os.path.basename(file_path)}
+  headers = {'Content-Type': 'application/zip'}
+  files = {'file': open(file_path, 'rb')}
+  github.repos(ATOM_SHELL_REPO).releases(release_id).assets.post(
+      params=params, headers=headers, files=files, verify=False)
+
+
+def publish_release(github, release_id):
+  data = dict(draft=False)
+  github.repos(ATOM_SHELL_REPO).releases(release_id).patch(data=data)
+
+
+def upload_node(bucket, access_key, secret_key, version):
   os.chdir(DIST_DIR)
 
+  # TODO(zcbenz): Remove me when Atom starts to use Releases API.
   s3put(bucket, access_key, secret_key, DIST_DIR,
-        'atom-shell/{0}'.format(version), [DIST_NAME])
+        'atom-shell/{0}'.format(ATOM_SHELL_VRESION), [DIST_NAME])
+
   s3put(bucket, access_key, secret_key, DIST_DIR,
-        'atom-shell/dist/{0}'.format(NODE_VERSION), glob.glob('node-*.tar.gz'))
+        'atom-shell/dist/{0}'.format(version), glob.glob('node-*.tar.gz'))
 
   if TARGET_PLATFORM == 'win32':
     # Generate the node.lib.
     build = os.path.join(SOURCE_ROOT, 'script', 'build.py')
-    subprocess.check_call([sys.executable, build, '-c', 'Release',
-                          '-t', 'generate_node_lib'])
+    subprocess.check_output([sys.executable, build, '-c', 'Release',
+                             '-t', 'generate_node_lib'])
 
     # Upload the 32bit node.lib.
     node_lib = os.path.join(OUT_DIR, 'node.lib')
     s3put(bucket, access_key, secret_key, OUT_DIR,
-          'atom-shell/dist/{0}'.format(NODE_VERSION), [node_lib])
+          'atom-shell/dist/{0}'.format(version), [node_lib])
 
     # Upload the fake 64bit node.lib.
     touch_x64_node_lib()
     node_lib = os.path.join(OUT_DIR, 'x64', 'node.lib')
     s3put(bucket, access_key, secret_key, OUT_DIR,
-          'atom-shell/dist/{0}'.format(NODE_VERSION), [node_lib])
+          'atom-shell/dist/{0}'.format(version), [node_lib])
 
 
-def update_version(bucket, access_key, secret_key):
-  prefix = os.path.join(SOURCE_ROOT, 'dist')
-  version = os.path.join(prefix, 'version')
-  s3put(bucket, access_key, secret_key, prefix, 'atom-shell', [version])
+def auth_token():
+  token = os.environ.get('ATOM_SHELL_GITHUB_TOKEN')
+  message = ('Error: Please set the $ATOM_SHELL_GITHUB_TOKEN '
+             'environment variable, which is your personal token')
+  assert token, message
+  return token
 
 
 def s3_config():
@@ -116,7 +184,7 @@ def s3put(bucket, access_key, secret_key, prefix, key_prefix, files):
     '--grant', 'public-read'
   ] + files
 
-  subprocess.check_call(args)
+  subprocess.check_output(args)
 
 
 def touch_x64_node_lib():

spec/api/ipc.coffee

@@ -2,6 +2,9 @@ assert = require 'assert'
 ipc = require 'ipc'
 path = require 'path'
 remote = require 'remote'
+BrowserWindow = remote.require 'browser-window'
+
+fixtures = path.resolve __dirname, '..', 'fixtures'
 
 describe 'ipc', ->
   fixtures = path.join __dirname, '..', 'fixtures'
@@ -49,7 +52,22 @@ describe 'ipc', ->
   describe 'ipc.send', ->
     it 'should work when sending an object containing id property', (done) ->
       obj = id: 1, name: 'ly'
-      ipc.on 'message', (message) ->
+      ipc.once 'message', (message) ->
         assert.deepEqual message, obj
         done()
       ipc.send obj
+
+  describe 'ipc.sendSync', ->
+    it 'can be replied by setting event.returnValue', ->
+      msg = ipc.sendChannelSync 'echo', 'test'
+      assert.equal msg, 'test'
+
+    it 'does not crash when reply is not sent and both browser and event are destroyed', (done) ->
+      w = new BrowserWindow(show: false)
+      remote.require('ipc').once 'send-sync-message', (event) ->
+        event.returnValue = null
+
+        w.destroy()
+        event.destroy()
+        done()
+      w.loadUrl 'file://' + path.join(fixtures, 'api', 'send-sync-message.html')

spec/api/window.coffee

@@ -67,6 +67,14 @@ describe 'window module', ->
         ++count
       w.loadUrl 'about:blank'
 
+  describe 'BrowserWindow.focus()', ->
+    it 'does not make the window become visible', ->
+      w = new BrowserWindow(show: false)
+      assert.equal w.isVisible(), false
+      w.focus()
+      assert.equal w.isVisible(), false
+      w.close()
+
   describe 'beforeunload handler', ->
     it 'returning true would not prevent close', (done) ->
       w = new BrowserWindow(show: false)

spec/fixtures/api/send-sync-message.html

@@ -0,0 +1,9 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  var ipc = require('ipc');
+  ipc.sendChannelSync('send-sync-message', 'message');
+</script>
+</body>
+</html>
+

spec/main.js

@@ -23,7 +23,11 @@ ipc.on('process.exit', function(pid, rid, code) {
 });
 
 ipc.on('eval', function(ev, pid, rid, script) {
-  ev.result = eval(script);
+  ev.returnValue = eval(script);
+});
+
+ipc.on('echo', function(ev, pid, rid, msg) {
+  ev.returnValue = msg;
 });
 
 process.on('uncaughtException', function() {
@@ -35,6 +39,9 @@ app.on('window-all-closed', function() {
 });
 
 app.on('finish-launching', function() {
+  // Test if using protocol module would crash.
+  require('protocol').registerProtocol('test-if-crashes', function() {});
+
   window = new BrowserWindow({
     title: 'atom-shell tests',
     show: false,

spec/web/http.coffee

@@ -0,0 +1,7 @@
+describe 'http', ->
+  describe 'sending request of http protocol urls', ->
+    it 'should not crash', (done) ->
+      $.ajax
+        url: 'http://127.0.0.1'
+        success: -> done()
+        error: -> done()