Defer construction of NotificationUIManager to fix notification initialization.

This is instead of http://codereview.chromium.org/8566054/ and should allow the refactoring code to proceed.

BUG=chromium:103427
TEST=See issue


Review URL: http://codereview.chromium.org/8550003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@110853 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chrome_browser_main.cc

@@ -1843,11 +1843,6 @@ void ChromeBrowserMainParts::StartBrowserOrUITask() {
   // Still initializing, so need to allow IO.
   base::ThreadRestrictions::ScopedAllowIO allow_io;
 
-  // Set the notification UI manager after any desktop initialization in
-  // PreMainMessageLoopRun() is complete, and before starting the browser.
-  DesktopNotificationServiceFactory::GetForProfile(profile_)->SetUIManager(
-      g_browser_process->notification_ui_manager());
-
   // Tests should be able to tune login manager before showing it.
   // Thus only show login manager in normal (non-testing) mode.
   if (!parameters().ui_task)

chrome/browser/notifications/desktop_notification_service.cc

@@ -7,6 +7,7 @@
 #include "base/metrics/histogram.h"
 #include "base/threading/thread.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/content_settings_details.h"
 #include "chrome/browser/content_settings/content_settings_provider.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
@@ -224,12 +225,6 @@ DesktopNotificationService::~DesktopNotificationService() {
   StopObserving();
 }
 
-void DesktopNotificationService::SetUIManager(
-    NotificationUIManager* ui_manager) {
-  DCHECK(!ui_manager_);
-  ui_manager_ = ui_manager;
-}
-
 void DesktopNotificationService::StartObserving() {
   if (!profile_->IsOffTheRecord()) {
     notification_registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
@@ -274,8 +269,8 @@ void DesktopNotificationService::Observe(
     // which was unloaded.
     const Extension* extension =
         content::Details<UnloadedExtensionInfo>(details)->extension;
-    if (extension && ui_manager_)
-      ui_manager_->CancelAllBySourceOrigin(extension->url());
+    if (extension)
+      GetUIManager()->CancelAllBySourceOrigin(extension->url());
   } else if (type == chrome::NOTIFICATION_PROFILE_DESTROYED) {
     StopObserving();
   }
@@ -371,8 +366,7 @@ void DesktopNotificationService::RequestPermission(
 
 void DesktopNotificationService::ShowNotification(
     const Notification& notification) {
-  // ui_manager_ should never be NULL here.
-  ui_manager_->Add(notification, profile_);
+  GetUIManager()->Add(notification, profile_);
 }
 
 bool DesktopNotificationService::CancelDesktopNotification(
@@ -380,9 +374,7 @@ bool DesktopNotificationService::CancelDesktopNotification(
   scoped_refptr<NotificationObjectProxy> proxy(
       new NotificationObjectProxy(process_id, route_id, notification_id,
                                   false));
-  if (ui_manager_)
-    return ui_manager_->CancelById(proxy->id());
-  return false;
+  return GetUIManager()->CancelById(proxy->id());
 }
 
 bool DesktopNotificationService::ShowDesktopNotification(
@@ -431,6 +423,14 @@ void DesktopNotificationService::NotifySettingsChange() {
       content::NotificationService::NoDetails());
 }
 
+NotificationUIManager* DesktopNotificationService::GetUIManager() {
+  // We defer setting ui_manager_ to the global singleton until we need it
+  // in order to avoid UI dependent construction during startup.
+  if (!ui_manager_)
+    ui_manager_ = g_browser_process->notification_ui_manager();
+  return ui_manager_;
+}
+
 WebKit::WebNotificationPresenter::Permission
     DesktopNotificationService::HasPermission(const GURL& origin) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

chrome/browser/notifications/desktop_notification_service.h

@@ -46,9 +46,6 @@ class DesktopNotificationService : public content::NotificationObserver,
                              NotificationUIManager* ui_manager);
   virtual ~DesktopNotificationService();
 
-  // Sets a UI manager to display notifications.
-  void SetUIManager(NotificationUIManager* ui_manager);
-
   // Requests permission (using an info-bar) for a given origin.
   // |callback_context| contains an opaque value to pass back to the
   // requesting process when the info-bar finishes.  A NULL tab can be given if
@@ -139,6 +136,8 @@ class DesktopNotificationService : public content::NotificationObserver,
   // Notifies the observers when permissions settings change.
   void NotifySettingsChange();
 
+  NotificationUIManager* GetUIManager();
+
   // The profile which owns this object.
   Profile* profile_;
 

chrome/test/functional/PYAUTO_TESTS

@@ -94,9 +94,6 @@
       '-indexeddb.IndexedDBTest.testVersionChangeCrashResilience',
       # crbug.com/99506
       '-notifications.NotificationsTest.testSpecialURLNotification',
-      # Cause browser crash.  crbug.com/103427
-      '-notifications.NotificationsTest.testIncognitoNotification',
-      '-notifications.NotificationsTest.testOriginPrefsNotSavedInIncognito',
       # crbug.com/71715
       '-omnibox.OmniboxTest.testHistoryResult',
       # crbug.com/97140