Handle unsucessful JsonPrefStore initialization in SupervisedUserSettingsService.

chrome/browser/supervised_user/supervised_user_settings_service.cc

 // Copyright 2014 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/supervised_user/supervised_user_settings_service.h"
 
 #include "base/callback.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/prefs/json_pref_store.h"
@@ skipping 32 matching lines @@
 
 namespace {
 
 bool SettingShouldApplyToPrefs(const std::string& name) {
   return !StartsWithASCII(name, kSupervisedUserInternalItemPrefix, false);
 }
 
 }  // namespace
 
 SupervisedUserSettingsService::SupervisedUserSettingsService()
-    : active_(false), local_settings_(new base::DictionaryValue) {}
+    : active_(false),
+      initialization_failed_(false),
+      local_settings_(new base::DictionaryValue) {
+}
 
 SupervisedUserSettingsService::~SupervisedUserSettingsService() {}
 
 void SupervisedUserSettingsService::Init(
     base::FilePath profile_path,
     base::SequencedTaskRunner* sequenced_task_runner,
     bool load_synchronously) {
   base::FilePath path =
       profile_path.Append(chrome::kSupervisedUserSettingsFilename);
   PersistentPrefStore* store = new JsonPrefStore(
       path, sequenced_task_runner, scoped_ptr<PrefFilter>());
   Init(store);
   if (load_synchronously) {
     store_->ReadPrefs();
     // TODO(bauerb): Temporary CHECK while investigating
     // https://crbug.com/425785. Remove (or change to DCHECK) once the bug
     // is fixed.
     CHECK(store_->IsInitializationComplete());

[Marc Treib, 2015/02/27 15:51:43]

Do you need to handle a failing PrefStore initialization here as well?

[Bernhard Bauer, 2015/02/27 16:03:44]

Interestingly enough, this CHECK does not seem to fail (I could imagine because
if the profile directory doesn't exist, the synchronous case already fails
before that). I'm tempted to keep the CHECK at least while this bug is still
investigated, so that we learn if that assumption turns out not to be the case.

[Marc Treib, 2015/02/27 16:08:32]

Okay, fair enough. LGTM then.

   } else {
     store_->ReadPrefsAsync(NULL);
   }
 }
 
 void SupervisedUserSettingsService::Init(
     scoped_refptr<PersistentPrefStore> store) {
   DCHECK(!store_.get());
   store_ = store;
   store_->AddObserver(this);
 }
 
 void SupervisedUserSettingsService::Subscribe(
     const SettingsCallback& callback) {
   if (IsReady()) {
     scoped_ptr<base::DictionaryValue> settings = GetSettings();
     callback.Run(settings.get());
   }
 
   subscribers_.push_back(callback);
 }
 
 void SupervisedUserSettingsService::SetActive(bool active) {
   active_ = active;
   InformSubscribers();
 }
 
 bool SupervisedUserSettingsService::IsReady() {
-  return store_->IsInitializationComplete();
+  // Initialization cannot be complete but have failed at the same time.
+  DCHECK(!(store_->IsInitializationComplete() && initialization_failed_));
+  return initialization_failed_ || store_->IsInitializationComplete();
 }
 
 void SupervisedUserSettingsService::Clear() {
   store_->RemoveValue(kAtomicSettings);
   store_->RemoveValue(kSplitSettings);
 }
 
 // static
 std::string SupervisedUserSettingsService::MakeSplitSettingKey(
     const std::string& prefix,
@@ skipping 181 matching lines @@
   InformSubscribers();
 
   SyncError error;
   return error;
 }
 
 void SupervisedUserSettingsService::OnPrefValueChanged(const std::string& key) {
 }
 
 void SupervisedUserSettingsService::OnInitializationCompleted(bool success) {
+  if (!success) {
+    // If this happens, it means the profile directory was not found. There is
+    // not much we can do, but the whole profile will probably be useless
+    // anyway. Just mark initialization as failed and continue otherwise,
+    // because subscribers might still expect to be called back.
+    initialization_failed_ = true;
+  }
+
   // TODO(bauerb): Temporary CHECK while investigating https://crbug.com/425785.
   // Remove (or change back to DCHECK) once the bug is fixed.
-  CHECK(success);
   CHECK(IsReady());
   InformSubscribers();
 }
 
 base::DictionaryValue* SupervisedUserSettingsService::GetOrCreateDictionary(
     const std::string& key) const {
   base::Value* value = NULL;
   base::DictionaryValue* dict = NULL;
   if (store_->GetMutableValue(key, &value)) {
     bool success = value->GetAsDictionary(&dict);
@@ skipping 32 matching lines @@
     dict = new base::DictionaryValue;
     DCHECK(!split_settings->HasKey(prefix));
     split_settings->Set(prefix, dict);
   }
   key->erase(0, pos + 1);
   return dict;
 }
 
 scoped_ptr<base::DictionaryValue> SupervisedUserSettingsService::GetSettings() {
   DCHECK(IsReady());
-  if (!active_)
+  if (!active_ || initialization_failed_)
     return scoped_ptr<base::DictionaryValue>();
 
   scoped_ptr<base::DictionaryValue> settings(local_settings_->DeepCopy());
 
   base::DictionaryValue* atomic_settings = GetAtomicSettings();
   for (base::DictionaryValue::Iterator it(*atomic_settings); !it.IsAtEnd();
        it.Advance()) {
     if (!SettingShouldApplyToPrefs(it.key()))
       continue;
 
@@ skipping 13 matching lines @@
 }
 
 void SupervisedUserSettingsService::InformSubscribers() {
   if (!IsReady())
     return;
 
   scoped_ptr<base::DictionaryValue> settings = GetSettings();
   for (const auto& callback : subscribers_)
     callback.Run(settings.get());
 }