Remove NOTIFICATION_SYSTEM_SETTING_CHANGED, switch CrosSettings to base::CallbackRegistry.

chrome/browser/chromeos/login/version_info_updater.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/chromeos/login/version_info_updater.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/chromeos/chromeos_version.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"

[Mattias Nissler (ping if slow), 2013/09/18 09:09:48]

remove?

[Avi (use Gerrit), 2013/09/18 16:41:44]

Done.

 #include "chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/cros_settings_names.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/common/chrome_version_info.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
 namespace chromeos {
 
-namespace {
-
-const char* kReportingFlags[] = {
-  chromeos::kReportDeviceVersionInfo,
-  chromeos::kReportDeviceActivityTimes,
-  chromeos::kReportDeviceBootMode,
-  chromeos::kReportDeviceLocation,
-};
-
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // VersionInfoUpdater public:
 
 VersionInfoUpdater::VersionInfoUpdater(Delegate* delegate)
     : cros_settings_(chromeos::CrosSettings::Get()),
       delegate_(delegate),
       weak_pointer_factory_(this) {
 }
 
 VersionInfoUpdater::~VersionInfoUpdater() {
   policy::DeviceCloudPolicyManagerChromeOS* policy_manager =
       g_browser_process->browser_policy_connector()->
           GetDeviceCloudPolicyManager();
   if (policy_manager)
     policy_manager->core()->store()->RemoveObserver(this);
-
-  for (unsigned int i = 0; i < arraysize(kReportingFlags); ++i)
-    cros_settings_->RemoveSettingsObserver(kReportingFlags[i], this);
 }
 
 void VersionInfoUpdater::StartUpdate(bool is_official_build) {
   if (base::chromeos::IsRunningOnChromeOS()) {
     version_loader_.GetVersion(
         is_official_build ? VersionLoader::VERSION_SHORT_WITH_DATE
                           : VersionLoader::VERSION_FULL,
         base::Bind(&VersionInfoUpdater::OnVersion,
                    weak_pointer_factory_.GetWeakPtr()),
         &tracker_);
   } else {
     UpdateVersionLabel();
   }
 
   policy::DeviceCloudPolicyManagerChromeOS* policy_manager =
       g_browser_process->browser_policy_connector()->
           GetDeviceCloudPolicyManager();
   if (policy_manager) {
     policy_manager->core()->store()->AddObserver(this);
 
     // Ensure that we have up-to-date enterprise info in case enterprise policy
     // is already fetched and has finished initialization.
     UpdateEnterpriseInfo();
   }
 
   // Watch for changes to the reporting flags.
-  for (unsigned int i = 0; i < arraysize(kReportingFlags); ++i)
-    cros_settings_->AddSettingsObserver(kReportingFlags[i], this);
+  version_info_subscription_ = cros_settings_->AddSettingsObserver(
+      kReportDeviceVersionInfo,
+      base::Bind(&VersionInfoUpdater::UpdateEnterpriseInfo,

[Lei Zhang, 2013/09/18 05:00:53]

Can you assign this to a base::Closure variable and re-use it in the 4 different
AddSettingsObserver() calls?

[Mattias Nissler (ping if slow), 2013/09/18 09:09:48]

Could also keep the array and loop in that case and switch to a ScopedVector for
the subscriptions.

[Avi (use Gerrit), 2013/09/18 16:41:44]

Done.

[Lei Zhang, 2013/09/18 18:51:07]

chrome/browser/chromeos/login/existing_user_controller.cc can probably use the
same treatment.

[Avi (use Gerrit), 2013/09/18 19:39:07]

If you look at ExistingUserControllerAutoLoginTest, it explicitly resets two of
the six notifications, so I'd have to use a loop for four notifications while
keeping those two separate. That's weird :(

+                 base::Unretained(this)));
+  activity_times_subscription_ = cros_settings_->AddSettingsObserver(
+      kReportDeviceActivityTimes,
+      base::Bind(&VersionInfoUpdater::UpdateEnterpriseInfo,
+                 base::Unretained(this)));
+  boot_mode_subscription_ = cros_settings_->AddSettingsObserver(
+      kReportDeviceBootMode,
+      base::Bind(&VersionInfoUpdater::UpdateEnterpriseInfo,
+                 base::Unretained(this)));
+  location_subscription_ = cros_settings_->AddSettingsObserver(
+      kReportDeviceLocation,
+      base::Bind(&VersionInfoUpdater::UpdateEnterpriseInfo,
+                 base::Unretained(this)));
 }
 
 void VersionInfoUpdater::UpdateVersionLabel() {
   if (version_text_.empty())
     return;
 
   chrome::VersionInfo version_info;
   std::string label_text = l10n_util::GetStringFUTF8(
       IDS_LOGIN_VERSION_LABEL_FORMAT,
       l10n_util::GetStringUTF16(IDS_PRODUCT_NAME),
@@ skipping 30 matching lines @@
 }
 
 void VersionInfoUpdater::OnStoreLoaded(policy::CloudPolicyStore* store) {
   UpdateEnterpriseInfo();
 }
 
 void VersionInfoUpdater::OnStoreError(policy::CloudPolicyStore* store) {
   UpdateEnterpriseInfo();
 }
 
-void VersionInfoUpdater::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  if (type == chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED)
-    UpdateEnterpriseInfo();
-  else
-    NOTREACHED();
-}
-
 }  // namespace chromeos