Create MediaRouterActionController and MediaRouterUIService

chrome/browser/extensions/component_migration_helper.cc

 // Copyright 2016 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/extensions/component_migration_helper.h"
 
 #include "base/stl_util.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system_impl.h"
@@ skipping 61 matching lines @@
     if (IsExtensionInstalledAndEnabled(id)) {
       extension_was_installed = true;
       UnloadExtension(id);
     }
   }
 
   // Read the pref to determine component action status.  If not set, set to
   // true if we unloaded an extension.
   const base::DictionaryValue* migration_pref = pref_service_->GetDictionary(
       ::prefs::kToolbarMigratedComponentActionStatus);
-  bool has_component_action_pref = migration_pref->HasKey(component_action_id);
   bool component_action_pref = false;
-  if (has_component_action_pref) {
-    bool success =
-        migration_pref->GetBoolean(component_action_id, &component_action_pref);
-    DCHECK(success);  // Shouldn't fail, but can in case of pref corruption.
-  }
-
-  if (!has_component_action_pref && extension_was_installed) {
+  if (migration_pref->HasKey(component_action_id)) {
+    component_action_pref = GetComponentActionPref(component_action_id);
+  } else if (extension_was_installed) {
     SetComponentActionPref(component_action_id, true);
     component_action_pref = true;
   }
 
   if (component_action_pref &&
       !delegate_->HasComponentAction(component_action_id)) {
     delegate_->AddComponentAction(component_action_id);
   }
 }
 
@@ skipping 31 matching lines @@
     return;
   if (base::ContainsKey(enabled_actions_, component_action_id)) {
     UnloadExtension(extension_id);
     SetComponentActionPref(component_action_id, true);
 
     if (!delegate_->HasComponentAction(component_action_id))
       delegate_->AddComponentAction(component_action_id);
   }
 }
 
+bool ComponentMigrationHelper::GetComponentActionPref(
+    const std::string& component_action_id) const {
+  const base::DictionaryValue* migration_pref = pref_service_->GetDictionary(
+      ::prefs::kToolbarMigratedComponentActionStatus);
+  bool component_action_pref = false;
+
+  migration_pref->GetBoolean(component_action_id, &component_action_pref);

[takumif, 2016/09/16 21:04:05]

I can't DCHECK here because the MRActionController may want to call this before
ToolbarActionsModel::OnReady() sets the pref for MRAction. I think it would make
sense for this method to return false in that case.

The DCHECK here was to detect pref corruption. Is that something we should
generally DCHECK for?

[Devlin, 2016/09/16 22:37:32]

The DCHECK was actually because it was always supposed to be set.  The comment
"// This shouldn't fail, but can in the cases of pref corruption" was explaining
why it couldn't instead be a CHECK.

If this can legitimately happen, this is fine.  But it's worth documenting.

[takumif, 2016/09/17 00:24:29]

I see. Yes, it happens when MRActionController observes media route changes
before ToolbarActionsModel is ready. Added a comment saying that GetBoolean()
might not modify the boolean.

+  return component_action_pref;
+}
+
 void ComponentMigrationHelper::SetComponentActionPref(
     const std::string& component_action_id,
     bool enabled) {
   DictionaryPrefUpdate update(pref_service_,
                               ::prefs::kToolbarMigratedComponentActionStatus);
   update->SetBoolean(component_action_id, enabled);
 }
 
 bool ComponentMigrationHelper::IsExtensionInstalledAndEnabled(
     const ExtensionId& extension_id) const {
@@ skipping 27 matching lines @@
 std::string ComponentMigrationHelper::GetActionIdForExtensionId(
     const ExtensionId& extension_id) const {
   for (const auto& i : migrated_actions_) {
     if (i.second == extension_id)
       return i.first;
   }
   return "";
 }
 
 }  // namespace extensions