Create MediaRouterActionController and MediaRouterUIService

chrome/browser/extensions/component_migration_helper.cc

Index: chrome/browser/extensions/component_migration_helper.cc
diff --git a/chrome/browser/extensions/component_migration_helper.cc b/chrome/browser/extensions/component_migration_helper.cc
index 7de2caa9773e3e49033483107317284b888d23b4..1c88a10177f881a18e777d4f1173255ec12ffeb5 100644
--- a/chrome/browser/extensions/component_migration_helper.cc
+++ b/chrome/browser/extensions/component_migration_helper.cc
@@ -79,15 +79,10 @@ void ComponentMigrationHelper::OnFeatureEnabled(
   // 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;
   }
@@ -139,6 +134,16 @@ void ComponentMigrationHelper::OnExtensionReady(
   }
 }
 
+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) {