Properly propagate the current Chrome channel into the Feature system on the

renderer.


BUG=141093


Review URL: https://chromiumcodereview.appspot.com/10826199

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@150731 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/active_tab_unittest.cc

@@ -58,13 +58,8 @@ class ActiveTabTest : public TabContentsTestHarness {
       : extension(CreateTestExtension("deadbeef", true)),
         another_extension(CreateTestExtension("feedbeef", true)),
         extension_without_active_tab(CreateTestExtension("badbeef", false)),
-        ui_thread_(BrowserThread::UI, MessageLoop::current()) {
-  }
-
-  virtual void SetUp() {
-    TabContentsTestHarness::SetUp();
-    Feature::SetChannelForTesting(chrome::VersionInfo::CHANNEL_UNKNOWN);
-  }
+        ui_thread_(BrowserThread::UI, MessageLoop::current()),
+        current_channel_(chrome::VersionInfo::CHANNEL_DEV) {}
 
  protected:
   int tab_id() {
@@ -122,6 +117,7 @@ class ActiveTabTest : public TabContentsTestHarness {
 
  private:
   content::TestBrowserThread ui_thread_;
+  Feature::ScopedCurrentChannel current_channel_;
 };
 
 TEST_F(ActiveTabTest, GrantToSinglePage) {

chrome/browser/extensions/extension_browsertest.cc

@@ -29,7 +29,7 @@
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/extensions/features/feature.h"
+#include "chrome/common/chrome_version_info.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
@@ -57,9 +57,6 @@ ExtensionBrowserTest::~ExtensionBrowserTest() {
 }
 
 void ExtensionBrowserTest::SetUpCommandLine(CommandLine* command_line) {
-  extensions::Feature::SetChannelForTesting(
-      chrome::VersionInfo::CHANNEL_UNKNOWN);
-
   PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);
   test_data_dir_ = test_data_dir_.AppendASCII("extensions");
 

chrome/browser/extensions/extension_system.cc

@@ -36,6 +36,7 @@
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 #include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/chrome_version_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/features/feature.h"
 #include "chrome/common/pref_names.h"
@@ -50,10 +51,7 @@ namespace extensions {
 //
 
 ExtensionSystem::ExtensionSystem() {
-  // In lieu of a way for Feature to check whether it's running on the browser
-  // process, tell it.
-  // See http://crbug.com/126535.
-  Feature::SetChannelCheckingEnabled(true);
+  Feature::SetCurrentChannel(chrome::VersionInfo::GetChannel());
 }
 
 ExtensionSystem::~ExtensionSystem() {

chrome/browser/extensions/script_badge_controller_unittest.cc

@@ -15,8 +15,10 @@
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tab_contents/test_tab_contents.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_version_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_builder.h"
+#include "chrome/common/extensions/features/feature.h"
 #include "chrome/common/extensions/value_builder.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/browser_thread.h"
@@ -36,7 +38,8 @@ class ScriptBadgeControllerTest : public TabContentsTestHarness {
  public:
   ScriptBadgeControllerTest()
       : ui_thread_(BrowserThread::UI, MessageLoop::current()),
-        file_thread_(BrowserThread::FILE, MessageLoop::current()) {}
+        file_thread_(BrowserThread::FILE, MessageLoop::current()),
+        current_channel_(chrome::VersionInfo::CHANNEL_DEV) {}
 
   virtual void SetUp() OVERRIDE {
     // Note that this sets a PageActionController into the
@@ -83,6 +86,7 @@ class ScriptBadgeControllerTest : public TabContentsTestHarness {
  private:
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
+  Feature::ScopedCurrentChannel current_channel_;
 };
 
 struct CountingNotificationObserver : public content::NotificationObserver {

chrome/common/extensions/features/feature.cc

@@ -49,7 +49,7 @@ struct Mappings {
   std::map<std::string, VersionInfo::Channel> channels;
 };
 
-static base::LazyInstance<Mappings> g_mappings = LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<Mappings> g_mappings = LAZY_INSTANCE_INITIALIZER;
 
 std::string GetChannelName(VersionInfo::Channel channel) {
   typedef std::map<std::string, VersionInfo::Channel> ChannelsMap;
@@ -62,30 +62,7 @@ std::string GetChannelName(VersionInfo::Channel channel) {
   return "unknown";
 }
 
-static bool g_channel_checking_enabled = false;
-
-class Channel {
- public:
-  VersionInfo::Channel GetChannel() {
-    CHECK(g_channel_checking_enabled);
-    if (channel_for_testing_.get())
-      return *channel_for_testing_;
-    return VersionInfo::GetChannel();
-  }
-
-  void SetChannelForTesting(VersionInfo::Channel channel_for_testing) {
-    channel_for_testing_.reset(new VersionInfo::Channel(channel_for_testing));
-  }
-
-  void ResetChannelForTesting() {
-    channel_for_testing_.reset();
-  }
-
- private:
-  scoped_ptr<VersionInfo::Channel> channel_for_testing_;
-};
-
-static base::LazyInstance<Channel> g_channel = LAZY_INSTANCE_INITIALIZER;
+VersionInfo::Channel g_current_channel = VersionInfo::CHANNEL_STABLE;
 
 // TODO(aa): Can we replace all this manual parsing with JSON schema stuff?
 
@@ -320,10 +297,8 @@ Feature::Availability Feature::IsAvailableToManifest(
   if (max_manifest_version_ != 0 && manifest_version > max_manifest_version_)
     return INVALID_MAX_MANIFEST_VERSION;
 
-  if (g_channel_checking_enabled) {
-    if (channel_ < g_channel.Get().GetChannel())
-      return UNSUPPORTED_CHANNEL;
-  }
+  if (channel_ < g_current_channel)
+    return UNSUPPORTED_CHANNEL;
 
   return IS_AVAILABLE;
 }
@@ -350,30 +325,13 @@ Feature::Availability Feature::IsAvailableToContext(
 }
 
 // static
-void Feature::SetChannelCheckingEnabled(bool enabled) {
-  g_channel_checking_enabled = enabled;
-}
-
-// static
-void Feature::ResetChannelCheckingEnabled() {
-  g_channel_checking_enabled = false;
-}
-
-// static
-void Feature::SetChannelForTesting(VersionInfo::Channel channel) {
-  g_channel.Get().SetChannelForTesting(channel);
-}
-
-// static
-void Feature::ResetChannelForTesting() {
-  g_channel.Get().ResetChannelForTesting();
+chrome::VersionInfo::Channel Feature::GetCurrentChannel() {
+  return g_current_channel;
 }
 
 // static
-chrome::VersionInfo::Channel Feature::GetCurrentChannel() {
-  if (g_channel_checking_enabled)
-    return g_channel.Get().GetChannel();
-  return chrome::VersionInfo::GetChannel();
+void Feature::SetCurrentChannel(VersionInfo::Channel channel) {
+  g_current_channel = channel;
 }
 
 }  // namespace

chrome/common/extensions/features/feature.h

@@ -68,23 +68,32 @@ class Feature {
   Feature(const Feature& other);
   virtual ~Feature();
 
-  // (Re)Sets whether checking against "channel" should be done. This must only
-  // be called on the browser process, since the check involves accessing the
-  // filesystem.
-  // See http://crbug.com/126535.
-  static void SetChannelCheckingEnabled(bool enabled);
-  static void ResetChannelCheckingEnabled();
-
-  // (Re)Sets the Channel to for all Features to compare against. This is
-  // usually chrome::VersionInfo::GetChannel(), but for tests allow this to be
-  // overridden.
-  static void SetChannelForTesting(chrome::VersionInfo::Channel channel);
-  static void ResetChannelForTesting();
-
-  // Returns the current channel as seen by the Feature system (i.e. the
-  // ChannelForTesting if one is set, otherwise the actual channel).
+  // Gets the current channel as seen by the Feature system.
+  // Defaults to CHANNEL_STABLE.
   static chrome::VersionInfo::Channel GetCurrentChannel();
 
+  // Sets the current channel as seen by the Feature system. In the browser
+  // process this should be chrome::VersionInfo::GetChannel(), and in the
+  // renderer this will need to come from an IPC.
+  static void SetCurrentChannel(chrome::VersionInfo::Channel channel);
+
+  // Scoped channel setter. Use for tests.
+  class ScopedCurrentChannel {
+   public:
+    explicit ScopedCurrentChannel(chrome::VersionInfo::Channel channel)
+        : original_channel_(chrome::VersionInfo::CHANNEL_UNKNOWN) {
+      original_channel_ = GetCurrentChannel();
+      SetCurrentChannel(channel);
+    }
+
+    ~ScopedCurrentChannel() {
+      SetCurrentChannel(original_channel_);
+    }
+
+   private:
+    chrome::VersionInfo::Channel original_channel_;
+  };
+
   const std::string& name() const { return name_; }
   void set_name(const std::string& name) { name_ = name; }
 

chrome/common/extensions/features/feature_unittest.cc

@@ -454,8 +454,7 @@ TEST(ExtensionFeatureTest, Equals) {
 
 Feature::Availability IsAvailableInChannel(
     const std::string& channel, VersionInfo::Channel channel_for_testing) {
-  Feature::SetChannelCheckingEnabled(true);
-  Feature::SetChannelForTesting(channel_for_testing);
+  Feature::ScopedCurrentChannel current_channel(channel_for_testing);
 
   Feature feature;
   if (!channel.empty()) {
@@ -464,15 +463,11 @@ Feature::Availability IsAvailableInChannel(
     feature.Parse(&feature_value);
   }
 
-  Feature::Availability availability = feature.IsAvailableToManifest(
+  return feature.IsAvailableToManifest(
       "random-extension",
       Extension::TYPE_UNKNOWN,
       Feature::UNSPECIFIED_LOCATION,
       -1);
-
-  Feature::ResetChannelForTesting();
-  Feature::ResetChannelCheckingEnabled();
-  return availability;
 }
 
 TEST(ExtensionFeatureTest, SupportedChannel) {

chrome/common/extensions/manifest_tests/extension_manifests_background_unittest.cc

@@ -89,7 +89,8 @@ TEST_F(ExtensionManifestTest, BackgroundAllowNoJsAccess) {
 TEST_F(ExtensionManifestTest, BackgroundPageWebRequest) {
   CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableExperimentalExtensionApis);
-  Feature::SetChannelForTesting(chrome::VersionInfo::CHANNEL_UNKNOWN);
+  Feature::ScopedCurrentChannel current_channel(
+      chrome::VersionInfo::CHANNEL_DEV);
 
   std::string error;
   scoped_ptr<DictionaryValue> manifest(

chrome/renderer/extensions/dispatcher.cc

@@ -219,22 +219,6 @@ class ProcessInfoNativeHandler : public ChromeV8Extension {
   int manifest_version_;
 };
 
-class ChannelNativeHandler : public NativeHandler {
- public:
-  explicit ChannelNativeHandler(chrome::VersionInfo::Channel channel)
-      : channel_(channel) {
-    RouteFunction("IsDevChannel",
-        base::Bind(&ChannelNativeHandler::IsDevChannel,
-                   base::Unretained(this)));
-  }
-
-  v8::Handle<v8::Value> IsDevChannel(const v8::Arguments& args) {
-    return v8::Boolean::New(channel_ <= chrome::VersionInfo::CHANNEL_DEV);
-  }
-
-  chrome::VersionInfo::Channel channel_;
-};
-
 class LoggingNativeHandler : public NativeHandler {
  public:
   LoggingNativeHandler() {
@@ -303,8 +287,7 @@ Dispatcher::Dispatcher()
       webrequest_adblock_(false),
       webrequest_adblock_plus_(false),
       webrequest_other_(false),
-      source_map_(&ResourceBundle::GetSharedInstance()),
-      chrome_channel_(chrome::VersionInfo::CHANNEL_UNKNOWN) {
+      source_map_(&ResourceBundle::GetSharedInstance()) {
   const CommandLine& command_line = *(CommandLine::ForCurrentProcess());
   is_extension_process_ =
       command_line.HasSwitch(switches::kExtensionProcess) ||
@@ -398,7 +381,8 @@ void Dispatcher::OnSetFunctionNames(
 }
 
 void Dispatcher::OnSetChannel(int channel) {
-  chrome_channel_ = channel;
+  extensions::Feature::SetCurrentChannel(
+      static_cast<chrome::VersionInfo::Channel>(channel));
 }
 
 void Dispatcher::OnMessageInvoke(const std::string& extension_id,
@@ -755,13 +739,9 @@ void Dispatcher::DidCreateScriptContext(
       scoped_ptr<NativeHandler>(new PrintNativeHandler()));
   module_system->RegisterNativeHandler("lazy_background_page",
       scoped_ptr<NativeHandler>(new LazyBackgroundPageNativeHandler(this)));
-  module_system->RegisterNativeHandler("channel",
-      scoped_ptr<NativeHandler>(new ChannelNativeHandler(
-          static_cast<chrome::VersionInfo::Channel>(chrome_channel_))));
   module_system->RegisterNativeHandler("logging",
       scoped_ptr<NativeHandler>(new LoggingNativeHandler()));
 
-
   int manifest_version = extension ? extension->manifest_version() : 1;
   module_system->RegisterNativeHandler("process",
       scoped_ptr<NativeHandler>(new ProcessInfoNativeHandler(

chrome/renderer/extensions/dispatcher.h

@@ -256,10 +256,6 @@ class Dispatcher : public content::RenderProcessObserver {
   // Sends API requests to the extension host.
   scoped_ptr<RequestSender> request_sender_;
 
-  // The current channel. From VersionInfo::GetChannel().
-  // TODO(aa): Remove when we can restrict non-permission APIs to dev-only.
-  int chrome_channel_;
-
   DISALLOW_COPY_AND_ASSIGN(Dispatcher);
 };
 

chrome/renderer/resources/extensions/schema_generated_bindings.js

@@ -14,7 +14,6 @@
       requireNative('apiDefinitions').GetExtensionAPIDefinition;
   var sendRequest = require('sendRequest').sendRequest;
   var utils = require('utils');
-  var isDevChannel = requireNative('channel').IsDevChannel;
   var chromeHidden = requireNative('chrome_hidden').GetChromeHidden();
   var schemaUtils = require('schemaUtils');