Delay the metrics policy migration call to make sure ownership has been taken.

chrome/browser/chromeos/user_cros_settings_provider.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/chromeos/user_cros_settings_provider.h"
 
 #include <map>
 #include <set>
 
-#include "base/bind.h"
-#include "base/callback.h"
 #include "base/hash_tables.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/string_util.h"
 #include "base/task.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/login_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/cros_settings.h"
 #include "chrome/browser/chromeos/cros_settings_names.h"
 #include "chrome/browser/chromeos/login/ownership_service.h"
+#include "chrome/browser/chromeos/login/ownership_status_checker.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/ui/options/options_util.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "content/browser/browser_thread.h"
 
 namespace chromeos {
 
 namespace {
 
 const char kTrueIncantation[] = "true";
 const char kFalseIncantation[] = "false";
 const char kTrustedSuffix[] = "/trusted";
@@ skipping 11 matching lines @@
 
 const char* kStringSettings[] = {
   kDeviceOwner,
   kReleaseChannel
 };
 
 const char* kListSettings[] = {
   kAccountsPrefUsers
 };
 
-// Only write the property if the owner is the current logged on user.
-void StartStorePropertyOpIfOwner(const std::string& name,
-                                 const std::string& value,
-                                 SignedSettingsHelper::Callback* callback) {
-  if (OwnershipService::GetSharedInstance()->CurrentUserIsOwner()) {
-    BrowserThread::PostTask(BrowserThread::UI,
-                            FROM_HERE,
-                            base::Bind(
-                                &SignedSettingsHelper::StartStorePropertyOp,
-                                base::Unretained(SignedSettingsHelper::Get()),
-                                name,
-                                value,
-                                callback));
+// This class provides the means to migrate settings to the signed settings
+// store. It does one of three things - store the settings in the policy blob
+// immediately if the current user is the owner. Uses the
+// SignedSettingsTempStorage if there is no owner yet, or waits for an
+// OWNERSHIP_CHECKED notification to delay the storing until the owner has
+// logged in.
+class MigrationHelper : public NotificationObserver {
+ public:
+  explicit MigrationHelper() : callback_(NULL) {
+    registrar_.Add(this, chrome::NOTIFICATION_OWNERSHIP_CHECKED,
+                   NotificationService::AllSources());
   }
-}
+
+  void set_callback(SignedSettingsHelper::Callback* callback) {
+    callback_ = callback;
+  }
+
+  void AddMigrationValue(const std::string& path, const std::string& value) {
+    migration_values_[path] = value;
+  }
+
+  void MigrateValues(void) {
+    ownership_checker_.reset(new OwnershipStatusChecker(NewCallback(
+        this, &MigrationHelper::DoMigrateValues)));
+  }
+
+  void DoMigrateValues(OwnershipService::Status status,

[Mattias Nissler (ping if slow), 2011/08/31 15:08:33]

make this private

[pastarmovj, 2011/08/31 15:26:36]

Done.

+                       bool current_user_is_owner) {
+    ownership_checker_.reset(NULL);
+
+    if (current_user_is_owner || status != OwnershipService::OWNERSHIP_TAKEN) {

[Mattias Nissler (ping if slow), 2011/08/31 15:08:33]

Put a comment why we do this if ownership hasn't been taken yet (maybe move the
comment below in the empty else block up here)?

[pastarmovj, 2011/08/31 15:26:36]

Done.

+      std::map<std::string, std::string>::const_iterator i;
+      for (i = migration_values_.begin(); i != migration_values_.end(); ++i) {
+        // This is needed to avoid loosing the value after the clear below.

[Mattias Nissler (ping if slow), 2011/08/31 15:08:33]

Nope, this is not needed. A copy is created when constructing the
StorePropertyOpContext.

[pastarmovj, 2011/08/31 15:26:36]

Before I did need it because I was passing the result of a c_str instead of the
object and it was destroyed but not done anymore so removed. (Dunno why I
thought I need to pass pure char* instead of a string object).

On 2011/08/31 15:08:33, Mattias Nissler wrote:

+        // Temp objects passed by const ref will have the life time of the ref.
+        const std::string name = i->first;
+        const std::string value = i->second;
+        // Queue all values for storing.
+        SignedSettingsHelper::Get()->StartStorePropertyOp(name, value,
+                                                          callback_);
+      }
+      migration_values_.clear();
+    } else {
+      // Either we are not yet logged in or the user currently logged in is not
+      // the owner. So we should wait for user change. (Actually only the first
+      // case is interesting for us.)
+    }
+  }
+
+  // NotificationObserver overrides:
+  virtual void Observe(int type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) OVERRIDE {
+    if (type == chrome::NOTIFICATION_OWNERSHIP_CHECKED)
+      MigrateValues();
+  }
+
+ private:
+  NotificationRegistrar registrar_;
+  scoped_ptr<OwnershipStatusChecker> ownership_checker_;
+  SignedSettingsHelper::Callback* callback_;
+
+  std::map<std::string, std::string> migration_values_;
+
+  DISALLOW_COPY_AND_ASSIGN(MigrationHelper);
+};
 
 bool IsControlledBooleanSetting(const std::string& pref_path) {
   // TODO(nkostylev): Using std::find for 4 value array generates this warning
   // in chroot stl_algo.h:231: error: array subscript is above array bounds.
   // GCC 4.4.3
   return (pref_path == kAccountsPrefAllowNewUser) ||
          (pref_path == kAccountsPrefAllowGuest) ||
          (pref_path == kAccountsPrefShowUserNamesOnSignIn) ||
          (pref_path == kSignedDataRoamingEnabled) ||
          (pref_path == kStatsReportingPref);
@@ skipping 196 matching lines @@
     }
   }
 
  private:
   // upper bound for number of retries to fetch a signed setting.
   static const int kNumRetriesLimit = 9;
 
   UserCrosSettingsTrust()
       : ownership_service_(OwnershipService::GetSharedInstance()),
         retries_left_(kNumRetriesLimit) {
+    migration_helper_.set_callback(this);
     // Start prefetching Boolean and String preferences.
     Reload();
   }
 
   virtual ~UserCrosSettingsTrust() {
     if (BrowserThread::CurrentlyOn(BrowserThread::UI) &&
         CrosLibrary::Get()->EnsureLoaded()) {
       // Cancels all pending callbacks from us.
       SignedSettingsHelper::Get()->CancelCallback(this);
     }
@@ skipping 43 matching lines @@
       }
     } else if (path == kStatsReportingPref) {
       bool stats_consent = (use_value == USE_VALUE_SUPPLIED) ? value : false;
       // TODO(pastarmovj): Remove this once migration is not needed anymore.
       // If the value is not set we should try to migrate legacy consent file.
       if (use_value == USE_VALUE_DEFAULT) {
         // Loading consent file state causes us to do blocking IO on UI thread.
         // Temporarily allow it until we fix http://crbug.com/62626
         base::ThreadRestrictions::ScopedAllowIO allow_io;
         stats_consent = GoogleUpdateSettings::GetCollectStatsConsent();
-        // Only store settings if the owner is logged on, otherwise the write
-        // will fail, triggering another read and we'll end up in an infinite
-        // loop. Owner check needs to be done on the FILE thread.
-        BrowserThread::PostTask(BrowserThread::FILE,
-                                FROM_HERE,
-                                base::Bind(&StartStorePropertyOpIfOwner, path,
-                                           stats_consent ? "true" : "false",
-                                           this));
+        // Make sure the values will get eventually written to the policy file.
+        migration_helper_.AddMigrationValue(
+            path, stats_consent ? "true" : "false");
+        migration_helper_.MigrateValues();
         UpdateCacheBool(path, stats_consent, USE_VALUE_SUPPLIED);
         LOG(WARNING) << "No metrics policy set will revert to checking "
                      << "consent file which is "
                      << (stats_consent ? "on." : "off.");
       }
       // TODO(pastarmovj): Remove this once we don't need to regenerate the
       // consent file for the GUID anymore.
       VLOG(1) << "Metrics policy is being set to : " << stats_consent
               << "(reason : " << use_value << ")";
       OptionsUtil::ResolveMetricsReportingEnabled(stats_consent);
@@ skipping 111 matching lines @@
 
     // Reload the whitelist on settings op failure.
     if (code != SignedSettings::SUCCESS)
       CrosSettings::Get()->FireObservers(kAccountsPrefUsers);
   }
 
   // Pending callbacks that need to be invoked after settings verification.
   base::hash_map< std::string, std::vector< Task* > > callbacks_;
 
   OwnershipService* ownership_service_;
+  MigrationHelper migration_helper_;
 
   // In order to guard against occasional failure to fetch a property
   // we allow for some number of retries.
   int retries_left_;
 
   friend class SignedSettingsHelper;
   friend struct DefaultSingletonTraits<UserCrosSettingsTrust>;
 
   DISALLOW_COPY_AND_ASSIGN(UserCrosSettingsTrust);
 };
@@ skipping 194 matching lines @@
   if (cached_whitelist_update->Remove(email_value, NULL))
     prefs->ScheduleSavePersistentPrefs();
 }
 
 // static
 void UserCrosSettingsProvider::UpdateCachedOwner(const std::string& email) {
   UpdateCacheString(kDeviceOwner, email, USE_VALUE_SUPPLIED);
 }
 
 }  // namespace chromeos