Move the window destruction and registration out of cleanup and into BrowserProcessImpl::EndSession…

chrome/browser/browser_process_impl.cc

 // 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.
 
 #include "chrome/browser/browser_process_impl.h"
 
 #include <algorithm>
 #include <map>
 #include <set>
 #include <vector>
@@ skipping 32 matching lines @@
 #include "chrome/browser/net/sdch_dictionary_fetcher.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/policy/policy_service.h"
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prerender/prerender_tracker.h"
 #include "chrome/browser/printing/background_printing_manager.h"
 #include "chrome/browser/printing/print_job_manager.h"
 #include "chrome/browser/printing/print_preview_tab_controller.h"
+#include "chrome/browser/process_singleton.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/status_icons/status_tray.h"
 #include "chrome/browser/tab_closeable_state_watcher.h"
 #include "chrome/browser/tab_contents/thumbnail_generator.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_content_client.h"
@@ skipping 56 matching lines @@
 // Linux. We have a timeout here because we're unable to run the UI messageloop
 // and there's some deadlock risk. Our only option is to exit anyway.
 static const int kEndSessionTimeoutSeconds = 10;
 #endif
 
 using content::BrowserThread;
 using content::ChildProcessSecurityPolicy;
 using content::PluginService;
 using content::ResourceDispatcherHost;
 
-BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line)
+BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line,
+                                       ProcessSingleton* process_singleton)
     : created_metrics_service_(false),
       created_watchdog_thread_(false),
       created_browser_policy_connector_(false),
       created_profile_manager_(false),
       created_local_state_(false),
       created_icon_manager_(false),
       created_notification_ui_manager_(false),
       created_safe_browsing_service_(false),
       module_ref_count_(0),
       did_start_(false),
       checked_for_new_frames_(false),
       using_new_frames_(false),
       thumbnail_generator_(new ThumbnailGenerator),
-      download_status_updater_(new DownloadStatusUpdater) {
+      download_status_updater_(new DownloadStatusUpdater),
+      process_singleton_(process_singleton) {
   g_browser_process = this;
   clipboard_.reset(new ui::Clipboard);
 
 #if defined(ENABLE_PRINTING)
   // Must be created after the NotificationService.
   print_job_manager_.reset(new printing::PrintJobManager);
 #endif
 
   net_log_.reset(new ChromeNetLog);
 
@@ skipping 185 matching lines @@
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
       base::Bind(PostQuit, MessageLoop::current()));
   int quits_received = 0;
   do {
     MessageLoop::current()->Run();
     ++quits_received;
   } while (!g_end_session_file_thread_has_completed);
   // If we did get extra quits, then we should re-post them to the message loop.
   while (--quits_received > 0)
     MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+
+  DCHECK(process_singleton_);

[Jói, 2012/06/14 10:32:49]

This only checks that the pointer is not NULL (which you might as well DCHECK at
construction time); it does not guarantee that the object pointed to is still
around.

I would suggest moving the DCHECK to the constructor.

Here, you could have an invariant of data like this:

// BrowserProcess::EndSession is only called in response to a window
// message sent by Windows when the user logs out or shuts down.
// When this occurs, all of Chrome is still running, so the pointer
// to process_singleton_ should still be valid.

[rpetterson, 2012/06/14 22:12:50]

Done.

+  process_singleton_->UnregisterWindowClass();

[Jói, 2012/06/14 10:32:49]

BrowserProcess::EndSession is only called when the user logs off or shuts down
the machine.  We need to make sure we also unregister the window class in the
normal shutdown case (e.g. Alt+F4).

I think this is as simple as also calling UnregisterWindowClass in the
destructor of ProcessSingleton.

[rpetterson, 2012/06/14 22:12:50]

Done.

 #else
   NOTIMPLEMENTED();
 #endif
 }
 
 MetricsService* BrowserProcessImpl::metrics_service() {
   DCHECK(CalledOnValidThread());
   if (!created_metrics_service_)
     CreateMetricsService();
   return metrics_service_.get();
@@ skipping 575 matching lines @@
 }
 
 void BrowserProcessImpl::OnAutoupdateTimer() {
   if (CanAutorestartForUpdate()) {
     DLOG(WARNING) << "Detected update.  Restarting browser.";
     RestartBackgroundInstance();
   }
 }
 
 #endif  // (defined(OS_WIN) || defined(OS_LINUX)) && !defined(OS_CHROMEOS)