Merge pull request #3356 from atom/process-exit

Make process.exit() quit program gracefully

atom/browser/api/atom_api_app.cc

@@ -339,6 +339,7 @@ mate::ObjectTemplateBuilder App::GetObjectTemplateBuilder(
   auto browser = base::Unretained(Browser::Get());
   return mate::ObjectTemplateBuilder(isolate)
       .SetMethod("quit", base::Bind(&Browser::Quit, browser))
+      .SetMethod("exit", base::Bind(&Browser::Exit, browser))
       .SetMethod("focus", base::Bind(&Browser::Focus, browser))
       .SetMethod("getVersion", base::Bind(&Browser::GetVersion, browser))
       .SetMethod("setVersion", base::Bind(&Browser::SetVersion, browser))

atom/browser/api/lib/app.coffee

@@ -50,7 +50,6 @@ app.getAppPath = ->
 # Be compatible with old API.
 app.once 'ready', -> @emit 'finish-launching'
 app.terminate = app.quit
-app.exit = process.exit
 app.getHomeDir = -> @getPath 'home'
 app.getDataPath = -> @getPath 'userData'
 app.setDataPath = (path) -> @setPath 'userData', path

atom/browser/atom_browser_main_parts.cc

@@ -30,6 +30,7 @@ AtomBrowserMainParts* AtomBrowserMainParts::self_ = NULL;
 
 AtomBrowserMainParts::AtomBrowserMainParts()
     : fake_browser_process_(new BrowserProcess),
+      exit_code_(nullptr),
       browser_(new Browser),
       node_bindings_(NodeBindings::Create(true)),
       atom_bindings_(new AtomBindings),
@@ -47,6 +48,14 @@ AtomBrowserMainParts* AtomBrowserMainParts::Get() {
   return self_;
 }
 
+bool AtomBrowserMainParts::SetExitCode(int code) {
+  if (!exit_code_)
+    return false;
+
+  *exit_code_ = code;
+  return true;
+}
+
 void AtomBrowserMainParts::RegisterDestructionCallback(
     const base::Closure& callback) {
   destruction_callbacks_.push_back(callback);
@@ -118,6 +127,11 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
 #endif
 }
 
+bool AtomBrowserMainParts::MainMessageLoopRun(int* result_code) {
+  exit_code_ = result_code;
+  return brightray::BrowserMainParts::MainMessageLoopRun(result_code);
+}
+
 void AtomBrowserMainParts::PostMainMessageLoopStart() {
   brightray::BrowserMainParts::PostMainMessageLoopStart();
 #if defined(OS_POSIX)

atom/browser/atom_browser_main_parts.h

@@ -31,6 +31,9 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
 
   static AtomBrowserMainParts* Get();
 
+  // Sets the exit code, will fail if the the message loop is not ready.
+  bool SetExitCode(int code);
+
   // Register a callback that should be destroyed before JavaScript environment
   // gets destroyed.
   void RegisterDestructionCallback(const base::Closure& callback);
@@ -42,6 +45,7 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
   void PreEarlyInitialization() override;
   void PostEarlyInitialization() override;
   void PreMainMessageLoopRun() override;
+  bool MainMessageLoopRun(int* result_code) override;
   void PostMainMessageLoopStart() override;
   void PostMainMessageLoopRun() override;
 #if defined(OS_MACOSX)
@@ -66,6 +70,9 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
   // with a task runner that will post all work to main loop.
   scoped_refptr<BridgeTaskRunner> bridge_task_runner_;
 
+  // Pointer to exit code.
+  int* exit_code_;
+
   scoped_ptr<Browser> browser_;
   scoped_ptr<JavascriptEnvironment> js_env_;
   scoped_ptr<NodeBindings> node_bindings_;

atom/browser/browser.cc

@@ -7,6 +7,7 @@
 #include <string>
 
 #include "atom/browser/atom_browser_main_parts.h"
+#include "atom/browser/native_window.h"
 #include "atom/browser/window_list.h"
 #include "base/message_loop/message_loop.h"
 #include "content/public/browser/client_certificate_delegate.h"
@@ -45,6 +46,27 @@ void Browser::Quit() {
   window_list->CloseAllWindows();
 }
 
+void Browser::Exit(int code) {
+  if (!AtomBrowserMainParts::Get()->SetExitCode(code)) {
+    // Message loop is not ready, quit directly.
+    exit(code);
+  } else {
+    // Prepare to quit when all windows have been closed..
+    is_quiting_ = true;
+
+    // Must destroy windows before quitting, otherwise bad things can happen.
+    atom::WindowList* window_list = atom::WindowList::GetInstance();
+    if (window_list->size() == 0) {
+      NotifyAndShutdown();
+    } else {
+      // Unlike Quit(), we do not ask to close window, but destroy the window
+      // without asking.
+      for (NativeWindow* window : *window_list)
+        window->CloseContents(nullptr);  // e.g. Destroy()
+    }
+  }
+}
+
 void Browser::Shutdown() {
   if (is_shutdown_)
     return;

atom/browser/browser.h

@@ -42,6 +42,9 @@ class Browser : public WindowListObserver {
   // Try to close all windows and quit the application.
   void Quit();
 
+  // Exit the application immediately and set exit code.
+  void Exit(int code);
+
   // Cleanup everything and shutdown the application gracefully.
   void Shutdown();
 

atom/browser/lib/init.coffee

@@ -53,6 +53,9 @@ app = require 'app'
 app.on 'quit', ->
   process.emit 'exit'
 
+# Map process.exit to app.exit, which quits gracefully.
+process.exit = app.exit
+
 # Load the RPC server.
 require './rpc-server'
 

atom/common/native_mate_converters/callback.cc

@@ -4,6 +4,8 @@
 
 #include "atom/common/native_mate_converters/callback.h"
 
+#include "atom/browser/atom_browser_main_parts.h"
+
 namespace mate {
 
 namespace internal {
@@ -54,6 +56,33 @@ v8::Local<v8::Value> BindFunctionWith(v8::Isolate* isolate,
 
 }  // namespace
 
+SafeV8Function::SafeV8Function(v8::Isolate* isolate, v8::Local<v8::Value> value)
+    : v8_function_(new RefCountedPersistent<v8::Function>(isolate, value)),
+      weak_factory_(this) {
+  Init();
+}
+
+SafeV8Function::SafeV8Function(const SafeV8Function& other)
+    : v8_function_(other.v8_function_),
+      weak_factory_(this) {
+  Init();
+}
+
+v8::Local<v8::Function> SafeV8Function::NewHandle() const {
+  return v8_function_->NewHandle();
+}
+
+void SafeV8Function::Init() {
+  if (Locker::IsBrowserProcess() && atom::AtomBrowserMainParts::Get())
+    atom::AtomBrowserMainParts::Get()->RegisterDestructionCallback(
+        base::Bind(&SafeV8Function::FreeHandle, weak_factory_.GetWeakPtr()));
+}
+
+void SafeV8Function::FreeHandle() {
+  Locker locker(v8_function_->isolate());
+  v8_function_ = nullptr;
+}
+
 v8::Local<v8::Value> CreateFunctionFromTranslater(
     v8::Isolate* isolate, const Translater& translater) {
   // The FunctionTemplate is cached.

atom/common/native_mate_converters/callback.h

@@ -10,6 +10,7 @@
 #include "atom/common/api/locker.h"
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/memory/weak_ptr.h"
 #include "native_mate/function_template.h"
 #include "native_mate/scoped_persistent.h"
 #include "third_party/WebKit/public/web/WebScopedMicrotaskSuppression.h"
@@ -18,7 +19,24 @@ namespace mate {
 
 namespace internal {
 
-typedef scoped_refptr<RefCountedPersistent<v8::Function> > SafeV8Function;
+// Manages the V8 function with RAII, and automatically cleans the handle when
+// JavaScript context is destroyed, even when the class is not destroyed.
+class SafeV8Function {
+ public:
+  SafeV8Function(v8::Isolate* isolate, v8::Local<v8::Value> value);
+  SafeV8Function(const SafeV8Function& other);
+
+  bool is_alive() const { return v8_function_.get(); }
+
+  v8::Local<v8::Function> NewHandle() const;
+
+ private:
+  void Init();
+  void FreeHandle();
+
+  scoped_refptr<RefCountedPersistent<v8::Function>> v8_function_;
+  base::WeakPtrFactory<SafeV8Function> weak_factory_;
+};
 
 // Helper to invoke a V8 function with C++ parameters.
 template <typename Sig>
@@ -26,14 +44,17 @@ struct V8FunctionInvoker {};
 
 template <typename... ArgTypes>
 struct V8FunctionInvoker<v8::Local<v8::Value>(ArgTypes...)> {
-  static v8::Local<v8::Value> Go(v8::Isolate* isolate, SafeV8Function function,
+  static v8::Local<v8::Value> Go(v8::Isolate* isolate,
+                                 const SafeV8Function& function,
                                  ArgTypes... raw) {
     Locker locker(isolate);
     v8::EscapableHandleScope handle_scope(isolate);
+    if (!function.is_alive())
+      return v8::Null(isolate);
     scoped_ptr<blink::WebScopedRunV8Script> script_scope(
         Locker::IsBrowserProcess() ?
         nullptr : new blink::WebScopedRunV8Script(isolate));
-    v8::Local<v8::Function> holder = function->NewHandle();
+    v8::Local<v8::Function> holder = function.NewHandle();
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
     std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };
@@ -44,14 +65,17 @@ struct V8FunctionInvoker<v8::Local<v8::Value>(ArgTypes...)> {
 
 template <typename... ArgTypes>
 struct V8FunctionInvoker<void(ArgTypes...)> {
-  static void Go(v8::Isolate* isolate, SafeV8Function function,
+  static void Go(v8::Isolate* isolate,
+                 const SafeV8Function& function,
                  ArgTypes... raw) {
     Locker locker(isolate);
     v8::HandleScope handle_scope(isolate);
+    if (!function.is_alive())
+      return;
     scoped_ptr<blink::WebScopedRunV8Script> script_scope(
         Locker::IsBrowserProcess() ?
         nullptr : new blink::WebScopedRunV8Script(isolate));
-    v8::Local<v8::Function> holder = function->NewHandle();
+    v8::Local<v8::Function> holder = function.NewHandle();
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
     std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };
@@ -61,17 +85,20 @@ struct V8FunctionInvoker<void(ArgTypes...)> {
 
 template <typename ReturnType, typename... ArgTypes>
 struct V8FunctionInvoker<ReturnType(ArgTypes...)> {
-  static ReturnType Go(v8::Isolate* isolate, SafeV8Function function,
+  static ReturnType Go(v8::Isolate* isolate,
+                       const SafeV8Function& function,
                        ArgTypes... raw) {
     Locker locker(isolate);
     v8::HandleScope handle_scope(isolate);
+    ReturnType ret = ReturnType();
+    if (!function.is_alive())
+      return ret;
     scoped_ptr<blink::WebScopedRunV8Script> script_scope(
         Locker::IsBrowserProcess() ?
         nullptr : new blink::WebScopedRunV8Script(isolate));
-    v8::Local<v8::Function> holder = function->NewHandle();
+    v8::Local<v8::Function> holder = function.NewHandle();
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
-    ReturnType ret = ReturnType();
     std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };
     v8::Local<v8::Value> result;
     auto maybe_result =
@@ -119,9 +146,8 @@ struct Converter<base::Callback<Sig>> {
     if (!val->IsFunction())
       return false;
 
-    internal::SafeV8Function function(
-        new RefCountedPersistent<v8::Function>(isolate, val));
-    *out = base::Bind(&internal::V8FunctionInvoker<Sig>::Go, isolate, function);
+    *out = base::Bind(&internal::V8FunctionInvoker<Sig>::Go,
+                      isolate, internal::SafeV8Function(isolate, val));
     return true;
   }
 };

atom/common/node_bindings.cc

@@ -14,6 +14,7 @@
 #include "atom/common/node_includes.h"
 #include "base/command_line.h"
 #include "base/base_paths.h"
+#include "base/environment.h"
 #include "base/files/file_path.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
@@ -141,6 +142,14 @@ void NodeBindings::Initialize() {
   // Init node.
   // (we assume node::Init would not modify the parameters under embedded mode).
   node::Init(nullptr, nullptr, nullptr, nullptr);
+
+#if defined(OS_WIN)
+  // uv_init overrides error mode to suppress the default crash dialog, bring
+  // it back if user wants to show it.
+  scoped_ptr<base::Environment> env(base::Environment::Create());
+  if (env->HasVar("ELECTRON_DEFAULT_ERROR_MODE"))
+    SetErrorMode(0);
+#endif
 }
 
 node::Environment* NodeBindings::CreateEnvironment(

docs/api/app.md

@@ -208,6 +208,15 @@ This method guarantees that all `beforeunload` and `unload` event handlers are
 correctly executed. It is possible that a window cancels the quitting by
 returning `false` in the `beforeunload` event handler.
 
+### `app.exit(exitCode)`
+
+* `exitCode` Integer
+
+Exits immediately with `exitCode`.
+
+All windows will be closed immediately without asking user and the `before-quit`
+and `will-quit` events will not be emitted.
+
 ### `app.getAppPath()`
 
 Returns the current application directory.

spec/api-crash-reporter-spec.coffee

@@ -53,5 +53,6 @@ describe 'crash-reporter module', ->
         protocol: 'file'
         pathname: path.join fixtures, 'api', 'crash.html'
         search: "?port=#{port}"
-      crashReporter.start {'submitUrl': 'http://127.0.0.1:' + port}
+      if process.platform is 'darwin'
+        crashReporter.start {'submitUrl': 'http://127.0.0.1:' + port}
       w.loadUrl url