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

chrome/browser/media/router/media_router_feature.cc

 // Copyright 2015 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/media/router/media_router_feature.h"
 
 #include "build/build_config.h"
 #include "content/public/browser/browser_context.h"
 
 #if defined(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/api/tab_capture/tab_capture_api.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/feature_switch.h"
 #endif
 
 namespace media_router {
 
+#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 false;
+  }
+
+  const extensions::ExtensionSet& extension_set =
+      registry->enabled_extensions();
+  return extension_set.Contains(extensions::kStableChromecastExtensionId) ||
+         extension_set.Contains(extensions::kBetaChromecastExtensionId);
+}
+
+}  // namespace
+#endif  // defined(ENABLE_MEDIA_ROUTER) && defined(ENABLE_EXTENSIONS)
+
 bool MediaRouterEnabled(content::BrowserContext* context) {
 #if defined(ENABLE_MEDIA_ROUTER)
 #if defined(OS_ANDROID)
   return true;
 #elif defined(ENABLE_EXTENSIONS)
-  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();
-  }
-
-  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 extensions::FeatureSwitch::media_router()->IsEnabled();
+  // |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.

 #else   // !defined(ENABLE_MEDIA_ROUTER)
   return false;
 #endif  // defined(ENABLE_MEDIA_ROUTER)
 }
 
 }  // namespace media_router