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

extensions/common/features/feature_channel.cc

Index: extensions/common/features/feature_channel.cc
diff --git a/extensions/common/features/feature_channel.cc b/extensions/common/features/feature_channel.cc
index 13bd6e1dcaee847aaccae4e4510fac40a71aea5f..1dff549bd62273cb0a4ee5591c8c378faa3eebbd 100644
--- a/extensions/common/features/feature_channel.cc
+++ b/extensions/common/features/feature_channel.cc
@@ -4,37 +4,45 @@
 
 #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;

[Devlin, 2017/05/19 22:14:17]

nit: \n

[emaxx, 2017/05/20 02:48:53]

Done.

+// The current channel overridden by |ScopedCurrentChannel|. The value is valid
+// only whenever |g_channel_is_overridden| is true.
+version_info::Channel g_overridden_channel = version_info::Channel::STABLE;

[Devlin, 2017/05/19 22:14:17]

\n

[emaxx, 2017/05/20 02:48:53]

Done.

+// Whether an instance of |ScopedCurrentChannel| exists currently.
+bool g_channel_is_overridden = false;
 
 }  // namespace
 
 namespace extensions {
 
 version_info::Channel GetCurrentChannel() {
-  return g_current_channel;
+  return g_channel_is_overridden ? 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_channel_is_overridden_(g_channel_is_overridden) {
+  g_overridden_channel = channel;
+  g_channel_is_overridden = true;
 }
 
 ScopedCurrentChannel::~ScopedCurrentChannel() {
-  SetCurrentChannel(original_channel_);
+  DCHECK_EQ(g_overridden_channel, channel_)
+      << "Scoped channel setters are not nested properly";

[Devlin, 2017/05/19 22:14:17]

One last bit of bikeshedding :)

We could actually combine this approach with your previous counter approach, and
do something like:

int g_override_count = 0;

ScopedCurrentChannel::ScopedCurrentChannel(version_info::Channel channel)
    : channel_(channel),
      original_overridden_channel_(g_overridden_channel),
      original_override_count_(g_override_count) {
  g_overridden_channel = channel;
  ++g_override_count;
}

ScopedCurrentChannel::~ScopedCurrentChannel() {
  DCHECK_EQ(original_override_count_ + 1, g_override_count)
      << "Scoped channel setters are not nested properly";
  g_overridden_channel = original_overridden_channel_;
  --g_override_count;
}

That way, we even catch the case of scoped channels not being properly nested
when they have the same values (which would be a problem, because the original
channel could be different).

[emaxx, 2017/05/20 02:48:53]

Done.

+  g_overridden_channel = original_overridden_channel_;
+  g_channel_is_overridden = original_channel_is_overridden_;
 }
 
 }  // namespace extensions