Remove NotifierGroup.

{Profile}NotifierGroup had one purpose, which was to separate
notification settings from different profiles on Win-ash. ChromeOS
doesn't mix notification settings from different profiles and only shows
those from the active user, so they're not needed.

MessageCenterSettingsController is only used by NotifierSettingsView,
which now only compiles on ash/. Hence it can be ChromeOS only now
(although it's still difficult to move to ash/). This class gets a lot
simpler.

Other random cleanups:
- move more code into NotifierController
- cut NotificationUiManager out of the settings provider initialization
  loop; instead set it directly on MessageCenter.

Change-Id: Ib9a60e0eb3c167513eba555938369bad3f9d4634
Bug: 755413
Reviewed-on: https://chromium-review.googlesource.com/685986
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509987}

ash/message_center/message_center_button_bar.h

@@ -26,7 +26,7 @@ namespace ash {
 class MessageCenterView;
 
 // MessageCenterButtonBar is the class that shows the content outside the main
-// notification area - the label (or NotifierGroup switcher) and the buttons.
+// notification area - the label and the buttons.
 class MessageCenterButtonBar : public views::View,
                                public views::ButtonListener {
  public:

ash/message_center/notifier_settings_view.cc

@@ -52,7 +52,6 @@ namespace ash {
 
 using message_center::MessageCenter;
 using message_center::Notifier;
-using message_center::NotifierGroup;
 using message_center::NotifierId;
 using message_center::NotifierSettingsProvider;
 
@@ -497,17 +496,6 @@ void NotifierSettingsView::UpdateIconImage(const NotifierId& notifier_id,
   }
 }
 
-void NotifierSettingsView::NotifierGroupChanged() {
-  std::vector<std::unique_ptr<Notifier>> notifiers;
-  if (provider_)
-    provider_->GetNotifierList(&notifiers);
-
-  UpdateContentsView(std::move(notifiers));
-}
-
-void NotifierSettingsView::NotifierEnabledChanged(const NotifierId& notifier_id,
-                                                  bool enabled) {}
-
 void NotifierSettingsView::UpdateContentsView(
     std::vector<std::unique_ptr<Notifier>> notifiers) {
   buttons_.clear();

ash/message_center/notifier_settings_view.h

@@ -42,9 +42,6 @@ class ASH_EXPORT NotifierSettingsView
   // Overridden from NotifierSettingsDelegate:
   void UpdateIconImage(const message_center::NotifierId& notifier_id,
                        const gfx::Image& icon) override;
-  void NotifierGroupChanged() override;
-  void NotifierEnabledChanged(const message_center::NotifierId& notifier_id,
-                              bool enabled) override;
 
   void set_provider(message_center::NotifierSettingsProvider* new_provider) {
     provider_ = new_provider;

ash/message_center/notifier_settings_view_unittest.cc

@@ -14,7 +14,6 @@
 #include "ui/views/controls/scroll_view.h"
 
 using message_center::Notifier;
-using message_center::NotifierGroup;
 using message_center::NotifierId;
 using message_center::NotifierSettingsObserver;
 using message_center::NotifierSettingsProvider;
@@ -39,26 +38,6 @@ class TestingNotifierSettingsProvider : public NotifierSettingsProvider {
   void AddObserver(NotifierSettingsObserver* observer) override {}
   void RemoveObserver(NotifierSettingsObserver* observer) override {}
 
-  size_t GetNotifierGroupCount() const override { return 1; }
-
-  const message_center::NotifierGroup& GetNotifierGroupAt(
-      size_t index) const override {
-    DCHECK_EQ(0u, index);
-    return GetActiveNotifierGroup();
-  }
-
-  bool IsNotifierGroupActiveAt(size_t index) const override {
-    return index == 0;
-  }
-
-  void SwitchToNotifierGroup(size_t index) override { NOTREACHED(); }
-
-  const NotifierGroup& GetActiveNotifierGroup() const override {
-    static NotifierGroup group{base::UTF8ToUTF16("Fake name"),
-                               base::UTF8ToUTF16("fake@email.com")};
-    return group;
-  }
-
   void GetNotifierList(
       std::vector<std::unique_ptr<Notifier>>* notifiers) override {
     notifiers->clear();

chrome/browser/BUILD.gn

@@ -2455,24 +2455,15 @@ split_static_library("browser") {
       "metrics/tab_usage_recorder.h",
       "net/firefox_proxy_settings.cc",
       "net/firefox_proxy_settings.h",
-      "notifications/arc_application_notifier_controller_chromeos.cc",
-      "notifications/arc_application_notifier_controller_chromeos.h",
-      "notifications/extension_notifier_controller.cc",
-      "notifications/extension_notifier_controller.h",
       "notifications/message_center_notification_manager.cc",
       "notifications/message_center_notification_manager.h",
-      "notifications/message_center_settings_controller.cc",
-      "notifications/message_center_settings_controller.h",
       "notifications/message_center_stats_collector.cc",
       "notifications/message_center_stats_collector.h",
       "notifications/notification_system_observer.cc",
       "notifications/notification_system_observer.h",
       "notifications/notification_ui_manager_desktop.cc",
-      "notifications/notifier_controller.h",
       "notifications/profile_notification.cc",
       "notifications/profile_notification.h",
-      "notifications/web_page_notifier_controller.cc",
-      "notifications/web_page_notifier_controller.h",
       "obsolete_system/obsolete_system.h",
       "obsolete_system/obsolete_system_linux.cc",
       "obsolete_system/obsolete_system_mac.cc",
@@ -2716,6 +2707,16 @@ split_static_library("browser") {
       "metrics/perf/random_selector.h",
       "metrics/perf/windowed_incognito_observer.cc",
       "metrics/perf/windowed_incognito_observer.h",
+      "notifications/arc_application_notifier_controller_chromeos.cc",
+      "notifications/arc_application_notifier_controller_chromeos.h",
+      "notifications/extension_notifier_controller.cc",
+      "notifications/extension_notifier_controller.h",
+      "notifications/message_center_settings_controller_chromeos.cc",
+      "notifications/message_center_settings_controller_chromeos.h",
+      "notifications/notifier_controller.cc",
+      "notifications/notifier_controller.h",
+      "notifications/web_page_notifier_controller.cc",
+      "notifications/web_page_notifier_controller.h",
       "policy/default_geolocation_policy_handler.cc",
       "policy/default_geolocation_policy_handler.h",
       "signin/signin_error_notifier_ash.cc",

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -101,6 +101,7 @@
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
+#include "chrome/browser/notifications/message_center_settings_controller_chromeos.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/task_manager/task_manager_interface.h"
@@ -858,6 +859,10 @@ void ChromeBrowserMainPartsChromeos::PreProfileInit() {
   message_center::MessageCenter::Get()->SetProductOSName(
       l10n_util::GetStringUTF16(IDS_SHORT_PRODUCT_OS_NAME));
 
+  // Create and set the settings provider for the message center.
+  message_center::MessageCenter::Get()->SetNotifierSettingsProvider(
+      std::make_unique<MessageCenterSettingsControllerChromeOs>());
+
   // Register all installed components for regular update.
   base::PostTaskWithTraitsAndReplyWithResult(
       FROM_HERE, {base::MayBlock()},

chrome/browser/notifications/extension_notifier_controller.cc

@@ -9,6 +9,8 @@
 #include "chrome/browser/notifications/notifier_state_tracker.h"
 #include "chrome/browser/notifications/notifier_state_tracker_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/extensions/api/notifications.h"
+#include "extensions/browser/event_router.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/permissions/api_permission.h"
@@ -69,6 +71,33 @@ void ExtensionNotifierController::SetNotifierEnabled(
   observer_->OnNotifierEnabledChanged(notifier_id, enabled);
 }
 
+bool ExtensionNotifierController::HasAdvancedSettings(
+    Profile* profile,
+    const message_center::NotifierId& notifier_id) const {
+  const std::string& extension_id = notifier_id.id;
+
+  extensions::EventRouter* event_router = extensions::EventRouter::Get(profile);
+
+  return event_router->ExtensionHasEventListener(
+      extension_id, extensions::api::notifications::OnShowSettings::kEventName);
+}
+
+void ExtensionNotifierController::OnNotifierAdvancedSettingsRequested(
+    Profile* profile,
+    const message_center::NotifierId& notifier_id,
+    const std::string* notification_id) {
+  const std::string& extension_id = notifier_id.id;
+
+  extensions::EventRouter* event_router = extensions::EventRouter::Get(profile);
+  std::unique_ptr<base::ListValue> args(new base::ListValue());
+
+  std::unique_ptr<extensions::Event> event(new extensions::Event(
+      extensions::events::NOTIFICATIONS_ON_SHOW_SETTINGS,
+      extensions::api::notifications::OnShowSettings::kEventName,
+      std::move(args)));
+  event_router->DispatchEventToExtension(extension_id, std::move(event));
+}
+
 void ExtensionNotifierController::OnAppImageUpdated(
     const std::string& id,
     const gfx::ImageSkia& image) {

chrome/browser/notifications/extension_notifier_controller.h

@@ -18,12 +18,19 @@ class ExtensionNotifierController : public NotifierController,
   explicit ExtensionNotifierController(Observer* observer);
   ~ExtensionNotifierController() override;
 
+  // NotifierController:
   std::vector<std::unique_ptr<message_center::Notifier>> GetNotifierList(
       Profile* profile) override;
-
   void SetNotifierEnabled(Profile* profile,
                           const message_center::NotifierId& notifier_id,
                           bool enabled) override;
+  bool HasAdvancedSettings(
+      Profile* profile,
+      const message_center::NotifierId& notifier_id) const override;
+  void OnNotifierAdvancedSettingsRequested(
+      Profile* profile,
+      const message_center::NotifierId& notifier_id,
+      const std::string* notification_id) override;
 
  private:
   // Overridden from AppIconLoaderDelegate.

chrome/browser/notifications/message_center_notification_manager.cc

@@ -9,7 +9,6 @@
 
 #include "base/logging.h"
 #include "build/build_config.h"
-#include "chrome/browser/notifications/message_center_settings_controller.h"
 #include "chrome/browser/notifications/profile_notification.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
@@ -33,14 +32,11 @@
 using message_center::NotifierId;
 
 MessageCenterNotificationManager::MessageCenterNotificationManager(
-    message_center::MessageCenter* message_center,
-    std::unique_ptr<message_center::NotifierSettingsProvider> settings_provider)
+    message_center::MessageCenter* message_center)
     : message_center_(message_center),
-      settings_provider_(std::move(settings_provider)),
       system_observer_(this),
       stats_collector_(message_center) {
   message_center_->AddObserver(this);
-  message_center_->SetNotifierSettingsProvider(settings_provider_.get());
 
 #if !defined(OS_CHROMEOS)
   blockers_.push_back(
@@ -58,7 +54,6 @@ MessageCenterNotificationManager::MessageCenterNotificationManager(
 }
 
 MessageCenterNotificationManager::~MessageCenterNotificationManager() {
-  message_center_->SetNotifierSettingsProvider(nullptr);
   message_center_->RemoveObserver(this);
 
   profile_notifications_.clear();

chrome/browser/notifications/message_center_notification_manager.h

@@ -44,10 +44,8 @@ class MessageCenterNotificationManager
     : public NotificationUIManager,
       public message_center::MessageCenterObserver {
  public:
-  MessageCenterNotificationManager(
-      message_center::MessageCenter* message_center,
-      std::unique_ptr<message_center::NotifierSettingsProvider>
-          settings_provider);
+  explicit MessageCenterNotificationManager(
+      message_center::MessageCenter* message_center);
   ~MessageCenterNotificationManager() override;
 
   // NotificationUIManager
@@ -100,8 +98,6 @@ class MessageCenterNotificationManager
   // notification is found.
   ProfileNotification* FindProfileNotification(const std::string& id) const;
 
-  std::unique_ptr<message_center::NotifierSettingsProvider> settings_provider_;
-
   // To own the blockers.
   std::vector<std::unique_ptr<message_center::NotificationBlocker>> blockers_;
 

chrome/browser/notifications/message_center_settings_controller_chromeos.cc

+// Copyright (c) 2013 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/message_center_settings_controller_chromeos.h"
+
+#include <algorithm>
+#include <string>
+#include <utility>
+
+#include "base/i18n/string_compare.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/notifications/arc_application_notifier_controller_chromeos.h"
+#include "chrome/browser/notifications/extension_notifier_controller.h"
+#include "chrome/browser/notifications/web_page_notifier_controller.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/user_manager/user_manager.h"
+#include "ui/gfx/image/image.h"
+
+using message_center::Notifier;
+using message_center::NotifierId;
+
+namespace {
+
+class NotifierComparator {
+ public:
+  explicit NotifierComparator(icu::Collator* collator) : collator_(collator) {}
+
+  bool operator()(const std::unique_ptr<Notifier>& n1,
+                  const std::unique_ptr<Notifier>& n2) {
+    if (n1->notifier_id.type != n2->notifier_id.type)
+      return n1->notifier_id.type < n2->notifier_id.type;
+
+    if (collator_) {
+      return base::i18n::CompareString16WithCollator(*collator_, n1->name,
+                                                     n2->name) == UCOL_LESS;
+    }
+    return n1->name < n2->name;
+  }
+
+ private:
+  icu::Collator* collator_;
+};
+
+}  // namespace
+
+MessageCenterSettingsControllerChromeOs::
+    MessageCenterSettingsControllerChromeOs() {
+  sources_.insert(std::make_pair(NotifierId::APPLICATION,
+                                 std::unique_ptr<NotifierController>(
+                                     new ExtensionNotifierController(this))));
+
+  sources_.insert(std::make_pair(NotifierId::WEB_PAGE,
+                                 std::unique_ptr<NotifierController>(
+                                     new WebPageNotifierController(this))));
+
+  sources_.insert(std::make_pair(
+      NotifierId::ARC_APPLICATION,
+      std::unique_ptr<NotifierController>(
+          new arc::ArcApplicationNotifierControllerChromeOS(this))));
+}
+
+MessageCenterSettingsControllerChromeOs::
+    ~MessageCenterSettingsControllerChromeOs() {}
+
+void MessageCenterSettingsControllerChromeOs::AddObserver(
+    message_center::NotifierSettingsObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void MessageCenterSettingsControllerChromeOs::RemoveObserver(
+    message_center::NotifierSettingsObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void MessageCenterSettingsControllerChromeOs::GetNotifierList(
+    std::vector<std::unique_ptr<Notifier>>* notifiers) {
+  DCHECK(notifiers);
+  for (auto& source : sources_) {
+    auto source_notifiers = source.second->GetNotifierList(GetProfile());
+    for (auto& notifier : source_notifiers) {
+      notifiers->push_back(std::move(notifier));
+    }
+  }
+
+  UErrorCode error = U_ZERO_ERROR;
+  std::unique_ptr<icu::Collator> collator(icu::Collator::createInstance(error));
+  NotifierComparator comparator(U_SUCCESS(error) ? collator.get() : nullptr);
+
+  std::sort(notifiers->begin(), notifiers->end(), comparator);
+}
+
+void MessageCenterSettingsControllerChromeOs::SetNotifierEnabled(
+    const NotifierId& notifier_id,
+    bool enabled) {
+  sources_[notifier_id.type]->SetNotifierEnabled(GetProfile(), notifier_id,
+                                                 enabled);
+}
+
+void MessageCenterSettingsControllerChromeOs::OnNotifierSettingsClosing() {
+  for (auto& source : sources_) {
+    source.second->OnNotifierSettingsClosing();
+  }
+}
+
+bool MessageCenterSettingsControllerChromeOs::NotifierHasAdvancedSettings(
+    const NotifierId& notifier_id) const {
+  return sources_.find(notifier_id.type)
+      ->second->HasAdvancedSettings(GetProfile(), notifier_id);
+}
+
+void MessageCenterSettingsControllerChromeOs::
+    OnNotifierAdvancedSettingsRequested(const NotifierId& notifier_id,
+                                        const std::string* notification_id) {
+  return sources_[notifier_id.type]->OnNotifierAdvancedSettingsRequested(
+      GetProfile(), notifier_id, notification_id);
+}
+
+void MessageCenterSettingsControllerChromeOs::OnIconImageUpdated(
+    const message_center::NotifierId& id,
+    const gfx::Image& image) {
+  for (message_center::NotifierSettingsObserver& observer : observers_)
+    observer.UpdateIconImage(id, image);
+}
+
+Profile* MessageCenterSettingsControllerChromeOs::GetProfile() const {
+  return chromeos::ProfileHelper::Get()->GetProfileByUser(
+      user_manager::UserManager::Get()->GetActiveUser());
+}
+
+void MessageCenterSettingsControllerChromeOs::OnNotifierEnabledChanged(
+    const message_center::NotifierId& id,
+    bool enabled) {
+  for (message_center::NotifierSettingsObserver& observer : observers_)
+    observer.NotifierEnabledChanged(id, enabled);
+}

chrome/browser/notifications/message_center_settings_controller.h → chrome/browser/notifications/message_center_settings_controller_chromeos.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_H_
-#define CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_H_
+#ifndef CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_CHROMEOS_H_
+#define CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_CHROMEOS_H_
 
 #include <stddef.h>
 
@@ -12,50 +12,28 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/notifications/notifier_controller.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/ui/app_icon_loader.h"
-#include "components/content_settings/core/common/content_settings.h"
-#include "components/favicon_base/favicon_types.h"
 #include "ui/message_center/notifier_settings.h"
 
-#if defined(OS_CHROMEOS)
-#include "components/user_manager/user_manager.h"
-#endif
-
 class NotifierController;
 
-namespace message_center {
-class ProfileNotifierGroup;
-}
-
 // The class to bridge between the settings UI of notifiers and the preference
 // storage.
-class MessageCenterSettingsController
+class MessageCenterSettingsControllerChromeOs
     : public message_center::NotifierSettingsProvider,
-      public ProfileAttributesStorage::Observer,
-#if defined(OS_CHROMEOS)
-      public user_manager::UserManager::UserSessionStateObserver,
-#endif
       public NotifierController::Observer {
  public:
-  explicit MessageCenterSettingsController(
-      ProfileAttributesStorage& profile_attributes_storage);
-  ~MessageCenterSettingsController() override;
+  MessageCenterSettingsControllerChromeOs();
+  ~MessageCenterSettingsControllerChromeOs() override;
 
-  // Overridden from message_center::NotifierSettingsProvider.
+  // message_center::NotifierSettingsProvider
   void AddObserver(message_center::NotifierSettingsObserver* observer) override;
   void RemoveObserver(
       message_center::NotifierSettingsObserver* observer) override;
-  size_t GetNotifierGroupCount() const override;
-  const message_center::NotifierGroup& GetNotifierGroupAt(
-      size_t index) const override;
-  bool IsNotifierGroupActiveAt(size_t index) const override;
-  void SwitchToNotifierGroup(size_t index) override;
-  const message_center::NotifierGroup& GetActiveNotifierGroup() const override;
   void GetNotifierList(std::vector<std::unique_ptr<message_center::Notifier>>*
                            notifiers) override;
   void SetNotifierEnabled(const message_center::NotifierId& notifier_id,
@@ -67,61 +45,24 @@ class MessageCenterSettingsController
       const message_center::NotifierId& notifier_id,
       const std::string* notification_id) override;
 
-#if defined(OS_CHROMEOS)
-  // Overridden from user_manager::UserManager::UserSessionStateObserver.
-  void ActiveUserChanged(const user_manager::User* active_user) override;
-#endif
-
  private:
-  // Overridden from ProfileAttributesStorage::Observer.
-  void OnProfileAdded(const base::FilePath& profile_path) override;
-  void OnProfileWasRemoved(const base::FilePath& profile_path,
-      const base::string16& profile_name) override;
-  void OnProfileNameChanged(const base::FilePath& profile_path,
-      const base::string16& old_profile_name) override;
-  void OnProfileAuthInfoChanged(const base::FilePath& profile_path) override;
-
-  // Overridden from NotifierController::Observer.
+  // NotifierController::Observer
   void OnIconImageUpdated(const message_center::NotifierId&,
                           const gfx::Image&) override;
   void OnNotifierEnabledChanged(const message_center::NotifierId&,
                                 bool) override;
 
-  void DispatchNotifierGroupChanged();
-
-#if defined(OS_CHROMEOS)
-  // Sets up the notifier group for the guest session. This needs to be
-  // separated from RebuildNotifierGroup() and called asynchronously to avoid
-  // the infinite loop of creating profile. See more the comment of
-  // RebuildNotifierGroups().
-  void CreateNotifierGroupForGuestLogin();
-#endif
-
-  // Sets up the notifier groups for all profiles. If |notify| is true, then
-  // it sends out a NotifierGroupChange notification to each observer.
-  void RebuildNotifierGroups(bool notify);
+  Profile* GetProfile() const;
 
   // The views displaying notifier settings.
   base::ObserverList<message_center::NotifierSettingsObserver> observers_;
 
-  // The list of all configurable notifier groups. This is each profile that is
-  // loaded (and in the ProfileAttributesStorage - so no incognito profiles go
-  // here).
-  std::vector<std::unique_ptr<message_center::ProfileNotifierGroup>>
-      notifier_groups_;
-
   // Notifier source for each notifier type.
   std::map<message_center::NotifierId::NotifierType,
            std::unique_ptr<NotifierController>>
       sources_;
 
-  size_t current_notifier_group_;
-
-  ProfileAttributesStorage& profile_attributes_storage_;
-
-  base::WeakPtrFactory<MessageCenterSettingsController> weak_factory_;
-
-  DISALLOW_COPY_AND_ASSIGN(MessageCenterSettingsController);
+  DISALLOW_COPY_AND_ASSIGN(MessageCenterSettingsControllerChromeOs);
 };
 
-#endif  // CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_H_
+#endif  // CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_CHROMEOS_H_

chrome/browser/notifications/message_center_settings_controller_unittest.cc → chrome/browser/notifications/message_center_settings_controller_chromeos_unittest.cc

@@ -2,17 +2,21 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/notifications/message_center_settings_controller_chromeos.h"
+
 #include <string>
 #include <utility>
 
+#include "ash/system/system_notifier.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
+#include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/test_extension_system.h"
-#include "chrome/browser/notifications/message_center_settings_controller.h"
 #include "chrome/browser/permissions/permission_manager.h"
 #include "chrome/browser/permissions/permission_result.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -26,82 +30,49 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/message_center/notifier_settings.h"
 
-#if defined(OS_CHROMEOS)
-#include "ash/system/system_notifier.h"
-#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
-#include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
-#endif
-
-class MessageCenterSettingsControllerBaseTest : public testing::Test {
+class MessageCenterSettingsControllerChromeOsTest : public testing::Test {
  protected:
-  MessageCenterSettingsControllerBaseTest()
+  MessageCenterSettingsControllerChromeOsTest()
       : testing_profile_manager_(TestingBrowserProcess::GetGlobal()) {}
-
-  ~MessageCenterSettingsControllerBaseTest() override {}
-
-  base::FilePath GetProfilePath(const std::string& base_name) {
-    return testing_profile_manager_.profile_manager()->user_data_dir()
-        .AppendASCII(base_name);
-  }
-
-  void SetUp() override { ASSERT_TRUE(testing_profile_manager_.SetUp()); }
-
-  virtual TestingProfile* CreateProfile(const std::string& name) {
-    return testing_profile_manager_.CreateTestingProfile(name);
-  }
-
-  void CreateController() {
-    controller_.reset(new MessageCenterSettingsController(
-        *testing_profile_manager_.profile_attributes_storage()));
-  }
-
-  void ResetController() {
-    controller_.reset();
-  }
-
-  MessageCenterSettingsController* controller() { return controller_.get(); }
-
- private:
-  content::TestBrowserThreadBundle thread_bundle_;
-  TestingProfileManager testing_profile_manager_;
-  std::unique_ptr<MessageCenterSettingsController> controller_;
-
-  DISALLOW_COPY_AND_ASSIGN(MessageCenterSettingsControllerBaseTest);
-};
-
-#if defined(OS_CHROMEOS)
-
-class MessageCenterSettingsControllerChromeOSTest
-    : public MessageCenterSettingsControllerBaseTest {
- protected:
-  MessageCenterSettingsControllerChromeOSTest() {}
-  ~MessageCenterSettingsControllerChromeOSTest() override {}
+  ~MessageCenterSettingsControllerChromeOsTest() override {}
 
   void SetUp() override {
-    MessageCenterSettingsControllerBaseTest::SetUp();
+    ASSERT_TRUE(testing_profile_manager_.SetUp());
 
     // Initialize the UserManager singleton to a fresh FakeUserManager instance.
     user_manager_enabler_.reset(new chromeos::ScopedUserManagerEnabler(
         new chromeos::FakeChromeUserManager));
   }
 
-  void TearDown() override {
-    ResetController();
-    MessageCenterSettingsControllerBaseTest::TearDown();
-  }
+  void TearDown() override { ResetController(); }
 
-  TestingProfile* CreateProfile(const std::string& name) override {
+  TestingProfile* CreateProfile(const std::string& name) {
     TestingProfile* profile =
-        MessageCenterSettingsControllerBaseTest::CreateProfile(name);
+        testing_profile_manager_.CreateTestingProfile(name);
 
     GetFakeUserManager()->AddUser(AccountId::FromUserEmail(name));
     GetFakeUserManager()->LoginUser(AccountId::FromUserEmail(name));
     return profile;
   }
 
+  base::FilePath GetProfilePath(const std::string& base_name) {
+    return testing_profile_manager_.profile_manager()
+        ->user_data_dir()
+        .AppendASCII(base_name);
+  }
+
   void SwitchActiveUser(const std::string& name) {
     GetFakeUserManager()->SwitchActiveUser(AccountId::FromUserEmail(name));
-    controller()->ActiveUserChanged(GetFakeUserManager()->GetActiveUser());
+  }
+
+  void CreateController() {
+    controller_.reset(new MessageCenterSettingsControllerChromeOs());
+  }
+
+  void ResetController() { controller_.reset(); }
+
+  MessageCenterSettingsControllerChromeOs* controller() {
+    return controller_.get();
   }
 
  private:
@@ -110,68 +81,18 @@ class MessageCenterSettingsControllerChromeOSTest
         user_manager::UserManager::Get());
   }
 
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingProfileManager testing_profile_manager_;
+  std::unique_ptr<MessageCenterSettingsControllerChromeOs> controller_;
   std::unique_ptr<chromeos::ScopedUserManagerEnabler> user_manager_enabler_;
 
-  DISALLOW_COPY_AND_ASSIGN(MessageCenterSettingsControllerChromeOSTest);
+  DISALLOW_COPY_AND_ASSIGN(MessageCenterSettingsControllerChromeOsTest);
 };
 
-typedef MessageCenterSettingsControllerChromeOSTest
-    MessageCenterSettingsControllerTest;
-#else
-typedef MessageCenterSettingsControllerBaseTest
-    MessageCenterSettingsControllerTest;
-#endif  // OS_CHROMEOS
-
-#if !defined(OS_CHROMEOS)
-TEST_F(MessageCenterSettingsControllerTest, NotifierGroups) {
-  CreateProfile("Profile-1");
-  CreateProfile("Profile-2");
-  CreateController();
-
-  EXPECT_EQ(controller()->GetNotifierGroupCount(), 2u);
-
-  EXPECT_EQ(controller()->GetNotifierGroupAt(0).name,
-            base::UTF8ToUTF16("Profile-1"));
-  EXPECT_EQ(controller()->GetNotifierGroupAt(1).name,
-            base::UTF8ToUTF16("Profile-2"));
-
-  EXPECT_EQ(controller()->GetActiveNotifierGroup().name,
-            base::UTF8ToUTF16("Profile-1"));
-
-  controller()->SwitchToNotifierGroup(1);
-  EXPECT_EQ(controller()->GetActiveNotifierGroup().name,
-            base::UTF8ToUTF16("Profile-2"));
-
-  controller()->SwitchToNotifierGroup(0);
-  EXPECT_EQ(controller()->GetActiveNotifierGroup().name,
-            base::UTF8ToUTF16("Profile-1"));
-}
-#else   // !defined(OS_CHROMEOS)
-TEST_F(MessageCenterSettingsControllerChromeOSTest, NotifierGroups) {
-  CreateProfile("Profile-1");
-  CreateProfile("Profile-2");
-  CreateController();
-
-  EXPECT_EQ(controller()->GetNotifierGroupCount(), 1u);
-
-  EXPECT_EQ(controller()->GetNotifierGroupAt(0).name,
-            base::UTF8ToUTF16("Profile-1"));
-
-  SwitchActiveUser("Profile-2");
-  EXPECT_EQ(controller()->GetNotifierGroupCount(), 1u);
-  EXPECT_EQ(controller()->GetNotifierGroupAt(0).name,
-            base::UTF8ToUTF16("Profile-2"));
-
-  SwitchActiveUser("Profile-1");
-  EXPECT_EQ(controller()->GetNotifierGroupCount(), 1u);
-  EXPECT_EQ(controller()->GetNotifierGroupAt(0).name,
-            base::UTF8ToUTF16("Profile-1"));
-}
 // TODO(mukai): write a test case to reproduce the actual guest session scenario
-// in ChromeOS -- no profiles in |profile_attributes_storage_|.
-#endif  // !defined(OS_CHROMEOS)
+// in ChromeOS.
 
-TEST_F(MessageCenterSettingsControllerTest, NotifierSortOrder) {
+TEST_F(MessageCenterSettingsControllerChromeOsTest, NotifierSortOrder) {
   TestingProfile* profile = CreateProfile("Profile-1");
   extensions::TestExtensionSystem* test_extension_system =
       static_cast<extensions::TestExtensionSystem*>(
@@ -281,14 +202,12 @@ TEST_F(MessageCenterSettingsControllerTest, NotifierSortOrder) {
 
   std::vector<std::unique_ptr<message_center::Notifier>> notifiers;
   controller()->GetNotifierList(&notifiers);
-
   EXPECT_EQ(2u, notifiers.size());
-
   EXPECT_EQ(kBarId, notifiers[0]->notifier_id.id);
   EXPECT_EQ(kFooId, notifiers[1]->notifier_id.id);
 }
 
-TEST_F(MessageCenterSettingsControllerTest, SetWebPageNotifierEnabled) {
+TEST_F(MessageCenterSettingsControllerChromeOsTest, SetWebPageNotifierEnabled) {
   Profile* profile = CreateProfile("MyProfile");
   CreateController();
 

chrome/browser/notifications/notification_ui_manager_desktop.cc

@@ -9,20 +9,9 @@
 
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/notifications/message_center_notification_manager.h"
-#include "chrome/browser/notifications/message_center_settings_controller.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_manager.h"
 
 // static
 NotificationUIManager* NotificationUIManager::Create() {
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-  if (!profile_manager)
-    return nullptr;
-
-  std::unique_ptr<message_center::NotifierSettingsProvider> settings_provider(
-      new MessageCenterSettingsController(
-          profile_manager->GetProfileAttributesStorage()));
   return new MessageCenterNotificationManager(
-      g_browser_process->message_center(),
-      std::move(settings_provider));
+      g_browser_process->message_center());
 }

chrome/browser/notifications/notifier_controller.cc

+// Copyright 2017 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/notifier_controller.h"
+
+bool NotifierController::HasAdvancedSettings(
+    Profile* profile,
+    const message_center::NotifierId& notifier_id) const {
+  return false;
+}

chrome/browser/notifications/notifier_controller.h

@@ -46,6 +46,17 @@ class NotifierController {
                                   const message_center::NotifierId& notifier_id,
                                   bool enabled) = 0;
 
+  // Returns true if the given notifier should have an advanced settings button.
+  virtual bool HasAdvancedSettings(
+      Profile* profile,
+      const message_center::NotifierId& notifier_id) const;
+
+  // Called when the advanced settings button has been clicked.
+  virtual void OnNotifierAdvancedSettingsRequested(
+      Profile* profile,
+      const message_center::NotifierId& notifier_id,
+      const std::string* notification_id) {}
+
   // Release temporary resouces tagged with notifier list that is returned last
   // time.
   virtual void OnNotifierSettingsClosing() {}

chrome/test/BUILD.gn

@@ -3707,7 +3707,6 @@ test("unit_tests") {
       # Android does not use the Message Center notification system.
       "../browser/net/firefox_proxy_settings_unittest.cc",
       "../browser/notifications/message_center_notifications_unittest.cc",
-      "../browser/notifications/message_center_settings_controller_unittest.cc",
       "../browser/platform_util_unittest.cc",
       "../browser/policy/policy_path_parser_unittest.cc",
       "../browser/process_singleton_posix_unittest.cc",
@@ -3947,6 +3946,7 @@ test("unit_tests") {
     sources += [
       "../browser/component_updater/cros_component_installer_unittest.cc",
       "../browser/media/webrtc/desktop_media_list_ash_unittest.cc",
+      "../browser/notifications/message_center_settings_controller_chromeos_unittest.cc",
       "../browser/renderer_context_menu/mock_render_view_context_menu.cc",
       "../browser/renderer_context_menu/mock_render_view_context_menu.h",
       "../browser/signin/signin_error_notifier_ash_unittest.cc",

ui/message_center/fake_message_center.cc

@@ -110,17 +110,14 @@ void FakeMessageCenter::MarkSinglePopupAsShown(const std::string& id,
                                                bool mark_notification_as_read) {
 }
 
-void FakeMessageCenter::DisplayedNotification(
-    const std::string& id,
-    const DisplaySource source) {
-}
+void FakeMessageCenter::DisplayedNotification(const std::string& id,
+                                              const DisplaySource source) {}
 
 void FakeMessageCenter::SetNotifierSettingsProvider(
-    NotifierSettingsProvider* provider) {
-}
+    std::unique_ptr<NotifierSettingsProvider> provider) {}
 
 NotifierSettingsProvider* FakeMessageCenter::GetNotifierSettingsProvider() {
-  return NULL;
+  return nullptr;
 }
 
 void FakeMessageCenter::SetQuietMode(bool in_quiet_mode) {

ui/message_center/fake_message_center.h

@@ -58,7 +58,8 @@ class FakeMessageCenter : public MessageCenter {
                               bool mark_notification_as_read) override;
   void DisplayedNotification(const std::string& id,
                              const DisplaySource source) override;
-  void SetNotifierSettingsProvider(NotifierSettingsProvider* provider) override;
+  void SetNotifierSettingsProvider(
+      std::unique_ptr<NotifierSettingsProvider> provider) override;
   NotifierSettingsProvider* GetNotifierSettingsProvider() override;
   void SetQuietMode(bool in_quiet_mode) override;
   void SetLockedState(bool locked) override;

ui/message_center/message_center.h

@@ -56,8 +56,8 @@ class MESSAGE_CENTER_EXPORT MessageCenter {
   // Creates the global message center object.
   static void Initialize();
 
-  // Returns the global message center object. Returns NULL if Initialize is not
-  // called.
+  // Returns the global message center object. Returns null if Initialize is
+  // not called.
   static MessageCenter* Get();
 
   // Destroys the global message_center object.
@@ -75,8 +75,8 @@ class MESSAGE_CENTER_EXPORT MessageCenter {
   virtual bool IsLockedState() const = 0;
   virtual bool HasClickedListener(const std::string& id) = 0;
 
-  // Find the notification with the corresponding id. Returns NULL if not found.
-  // The returned instance is owned by the message center.
+  // Find the notification with the corresponding id. Returns null if not
+  // found. The returned instance is owned by the message center.
   virtual message_center::Notification* FindVisibleNotificationById(
       const std::string& id) = 0;
 
@@ -152,11 +152,10 @@ class MESSAGE_CENTER_EXPORT MessageCenter {
       const std::string& id,
       const DisplaySource source) = 0;
 
-  // Setter/getter of notifier settings provider. This will be a weak reference.
-  // This should be set at the initialization process. The getter may return
-  // NULL for tests.
+  // Setter/getter of notifier settings provider. The getter may return null
+  // for tests, and will always be null on non-ChromeOS.
   virtual void SetNotifierSettingsProvider(
-      NotifierSettingsProvider* provider) = 0;
+      std::unique_ptr<NotifierSettingsProvider> provider) = 0;
   virtual NotifierSettingsProvider* GetNotifierSettingsProvider() = 0;
 
   // This can be called to change the quiet mode state (without a timeout).

ui/message_center/message_center_impl.cc

@@ -89,14 +89,12 @@ class ScopedNotificationsIterationLock {
 
 MessageCenterImpl::MessageCenterImpl()
     : MessageCenter(),
-      popup_timers_controller_(new PopupTimersController(this)),
-      settings_provider_(NULL) {
+      popup_timers_controller_(new PopupTimersController(this)) {
   notification_list_.reset(new NotificationList(this));
   notification_change_queue_.reset(new ChangeQueue());
 }
 
 MessageCenterImpl::~MessageCenterImpl() {
-  SetNotifierSettingsProvider(NULL);
 }
 
 void MessageCenterImpl::AddObserver(MessageCenterObserver* observer) {
@@ -155,15 +153,6 @@ void MessageCenterImpl::OnBlockingStateChanged(NotificationBlocker* blocker) {
     observer.OnBlockingStateChanged(blocker);
 }
 
-void MessageCenterImpl::UpdateIconImage(const NotifierId& notifier_id,
-                                        const gfx::Image& icon) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-}
-
-void MessageCenterImpl::NotifierGroupChanged() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-}
-
 void MessageCenterImpl::NotifierEnabledChanged(
     const NotifierId& notifier_id, bool enabled) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
@@ -622,20 +611,16 @@ void MessageCenterImpl::DisplayedNotification(
 }
 
 void MessageCenterImpl::SetNotifierSettingsProvider(
-    NotifierSettingsProvider* provider) {
+    std::unique_ptr<NotifierSettingsProvider> provider) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (settings_provider_) {
-    settings_provider_->RemoveObserver(this);
-    settings_provider_ = NULL;
-  }
-  settings_provider_ = provider;
-  if (settings_provider_)
-    settings_provider_->AddObserver(this);
+  DCHECK(!settings_provider_);
+  settings_provider_ = std::move(provider);
+  settings_provider_->AddObserver(this);
 }
 
 NotifierSettingsProvider* MessageCenterImpl::GetNotifierSettingsProvider() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  return settings_provider_;
+  return settings_provider_.get();
 }
 
 void MessageCenterImpl::SetQuietMode(bool in_quiet_mode) {

ui/message_center/message_center_impl.h

@@ -77,7 +77,8 @@ class MESSAGE_CENTER_EXPORT MessageCenterImpl
                               bool mark_notification_as_read) override;
   void DisplayedNotification(const std::string& id,
                              const DisplaySource source) override;
-  void SetNotifierSettingsProvider(NotifierSettingsProvider* provider) override;
+  void SetNotifierSettingsProvider(
+      std::unique_ptr<NotifierSettingsProvider> provider) override;
   NotifierSettingsProvider* GetNotifierSettingsProvider() override;
   void SetQuietMode(bool in_quiet_mode) override;
   void SetLockedState(bool locked) override;
@@ -91,9 +92,6 @@ class MESSAGE_CENTER_EXPORT MessageCenterImpl
   void OnBlockingStateChanged(NotificationBlocker* blocker) override;
 
   // message_center::NotifierSettingsObserver overrides:
-  void UpdateIconImage(const NotifierId& notifier_id,
-                       const gfx::Image& icon) override;
-  void NotifierGroupChanged() override;
   void NotifierEnabledChanged(const NotifierId& notifier_id,
                               bool enabled) override;
 
@@ -131,7 +129,8 @@ class MESSAGE_CENTER_EXPORT MessageCenterImpl
   base::ObserverList<MessageCenterObserver> observer_list_;
   std::unique_ptr<PopupTimersController> popup_timers_controller_;
   std::unique_ptr<base::OneShotTimer> quiet_mode_timer_;
-  NotifierSettingsProvider* settings_provider_;
+  // Null on !ChromeOS.
+  std::unique_ptr<NotifierSettingsProvider> settings_provider_;
   std::vector<NotificationBlocker*> blockers_;
   std::unique_ptr<ChangeQueue> notification_change_queue_;
 

ui/message_center/notifier_settings.cc

@@ -55,18 +55,8 @@ bool NotifierId::operator<(const NotifierId& other) const {
 Notifier::Notifier(const NotifierId& notifier_id,
                    const base::string16& name,
                    bool enabled)
-    : notifier_id(notifier_id),
-      name(name),
-      enabled(enabled) {
-}
-
-Notifier::~Notifier() {
-}
-
-NotifierGroup::NotifierGroup(const base::string16& name,
-                             const base::string16& login_info)
-    : name(name), login_info(login_info) {}
+    : notifier_id(notifier_id), name(name), enabled(enabled) {}
 
-NotifierGroup::~NotifierGroup() {}
+Notifier::~Notifier() {}
 
 }  // namespace message_center

ui/message_center/notifier_settings.h

@@ -122,34 +122,17 @@ struct MESSAGE_CENTER_EXPORT Notifier {
   DISALLOW_COPY_AND_ASSIGN(Notifier);
 };
 
-struct MESSAGE_CENTER_EXPORT NotifierGroup {
-  NotifierGroup(const base::string16& name, const base::string16& login_info);
-  ~NotifierGroup();
-
-  // Display name of a notifier group.
-  const base::string16 name;
-
-  // More display information about the notifier group.
-  base::string16 login_info;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(NotifierGroup);
-};
-
 // An observer class implemented by the view of the NotifierSettings to get
 // notified when the controller has changed data.
 class MESSAGE_CENTER_EXPORT NotifierSettingsObserver {
  public:
   // Called when an icon in the controller has been updated.
   virtual void UpdateIconImage(const NotifierId& notifier_id,
-                               const gfx::Image& icon) = 0;
-
-  // Called when any change happens to the set of notifier groups.
-  virtual void NotifierGroupChanged() = 0;
+                               const gfx::Image& icon){};
 
   // Called when a notifier is enabled or disabled.
   virtual void NotifierEnabledChanged(const NotifierId& notifier_id,
-                                      bool enabled) = 0;
+                                      bool enabled){};
 };
 
 // A class used by NotifierSettingsView to integrate with a setting system
@@ -162,24 +145,6 @@ class MESSAGE_CENTER_EXPORT NotifierSettingsProvider {
   virtual void AddObserver(NotifierSettingsObserver* observer) = 0;
   virtual void RemoveObserver(NotifierSettingsObserver* observer) = 0;
 
-  // Returns the number of notifier groups available.
-  virtual size_t GetNotifierGroupCount() const = 0;
-
-  // Requests the model for a particular notifier group.
-  virtual const message_center::NotifierGroup& GetNotifierGroupAt(
-      size_t index) const = 0;
-
-  // Returns true if the notifier group at |index| is active.
-  virtual bool IsNotifierGroupActiveAt(size_t index) const = 0;
-
-  // Informs the settings provider that further requests to GetNotifierList
-  // should return notifiers for the specified notifier group.
-  virtual void SwitchToNotifierGroup(size_t index) = 0;
-
-  // Requests the currently active notifier group.
-  virtual const message_center::NotifierGroup& GetActiveNotifierGroup()
-      const = 0;
-
   // Provides the current notifier list in |notifiers|.
   virtual void GetNotifierList(
       std::vector<std::unique_ptr<Notifier>>* notifiers) = 0;