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.
This commit is contained in:
		
					parent
					
						
							
								ecbeb0d117
							
						
					
				
			
			
				commit
				
					
						0ab83b301d
					
				
			
		
					 7 changed files with 22 additions and 16 deletions
				
			
		| 
						 | 
					@ -407,7 +407,12 @@ bool NotificationCallbackWrapper(
 | 
				
			||||||
    const base::FilePath& cwd) {
 | 
					    const base::FilePath& cwd) {
 | 
				
			||||||
  // Make sure the callback is called after app gets ready.
 | 
					  // Make sure the callback is called after app gets ready.
 | 
				
			||||||
  if (Browser::Get()->is_ready()) {
 | 
					  if (Browser::Get()->is_ready()) {
 | 
				
			||||||
    callback.Run(cmd, cwd);
 | 
					    // We definitely want to call this callback on the UI thread
 | 
				
			||||||
 | 
					    content::BrowserThread::PostTask(
 | 
				
			||||||
 | 
					      content::BrowserThread::UI,
 | 
				
			||||||
 | 
					      FROM_HERE,
 | 
				
			||||||
 | 
					      base::Bind(base::IgnoreResult(callback), cmd, cwd)
 | 
				
			||||||
 | 
					    );
 | 
				
			||||||
  } else {
 | 
					  } else {
 | 
				
			||||||
    scoped_refptr<base::SingleThreadTaskRunner> task_runner(
 | 
					    scoped_refptr<base::SingleThreadTaskRunner> task_runner(
 | 
				
			||||||
        base::ThreadTaskRunnerHandle::Get());
 | 
					        base::ThreadTaskRunnerHandle::Get());
 | 
				
			||||||
| 
						 | 
					@ -522,7 +527,6 @@ App::App(v8::Isolate* isolate) {
 | 
				
			||||||
          base::ProcessMetrics::CreateCurrentProcessMetrics()));
 | 
					          base::ProcessMetrics::CreateCurrentProcessMetrics()));
 | 
				
			||||||
  app_metrics_[pid] = std::move(process_metric);
 | 
					  app_metrics_[pid] = std::move(process_metric);
 | 
				
			||||||
  Init(isolate);
 | 
					  Init(isolate);
 | 
				
			||||||
  App::self_ = this;
 | 
					 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
App::~App() {
 | 
					App::~App() {
 | 
				
			||||||
| 
						 | 
					@ -533,12 +537,6 @@ App::~App() {
 | 
				
			||||||
  content::BrowserChildProcessObserver::Remove(this);
 | 
					  content::BrowserChildProcessObserver::Remove(this);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
App* App::self_ = nullptr;
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
App* App::Get() {
 | 
					 | 
				
			||||||
  return App::self_;
 | 
					 | 
				
			||||||
}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
void App::OnBeforeQuit(bool* prevent_default) {
 | 
					void App::OnBeforeQuit(bool* prevent_default) {
 | 
				
			||||||
  *prevent_default = Emit("before-quit");
 | 
					  *prevent_default = Emit("before-quit");
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					@ -586,7 +584,7 @@ void App::OnFinishLaunching(const base::DictionaryValue& launch_info) {
 | 
				
			||||||
  Emit("ready", launch_info);
 | 
					  Emit("ready", launch_info);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
void App::PreMainMessageLoopRun() {
 | 
					void App::OnPreMainMessageLoopRun() {
 | 
				
			||||||
  if (process_singleton_) {
 | 
					  if (process_singleton_) {
 | 
				
			||||||
    process_singleton_->OnBrowserReady();
 | 
					    process_singleton_->OnBrowserReady();
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -74,8 +74,6 @@ class App : public AtomBrowserClient::Delegate,
 | 
				
			||||||
  static void BuildPrototype(v8::Isolate* isolate,
 | 
					  static void BuildPrototype(v8::Isolate* isolate,
 | 
				
			||||||
                             v8::Local<v8::FunctionTemplate> prototype);
 | 
					                             v8::Local<v8::FunctionTemplate> prototype);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  static App* Get();
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
  // Called when window with disposition needs to be created.
 | 
					  // Called when window with disposition needs to be created.
 | 
				
			||||||
  void OnCreateWindow(
 | 
					  void OnCreateWindow(
 | 
				
			||||||
      const GURL& target_url,
 | 
					      const GURL& target_url,
 | 
				
			||||||
| 
						 | 
					@ -101,7 +99,6 @@ class App : public AtomBrowserClient::Delegate,
 | 
				
			||||||
 protected:
 | 
					 protected:
 | 
				
			||||||
  explicit App(v8::Isolate* isolate);
 | 
					  explicit App(v8::Isolate* isolate);
 | 
				
			||||||
  ~App() override;
 | 
					  ~App() override;
 | 
				
			||||||
  static App* self_;
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
  // BrowserObserver:
 | 
					  // BrowserObserver:
 | 
				
			||||||
  void OnBeforeQuit(bool* prevent_default) override;
 | 
					  void OnBeforeQuit(bool* prevent_default) override;
 | 
				
			||||||
| 
						 | 
					@ -116,6 +113,7 @@ class App : public AtomBrowserClient::Delegate,
 | 
				
			||||||
  void OnLogin(LoginHandler* login_handler,
 | 
					  void OnLogin(LoginHandler* login_handler,
 | 
				
			||||||
               const base::DictionaryValue& request_details) override;
 | 
					               const base::DictionaryValue& request_details) override;
 | 
				
			||||||
  void OnAccessibilitySupportChanged() override;
 | 
					  void OnAccessibilitySupportChanged() override;
 | 
				
			||||||
 | 
					  void OnPreMainMessageLoopRun() override;
 | 
				
			||||||
#if defined(OS_MACOSX)
 | 
					#if defined(OS_MACOSX)
 | 
				
			||||||
  void OnWillContinueUserActivity(
 | 
					  void OnWillContinueUserActivity(
 | 
				
			||||||
      bool* prevent_default,
 | 
					      bool* prevent_default,
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -185,7 +185,7 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
 | 
				
			||||||
  Browser::Get()->DidFinishLaunching(*empty_info);
 | 
					  Browser::Get()->DidFinishLaunching(*empty_info);
 | 
				
			||||||
#endif
 | 
					#endif
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  atom::api::App::Get()->PreMainMessageLoopRun();
 | 
					  Browser::Get()->PreMainMessageLoopRun();
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
bool AtomBrowserMainParts::MainMessageLoopRun(int* result_code) {
 | 
					bool AtomBrowserMainParts::MainMessageLoopRun(int* result_code) {
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -171,6 +171,12 @@ void Browser::RequestLogin(
 | 
				
			||||||
    observer.OnLogin(login_handler, *(request_details.get()));
 | 
					    observer.OnLogin(login_handler, *(request_details.get()));
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					void Browser::PreMainMessageLoopRun() {
 | 
				
			||||||
 | 
					  for (BrowserObserver& observer : observers_) {
 | 
				
			||||||
 | 
					    observer.OnPreMainMessageLoopRun();
 | 
				
			||||||
 | 
					  }
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
void Browser::NotifyAndShutdown() {
 | 
					void Browser::NotifyAndShutdown() {
 | 
				
			||||||
  if (is_shutdown_)
 | 
					  if (is_shutdown_)
 | 
				
			||||||
    return;
 | 
					    return;
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -224,6 +224,8 @@ class Browser : public WindowListObserver {
 | 
				
			||||||
  void RequestLogin(LoginHandler* login_handler,
 | 
					  void RequestLogin(LoginHandler* login_handler,
 | 
				
			||||||
                    std::unique_ptr<base::DictionaryValue> request_details);
 | 
					                    std::unique_ptr<base::DictionaryValue> request_details);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  void PreMainMessageLoopRun();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  void AddObserver(BrowserObserver* obs) {
 | 
					  void AddObserver(BrowserObserver* obs) {
 | 
				
			||||||
    observers_.AddObserver(obs);
 | 
					    observers_.AddObserver(obs);
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -55,6 +55,9 @@ class BrowserObserver {
 | 
				
			||||||
  // The browser's accessibility suppport has changed.
 | 
					  // The browser's accessibility suppport has changed.
 | 
				
			||||||
  virtual void OnAccessibilitySupportChanged() {}
 | 
					  virtual void OnAccessibilitySupportChanged() {}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  // The app message loop is ready
 | 
				
			||||||
 | 
					  virtual void OnPreMainMessageLoopRun() {}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
#if defined(OS_MACOSX)
 | 
					#if defined(OS_MACOSX)
 | 
				
			||||||
  // The browser wants to report that an user activity will resume. (macOS only)
 | 
					  // The browser wants to report that an user activity will resume. (macOS only)
 | 
				
			||||||
  virtual void OnWillContinueUserActivity(
 | 
					  virtual void OnWillContinueUserActivity(
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1048,10 +1048,9 @@ bool ProcessSingleton::Create() {
 | 
				
			||||||
  if (listen(sock, 5) < 0)
 | 
					  if (listen(sock, 5) < 0)
 | 
				
			||||||
    NOTREACHED() << "listen failed: " << base::safe_strerror(errno);
 | 
					    NOTREACHED() << "listen failed: " << base::safe_strerror(errno);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  DCHECK(BrowserThread::IsMessageLoopValid(BrowserThread::IO));
 | 
					 | 
				
			||||||
  sock_ = sock;
 | 
					  sock_ = sock;
 | 
				
			||||||
  
 | 
					  
 | 
				
			||||||
  if (atom::Browser::Get()->is_ready()) {
 | 
					  if (BrowserThread::IsMessageLoopValid(BrowserThread::IO)) {
 | 
				
			||||||
    StartListeningOnSocket();
 | 
					    StartListeningOnSocket();
 | 
				
			||||||
  } else {
 | 
					  } else {
 | 
				
			||||||
    listen_on_ready_ = true;
 | 
					    listen_on_ready_ = true;
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue