Backporting changes for 1.7.8 (#10586)

* Fix app.makeSingleInstance hanging on posix systems

Wait for the IO thread to be a thing before attempting to listen on the socket

Fixes #9880

* Move OnBrowserReady call to PreMainMessageLoopRun to account for timing issues on macOS

* Woops, how did that happen ;)

* Refactor as per @zcbenz comments

Also fix issue where we run the single instance callback *not* on the UI thread,
this apparently results in a hung process.

* Appease the linting gods

* Create watcher when message loop is ready

* spec: Add test case for app.makeSingleInstance

* Fix missing extension when saving a file without filters

Previously, when triggering the save dialog through e.g. `<a download>`
links (e.g. http://jsfiddle.net/koldev/cW7W5/), the extension was only
saved if Finder was set to show all extensions by default. We now always
display the extension to make sure that it is saved.

If we want to keep the extension hidden, we could also populate the
allowed file types array with the extension from the default filename,
but that would have interfered with how we set the filters.

* Try to make test less flaky

* Try simpler test

* Fix stdout detection

* Try longer timeout on test

atom/browser/api/atom_api_app.cc

@@ -575,6 +575,12 @@ void App::OnFinishLaunching(const base::DictionaryValue& launch_info) {
   Emit("ready", launch_info);
 }
 
+void App::OnPreMainMessageLoopRun() {
+  if (process_singleton_) {
+    process_singleton_->OnBrowserReady();
+  }
+}
+
 void App::OnAccessibilitySupportChanged() {
   Emit("accessibility-support-changed", IsAccessibilitySupportEnabled());
 }

atom/browser/api/atom_api_app.h

@@ -94,6 +94,7 @@ class App : public AtomBrowserClient::Delegate,
   base::FilePath GetAppPath() const;
   void RenderProcessReady(content::RenderProcessHost* host);
   void RenderProcessDisconnected(base::ProcessId host_pid);
+  void PreMainMessageLoopRun();
 
  protected:
   explicit App(v8::Isolate* isolate);
@@ -112,6 +113,7 @@ class App : public AtomBrowserClient::Delegate,
   void OnLogin(LoginHandler* login_handler,
                const base::DictionaryValue& request_details) override;
   void OnAccessibilitySupportChanged() override;
+  void OnPreMainMessageLoopRun() override;
 #if defined(OS_MACOSX)
   void OnContinueUserActivity(
       bool* prevent_default,

atom/browser/atom_browser_main_parts.cc

@@ -4,6 +4,7 @@
 
 #include "atom/browser/atom_browser_main_parts.h"
 
+#include "atom/browser/api/atom_api_app.h"
 #include "atom/browser/api/trackable_object.h"
 #include "atom/browser/atom_access_token_store.h"
 #include "atom/browser/atom_browser_client.h"
@@ -183,6 +184,8 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
   std::unique_ptr<base::DictionaryValue> empty_info(new base::DictionaryValue);
   Browser::Get()->DidFinishLaunching(*empty_info);
 #endif
+
+  Browser::Get()->PreMainMessageLoopRun();
 }
 
 bool AtomBrowserMainParts::MainMessageLoopRun(int* result_code) {

atom/browser/browser.cc

@@ -171,6 +171,12 @@ void Browser::RequestLogin(
     observer.OnLogin(login_handler, *(request_details.get()));
 }
 
+void Browser::PreMainMessageLoopRun() {
+  for (BrowserObserver& observer : observers_) {
+    observer.OnPreMainMessageLoopRun();
+  }
+}
+
 void Browser::NotifyAndShutdown() {
   if (is_shutdown_)
     return;

atom/browser/browser.h

@@ -202,6 +202,8 @@ class Browser : public WindowListObserver {
   void RequestLogin(LoginHandler* login_handler,
                     std::unique_ptr<base::DictionaryValue> request_details);
 
+  void PreMainMessageLoopRun();
+
   void AddObserver(BrowserObserver* obs) {
     observers_.AddObserver(obs);
   }

atom/browser/browser_observer.h

@@ -55,6 +55,9 @@ class BrowserObserver {
   // The browser's accessibility suppport has changed.
   virtual void OnAccessibilitySupportChanged() {}
 
+  // The app message loop is ready
+  virtual void OnPreMainMessageLoopRun() {}
+
 #if defined(OS_MACOSX)
   // The browser wants to resume a user activity via handoff. (macOS only)
   virtual void OnContinueUserActivity(

atom/browser/ui/file_dialog_mac.mm

@@ -40,7 +40,6 @@ void SetAllowedFileTypes(NSSavePanel* dialog, const Filters& filters) {
   if ([file_type_set count])
     file_types = [file_type_set allObjects];
 
-  [dialog setExtensionHidden:NO];
   [dialog setAllowedFileTypes:file_types];
 }
 
@@ -84,11 +83,14 @@ void SetupDialog(NSSavePanel* dialog,
     SetAllowedFileTypes(dialog, settings.filters);
   }
 
+  // Make sure the extension is always visible. Without this, the extension in
+  // the default filename will not be used in the saved file.
+  [dialog setExtensionHidden:NO];
+
   if (default_dir)
     [dialog setDirectoryURL:[NSURL fileURLWithPath:default_dir]];
   if (default_filename)
     [dialog setNameFieldStringValue:default_filename];
-
 }
 
 void SetupDialogForProperties(NSOpenPanel* dialog, int properties) {

chromium_src/chrome/browser/process_singleton.h

@@ -74,6 +74,8 @@ class ProcessSingleton : public base::NonThreadSafe {
   // TODO(brettw): Make the implementation of this method non-platform-specific
   // by making Linux re-use the Windows implementation.
   NotifyResult NotifyOtherProcessOrCreate();
+  void StartListeningOnSocket();
+  void OnBrowserReady();
 
   // Sets ourself up as the singleton instance.  Returns true on success.  If
   // false is returned, we are not the singleton instance and the caller must
@@ -173,6 +175,8 @@ class ProcessSingleton : public base::NonThreadSafe {
   // because it posts messages between threads.
   class LinuxWatcher;
   scoped_refptr<LinuxWatcher> watcher_;
+  int sock_;
+  bool listen_on_ready_ = false;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(ProcessSingleton);

chromium_src/chrome/browser/process_singleton_posix.cc

@@ -55,6 +55,7 @@
 
 #include <stddef.h>
 
+#include "atom/browser/browser.h"
 #include "atom/common/atom_command_line.h"
 
 #include "base/base_paths.h"
@@ -719,8 +720,7 @@ ProcessSingleton::ProcessSingleton(
     const base::FilePath& user_data_dir,
     const NotificationCallback& notification_callback)
     : notification_callback_(notification_callback),
-      current_pid_(base::GetCurrentProcId()),
-      watcher_(new LinuxWatcher(this)) {
+      current_pid_(base::GetCurrentProcId()) {
   // The user_data_dir may have not been created yet.
   base::CreateDirectoryAndGetError(user_data_dir, nullptr);
 
@@ -881,6 +881,23 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() {
       base::TimeDelta::FromSeconds(kTimeoutInSeconds));
 }
 
+void ProcessSingleton::StartListeningOnSocket() {
+  watcher_ = new LinuxWatcher(this);
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&ProcessSingleton::LinuxWatcher::StartListening,
+                 watcher_,
+                 sock_));
+}
+
+void ProcessSingleton::OnBrowserReady() {
+  if (listen_on_ready_) {
+    StartListeningOnSocket();
+    listen_on_ready_ = false;
+  }
+}
+
 ProcessSingleton::NotifyResult
 ProcessSingleton::NotifyOtherProcessWithTimeoutOrCreate(
     const base::CommandLine& command_line,
@@ -1031,13 +1048,13 @@ bool ProcessSingleton::Create() {
   if (listen(sock, 5) < 0)
     NOTREACHED() << "listen failed: " << base::safe_strerror(errno);
 
-  DCHECK(BrowserThread::IsMessageLoopValid(BrowserThread::IO));
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&ProcessSingleton::LinuxWatcher::StartListening,
-                 watcher_,
-                 sock));
+  sock_ = sock;
+
+  if (BrowserThread::IsMessageLoopValid(BrowserThread::IO)) {
+    StartListeningOnSocket();
+  } else {
+    listen_on_ready_ = true;
+  }
 
   return true;
 }

chromium_src/chrome/browser/process_singleton_win.cc

@@ -258,6 +258,9 @@ ProcessSingleton::NotifyOtherProcessOrCreate() {
   return result;
 }
 
+void ProcessSingleton::StartListeningOnSocket() {}
+void ProcessSingleton::OnBrowserReady() {}
+
 // Look for a Chrome instance that uses the same profile directory. If there
 // isn't one, create a message window with its title set to the profile
 // directory path.

spec/api-app-spec.js

@@ -149,6 +149,34 @@ describe('app module', function () {
     })
   })
 
+  describe('app.makeSingleInstance', function () {
+    it('prevents the second launch of app', function (done) {
+      this.timeout(120000)
+      const appPath = path.join(__dirname, 'fixtures', 'api', 'singleton')
+      // First launch should exit with 0.
+      let secondLaunched = false
+      const first = ChildProcess.spawn(remote.process.execPath, [appPath])
+      let launchOnce = true
+      first.stdout.on('data', (data) => {
+        if (data.toString().trim() === 'launched' && launchOnce) {
+          launchOnce = false
+          // Second launch should exit with 1.
+          const second = ChildProcess.spawn(remote.process.execPath, [appPath])
+          second.once('exit', (code) => {
+            assert.ok(!secondLaunched)
+            assert.equal(code, 1)
+            secondLaunched = true
+          })
+        }
+      })
+      first.once('exit', (code) => {
+        assert.ok(secondLaunched)
+        assert.equal(code, 0)
+        done()
+      })
+    })
+  })
+
   describe('app.relaunch', function () {
     let server = null
     const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-app-relaunch' : '/tmp/electron-app-relaunch'
@@ -211,6 +239,7 @@ describe('app module', function () {
   describe('app.importCertificate', function () {
     if (process.platform !== 'linux') return
 
+    this.timeout(120000)
     var w = null
 
     afterEach(function () {

spec/fixtures/api/singleton/main.js

@@ -0,0 +1,15 @@
+const {app} = require('electron')
+
+console.log('launched')
+
+process.on('uncaughtException', () => {
+  app.exit(2)
+})
+
+const shouldExit = app.makeSingleInstance(() => {
+  process.nextTick(() => app.exit(0))
+})
+
+if (shouldExit) {
+  app.exit(1)
+}

spec/fixtures/api/singleton/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "electron-app-singleton",
+  "main": "main.js"
+}
+