Refactor MessageCenterSettingsController.

chrome/browser/notifications/message_center_settings_controller.h

 // 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.
 
 #ifndef CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_H_
 #define CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_H_
 
 #include <stddef.h>
 
 #include <map>
@@ skipping 12 matching lines @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_source.h"
 #include "ui/message_center/notifier_settings.h"
 
 #if defined(OS_CHROMEOS)
 #include "components/user_manager/user_manager.h"
 #endif
 
 class Profile;
-
-#if defined(OS_CHROMEOS)
-namespace arc {
-class ArcNotifierManager;
-}
-#endif
+class NotifierSource;
 
 namespace base {
 class CancelableTaskTracker;
 }
 
 namespace favicon_base {
 struct FaviconImageResult;
 }
 
 namespace message_center {
 class ProfileNotifierGroup;
 }
 
 // The class to bridge between the settings UI of notifiers and the preference
 // storage.
 class MessageCenterSettingsController
     : public message_center::NotifierSettingsProvider,
       public content::NotificationObserver,
       public ProfileAttributesStorage::Observer,
 #if defined(OS_CHROMEOS)
       public user_manager::UserManager::UserSessionStateObserver,
 #endif
-      public AppIconLoaderDelegate {
+      public message_center::NotifierSettingsObserver {
  public:
   explicit MessageCenterSettingsController(
       ProfileAttributesStorage& profile_attributes_storage);
   ~MessageCenterSettingsController() override;
 
   // Overridden from message_center::NotifierSettingsProvider.
   void AddObserver(message_center::NotifierSettingsObserver* observer) override;
   void RemoveObserver(
       message_center::NotifierSettingsObserver* observer) override;
   size_t GetNotifierGroupCount() const override;
@@ skipping 11 matching lines @@
       const message_center::NotifierId& notifier_id) const override;
   void OnNotifierAdvancedSettingsRequested(
       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
 
-  // Overridden from AppIconLoaderDelegate.
-  void OnAppImageUpdated(const std::string& id,
-                         const gfx::ImageSkia& image) override;
-
  private:
   // Overridden from content::NotificationObserver.
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // ProfileAttributesStorage::Observer:
+  // 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;
 
-  void OnFaviconLoaded(const GURL& url,
-                       const favicon_base::FaviconImageResult& favicon_result);
+  // Overridden from message_center::NotifierSettingsObserver.

[dewittj, 2016/06/16 19:45:36]

While I agree that pulling these out is a nice thing to do,  I'm not sure that
this should implement NotifierSettingsObserver; it's confusing that this is a
NSObserver as well as an NSProvider.

So I'd rename these to:

DispatchUpdateIconImage
DispatchNotifierGroupChanged
DispatchNotifierEnabledChanged

or some similar prefix to make it clear that they are for the purpose of telling
all the observers that something happened.

Then I'd remove the inheritance from NotifierSettingsObserver.

You can maybe add an OnIconImageUpdated to NotifierSettingsProvider so that your
ApplicationNotifiereSource can take a NotifierSettingsProvider*.

+  void UpdateIconImage(const message_center::NotifierId&,
+                       const gfx::Image&) override;
+  void NotifierGroupChanged() override;
+  void NotifierEnabledChanged(const message_center::NotifierId&, bool) override;
 
 #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);
 
   // The views displaying notifier settings.
   base::ObserverList<message_center::NotifierSettingsObserver> observers_;
 
-  // The task tracker for loading favicons.
-  std::unique_ptr<base::CancelableTaskTracker> favicon_tracker_;
-
-  std::unique_ptr<AppIconLoader> app_icon_loader_;
-
-#if defined(OS_CHROMEOS)
-  std::unique_ptr<arc::ArcNotifierManager> arc_notifier_manager_;
-#endif
-
-  std::map<base::string16, ContentSettingsPattern> patterns_;
-
   // 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<NotifierSource>>
+      sources_;
+
   size_t current_notifier_group_;
 
   content::NotificationRegistrar registrar_;
 
   ProfileAttributesStorage& profile_attributes_storage_;
 
   base::WeakPtrFactory<MessageCenterSettingsController> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MessageCenterSettingsController);
 };
 
 #endif  // CHROME_BROWSER_NOTIFICATIONS_MESSAGE_CENTER_SETTINGS_CONTROLLER_H_