Fix losing of scoped feature channel in tests when it's equal to stable

This fixes the issue with the value set by ScopedCurrentChannel
potentially being overwritten by the browser process initialization
code.

This could happen when the ScopedCurrentChannel instance was constructed
with the default channel (that is, the STABLE channel) before the browser
initialization code happens, as the latter would then assume that the channel
wasn't set before and overwrite it.

This issue means that it was impossible to set up the STABLE channel in
the browser test in the setup code that runs before the browser initialization,
unless the building environment was configured to be stable.

BUG=none, 626343
TEST=existing tests

Review-Url: https://codereview.chromium.org/2854293003
Cr-Commit-Position: refs/heads/master@{#473402}

chrome/browser/extensions/chrome_extensions_browser_client.cc

@@ -74,9 +74,7 @@ namespace extensions {
 ChromeExtensionsBrowserClient::ChromeExtensionsBrowserClient() {
   process_manager_delegate_.reset(new ChromeProcessManagerDelegate);
   api_client_.reset(new ChromeExtensionsAPIClient);
-  // Only set if it hasn't already been set (e.g. by a test).
-  if (GetCurrentChannel() == GetDefaultChannel())
-    SetCurrentChannel(chrome::GetChannel());
+  SetCurrentChannel(chrome::GetChannel());
   resource_manager_.reset(new ChromeComponentExtensionResourceManager());
 }
 

extensions/common/features/feature_channel.cc

@@ -4,37 +4,49 @@
 
 #include "extensions/common/features/feature_channel.h"
 
+#include "base/logging.h"
 #include "components/version_info/version_info.h"
 
 namespace {
 
-const version_info::Channel kDefaultChannel = version_info::Channel::STABLE;
-version_info::Channel g_current_channel = kDefaultChannel;
+// The current channel to be reported, unless overridden by
+// |ScopedCurrentChannel|.
+version_info::Channel g_current_channel = version_info::Channel::STABLE;
+
+// The current channel overridden by |ScopedCurrentChannel|. The value is valid
+// only whenever |g_override_count| is non-zero.
+version_info::Channel g_overridden_channel = version_info::Channel::STABLE;
+
+// The number of currently existing instances of |ScopedCurrentChannel|.
+int g_override_count = 0;
 
 }  // namespace
 
 namespace extensions {
 
 version_info::Channel GetCurrentChannel() {
-  return g_current_channel;
+  return g_override_count ? g_overridden_channel : g_current_channel;
 }
 
 void SetCurrentChannel(version_info::Channel channel) {
   g_current_channel = channel;
 }
 
-version_info::Channel GetDefaultChannel() {
-  return kDefaultChannel;
-}
-
 ScopedCurrentChannel::ScopedCurrentChannel(version_info::Channel channel)
-    : original_channel_(version_info::Channel::UNKNOWN) {
-  original_channel_ = GetCurrentChannel();
-  SetCurrentChannel(channel);
+    : channel_(channel),
+      original_overridden_channel_(g_overridden_channel),
+      original_override_count_(g_override_count) {
+  g_overridden_channel = channel;
+  ++g_override_count;
 }
 
 ScopedCurrentChannel::~ScopedCurrentChannel() {
-  SetCurrentChannel(original_channel_);
+  DCHECK_EQ(original_override_count_ + 1, g_override_count)
+      << "Scoped channel setters are not nested properly";
+  DCHECK_EQ(g_overridden_channel, channel_)
+      << "Scoped channel setters are not nested properly";
+  g_overridden_channel = original_overridden_channel_;
+  --g_override_count;
 }
 
 }  // namespace extensions

extensions/common/features/feature_channel.h

@@ -18,20 +18,22 @@ version_info::Channel GetCurrentChannel();
 
 // Sets the current channel as seen by the Feature system. In the browser
 // process this should be chrome::GetChannel(), and in the renderer this will
-// need to come from an IPC.
+// need to come from an IPC. Note that the value set through this function may
+// be overridden by |ScopedCurrentChannel|.
 void SetCurrentChannel(version_info::Channel channel);
 
-// Gets the default channel as seen by the Feature system.
-version_info::Channel GetDefaultChannel();
-
 // Scoped channel setter. Use for tests.
+// Note that the lifetimes of multiple instances of this class must be disjoint
+// or nested, but never overlapping.
 class ScopedCurrentChannel {
  public:
   explicit ScopedCurrentChannel(version_info::Channel channel);
   ~ScopedCurrentChannel();
 
  private:
-  version_info::Channel original_channel_;
+  const version_info::Channel channel_;
+  const version_info::Channel original_overridden_channel_;
+  const int original_override_count_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedCurrentChannel);
 };