Defer construction of NotificationUIManager to fix notification initialization.

chrome/browser/notifications/desktop_notification_service.cc

 // Copyright (c) 2011 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/notifications/desktop_notification_service.h"
 
 #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"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
 #include "chrome/browser/notifications/desktop_notification_service_factory.h"
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_object_proxy.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ skipping 186 matching lines @@
   if (template_html.empty()) {
     NOTREACHED() << "unable to load template. ID: " << resource;
     return string16();
   }
 
   std::string data = ReplaceStringPlaceholders(template_html, subst, NULL);
   return UTF8ToUTF16("data:text/html;charset=utf-8," +
                       net::EscapeQueryParamValue(data, false));
 }
 
 DesktopNotificationService::DesktopNotificationService(Profile* profile,

[jam, 2011/11/19 01:18:32]

can we make this take a single argument now?

[stevenjb, 2011/11/19 01:24:20]

Unfortunately no. We do construct locally a NotificationUIManager and pass it in
for tests.

     NotificationUIManager* ui_manager)
     : profile_(profile),
       ui_manager_(ui_manager) {
   StartObserving();
 }
 
 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,
                                 content::Source<Profile>(profile_));
   }
   notification_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
                               content::Source<Profile>(profile_));
 }
 
 void DesktopNotificationService::StopObserving() {
@@ skipping 24 matching lines @@
 
 void DesktopNotificationService::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   if (type == chrome::NOTIFICATION_EXTENSION_UNLOADED) {
     // Remove all notifications currently shown or queued by the extension
     // 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();
   }
 }
 
 ContentSetting DesktopNotificationService::GetDefaultContentSetting(
     std::string* provider_id) {
   return profile_->GetHostContentSettingsMap()->GetDefaultContentSetting(
       CONTENT_SETTINGS_TYPE_NOTIFICATIONS, provider_id);
 }
@@ skipping 75 matching lines @@
   } else {
     // Notify renderer immediately.
     RenderViewHost* host = RenderViewHost::FromID(process_id, route_id);
     if (host)
       host->DesktopNotificationPermissionRequestDone(callback_context);
   }
 }
 
 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(
     int process_id, int route_id, int notification_id) {
   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(
     const content::ShowDesktopNotificationHostMsgParams& params,
     int process_id, int route_id, DesktopNotificationSource source) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   const GURL& origin = params.origin;
   NotificationObjectProxy* proxy =
       new NotificationObjectProxy(process_id, route_id,
                                   params.notification_id,
@@ skipping 28 matching lines @@
   return UTF8ToUTF16(origin.host());
 }
 
 void DesktopNotificationService::NotifySettingsChange() {
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_DESKTOP_NOTIFICATION_SETTINGS_CHANGED,
       content::Source<DesktopNotificationService>(this),
       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_)

[jam, 2011/11/19 01:18:32]

this member variable, and even this function, seem unnecessary

[stevenjb, 2011/11/19 01:24:20]

Tests might set this in which case we do not want to override it.

+    ui_manager_ = g_browser_process->notification_ui_manager();
+  return ui_manager_;
+}
+
 WebKit::WebNotificationPresenter::Permission
     DesktopNotificationService::HasPermission(const GURL& origin) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   HostContentSettingsMap* host_content_settings_map =
       profile_->GetHostContentSettingsMap();
   ContentSetting setting = host_content_settings_map->GetContentSetting(
       origin,
       origin,
       CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
       NO_RESOURCE_IDENTIFIER);
 
   if (setting == CONTENT_SETTING_ALLOW)
     return WebKit::WebNotificationPresenter::PermissionAllowed;
   if (setting == CONTENT_SETTING_BLOCK)
     return WebKit::WebNotificationPresenter::PermissionDenied;
   if (setting == CONTENT_SETTING_ASK)
     return WebKit::WebNotificationPresenter::PermissionNotAllowed;
   NOTREACHED() << "Invalid notifications settings value: " << setting;
   return WebKit::WebNotificationPresenter::PermissionNotAllowed;
 }