[Media Router] Ensure the same FeatureSwitch is used for lifetime of the

chrome/browser/media/router/media_router_feature.cc

Index: chrome/browser/media/router/media_router_feature.cc
diff --git a/chrome/browser/media/router/media_router_feature.cc b/chrome/browser/media/router/media_router_feature.cc
index 3491c43af96af3e241c76238da94eaca6ed2bbd4..20a0f765d790c5d7f96335a8fdd06bdac5ef6d36 100644
--- a/chrome/browser/media/router/media_router_feature.cc
+++ b/chrome/browser/media/router/media_router_feature.cc
@@ -16,28 +16,45 @@
 
 namespace media_router {
 
-bool MediaRouterEnabled(content::BrowserContext* context) {
-#if defined(ENABLE_MEDIA_ROUTER)
-#if defined(OS_ANDROID)
-  return true;
-#elif defined(ENABLE_EXTENSIONS)
+#if defined(ENABLE_MEDIA_ROUTER) && defined(ENABLE_EXTENSIONS)
+namespace {
+
+bool HasCastExtension(content::BrowserContext* context) {
   extensions::ExtensionRegistry* registry =
       extensions::ExtensionRegistry::Get(context);
   if (!registry) {
     DLOG(ERROR) << "ExtensionRegistry is null. Assume no cast extension "
                    "installed.";
-    return extensions::FeatureSwitch::media_router()->IsEnabled();
+    return false;
   }
 
   const extensions::ExtensionSet& extension_set =
       registry->enabled_extensions();
-  if (extension_set.Contains(extensions::kStableChromecastExtensionId) ||
-      extension_set.Contains(extensions::kBetaChromecastExtensionId)) {
-    return extensions::FeatureSwitch::media_router_with_cast_extension()
-        ->IsEnabled();
-  }
+  return extension_set.Contains(extensions::kStableChromecastExtensionId) ||
+         extension_set.Contains(extensions::kBetaChromecastExtensionId);
+}
+
+}  // namespace
+#endif  // defined(ENABLE_MEDIA_ROUTER) && defined(ENABLE_EXTENSIONS)
 
-  return extensions::FeatureSwitch::media_router()->IsEnabled();
+bool MediaRouterEnabled(content::BrowserContext* context) {
+#if defined(ENABLE_MEDIA_ROUTER)
+#if defined(OS_ANDROID)
+  return true;
+#elif defined(ENABLE_EXTENSIONS)
+  // |has_cast_extension| being static means that we determine which
+  // FeatureSwitch to use only on first invocation of this function. The same
+  // switch will be used for the lifetime of the browser. Note that the browser
+  // expects the same value to be returned by repeated calls to this function,
+  // since it will result in inconsistent behaviors otherwise.
+  // The caveat is we only use the BrowserContext the function is first called
+  // with to determine which field trial to use for the lifetime of the browser.
+  static bool has_cast_extension = HasCastExtension(context);

[mark a. foltz, 2016/04/21 19:30:56]

This needs to be per-|context|, since one profile might have an enabled
extension and another might not.

There's also an issue where an incognito profile might not have the extension
enabled, but the parent profile will.  In that case, the user would see
inconsistent results depending on which context was passed in (off-the-record or
not).

Finally, I'm not sure this solves the problem, since the extension will remain
unloaded after the migration, so users could have MR enabled by the
'media_router_with_cast_extension' experiment on browser startup, then disabled
on subsequent starts because they are out of the 'media_router' experiment.

+  extensions::FeatureSwitch* feature_switch =
+      has_cast_extension
+          ? extensions::FeatureSwitch::media_router_with_cast_extension()
+          : extensions::FeatureSwitch::media_router();
+  return feature_switch->IsEnabled();
 #else  // !defined(ENABLE_EXTENSIONS)
   return false;
 #endif  // defined(OS_ANDROID)

[mark a. foltz, 2016/04/21 19:30:56]

There are only two #if's above, but there are three #endif's below.