Extensions: make error console enabled if profile is in dev mode

Bug: 863145
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ia8b82930d525b9ba3eee1e3332da1d5e35809131
Reviewed-on: https://chromium-review.googlesource.com/1141220
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576356}

chrome/browser/extensions/error_console/error_console.cc

@@ -175,15 +175,7 @@ void ErrorConsole::RemoveObserver(Observer* observer) {
 }
 
 bool ErrorConsole::IsEnabledForChromeExtensionsPage() const {
-  if (!profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode)) {
-    return false;  // Only enabled in developer mode.
-  }
-  // If there is a command line switch or override, respect that.
-  if (FeatureSwitch::error_console()->HasValue()) {
-    return FeatureSwitch::error_console()->IsEnabled();
-  }
-  // Enable by default on dev channel, disabled on other channels.
-  return GetCurrentChannel() <= version_info::Channel::DEV;
+  return profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode);
 }
 
 bool ErrorConsole::IsEnabledForAppsDeveloperTools() const {

chrome/browser/extensions/error_console/error_console_unittest.cc

@@ -57,12 +57,6 @@ class ErrorConsoleUnitTest : public testing::Test {
 
 // Test that the error console is enabled/disabled appropriately.
 TEST_F(ErrorConsoleUnitTest, EnableAndDisableErrorConsole) {
-  // Start in Dev Channel, without the feature switch.
-  std::unique_ptr<ScopedCurrentChannel> channel_override(
-      new ScopedCurrentChannel(version_info::Channel::DEV));
-  ASSERT_EQ(version_info::Channel::DEV, GetCurrentChannel());
-  FeatureSwitch::error_console()->SetOverrideValue(
-      FeatureSwitch::OVERRIDE_DISABLED);
   profile_->GetPrefs()->SetBoolean(prefs::kExtensionsUIDeveloperMode, false);
 
   // At the start, the error console should be disabled because the user is not
@@ -72,8 +66,6 @@ TEST_F(ErrorConsoleUnitTest, EnableAndDisableErrorConsole) {
   EXPECT_FALSE(error_console_->IsEnabledForAppsDeveloperTools());
 
   // Switch the error_console on.
-  FeatureSwitch::error_console()->SetOverrideValue(
-      FeatureSwitch::OVERRIDE_ENABLED);
   profile_->GetPrefs()->SetBoolean(prefs::kExtensionsUIDeveloperMode, true);
 
   // The error console should now be enabled, and specifically enabled for the
@@ -88,36 +80,8 @@ TEST_F(ErrorConsoleUnitTest, EnableAndDisableErrorConsole) {
   EXPECT_FALSE(error_console_->IsEnabledForChromeExtensionsPage());
   EXPECT_FALSE(error_console_->IsEnabledForAppsDeveloperTools());
 
-  // Similarly, if we change the current to less fun than Dev, ErrorConsole
-  // should be disabled.
-  channel_override.reset();
-  channel_override.reset(
-      new ScopedCurrentChannel(version_info::Channel::BETA));
-  FeatureSwitch::error_console()->SetOverrideValue(
-      FeatureSwitch::OVERRIDE_NONE);
-  profile_->GetPrefs()->SetBoolean(prefs::kExtensionsUIDeveloperMode, true);
-  EXPECT_FALSE(error_console_->enabled());
-  EXPECT_FALSE(error_console_->IsEnabledForChromeExtensionsPage());
-  EXPECT_FALSE(error_console_->IsEnabledForAppsDeveloperTools());
-
-  // If we add the feature switch, that should override the channel.
-  FeatureSwitch::error_console()->SetOverrideValue(
-      FeatureSwitch::OVERRIDE_ENABLED);
-  ASSERT_TRUE(FeatureSwitch::error_console()->IsEnabled());
-  // We use a pref mod to "poke" the ErrorConsole, because it needs an
-  // indication that something changed (FeatureSwitches don't change in a real
-  // environment, so ErrorConsole doesn't listen for them).
-  profile_->GetPrefs()->SetBoolean(prefs::kExtensionsUIDeveloperMode, false);
-  profile_->GetPrefs()->SetBoolean(prefs::kExtensionsUIDeveloperMode, true);
-  EXPECT_TRUE(error_console_->enabled());
-  EXPECT_TRUE(error_console_->IsEnabledForChromeExtensionsPage());
-  EXPECT_FALSE(error_console_->IsEnabledForAppsDeveloperTools());
-
-  // Next, remove the feature switch (turning error console off), and install
-  // the Apps Developer Tools. If we have Apps Developer Tools, Error Console
-  // should be enabled by default.
-  FeatureSwitch::error_console()->SetOverrideValue(
-      FeatureSwitch::OVERRIDE_DISABLED);
+  // Installing the Chrome Apps and Extensions Developer Tools should enable
+  // the ErrorConsole, same as if the profile were in developer mode.
   const char kAppsDeveloperToolsExtensionId[] =
       "ohmmkhmmmpcnpikjeljgnaoabkaalbgc";
   scoped_refptr<Extension> adt =
@@ -144,6 +108,26 @@ TEST_F(ErrorConsoleUnitTest, EnableAndDisableErrorConsole) {
   EXPECT_FALSE(error_console_->IsEnabledForAppsDeveloperTools());
 }
 
+// Test that the error console is enabled for all channels.
+TEST_F(ErrorConsoleUnitTest, EnabledForAllChannels) {
+  version_info::Channel channels[] = {
+      version_info::Channel::UNKNOWN, version_info::Channel::CANARY,
+      version_info::Channel::DEV, version_info::Channel::BETA,
+      version_info::Channel::STABLE};
+  for (const version_info::Channel channel : channels) {
+    ScopedCurrentChannel channel_override(channel);
+    ASSERT_EQ(channel, GetCurrentChannel());
+
+    profile_->GetPrefs()->SetBoolean(prefs::kExtensionsUIDeveloperMode, false);
+    EXPECT_FALSE(error_console_->enabled());
+    EXPECT_FALSE(error_console_->IsEnabledForChromeExtensionsPage());
+
+    profile_->GetPrefs()->SetBoolean(prefs::kExtensionsUIDeveloperMode, true);
+    EXPECT_TRUE(error_console_->enabled());
+    EXPECT_TRUE(error_console_->IsEnabledForChromeExtensionsPage());
+  }
+}
+
 // Test that errors are successfully reported. This is a simple test, since it
 // is tested more thoroughly in extensions/browser/error_map_unittest.cc
 TEST_F(ErrorConsoleUnitTest, ReportErrors) {

chrome/browser/resources/md_extensions/error_page.js

@@ -76,6 +76,13 @@ cr.define('extensions', function() {
       /** @type {!extensions.ErrorPageDelegate|undefined} */
       delegate: Object,
 
+      // Whether or not dev mode is enabled.
+      inDevMode: {
+        type: Boolean,
+        value: false,
+        observer: 'onInDevModeChanged_',
+      },
+
       /** @private {!Array<!(ManifestError|RuntimeError)>} */
       entries_: Array,
 
@@ -198,6 +205,16 @@ cr.define('extensions', function() {
       e.stopPropagation();
     },
 
+    /** private */
+    onInDevModeChanged_: function() {
+      if (!this.inDevMode) {
+        // Wait until next render cycle in case error page is loading.
+        this.async(() => {
+          this.onCloseButtonTap_();
+        });
+      }
+    },
+
     /**
      * Fetches the source for the selected error and populates the code section.
      * @private

chrome/browser/resources/md_extensions/manager.html

@@ -104,7 +104,7 @@
       <cr-lazy-render id="error-page">
         <template>
           <extensions-error-page data="[[errorPageItem_]]" slot="view"
-              delegate="[[delegate]]" slot="view">
+              delegate="[[delegate]]" in-dev-mode="[[inDevMode]]">
           </extensions-error-page>
         </template>
       </cr-lazy-render>