Enable Error Console for ADT and <= Dev Channel

chrome/browser/extensions/error_console/error_console.cc

Index: chrome/browser/extensions/error_console/error_console.cc
diff --git a/chrome/browser/extensions/error_console/error_console.cc b/chrome/browser/extensions/error_console/error_console.cc
index 9fc50894cca3c21d190f7e69ac3749fe500e5408..091fbd3a1a87f83492c4b665e663c6d76372b611 100644
--- a/chrome/browser/extensions/error_console/error_console.cc
+++ b/chrome/browser/extensions/error_console/error_console.cc
@@ -16,10 +16,12 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
@@ -29,6 +31,7 @@
 namespace extensions {
 
 namespace {
+
 // The key into the Extension prefs for an Extension's specific reporting
 // settings.
 const char kStoreExtensionErrorsPref[] = "store_extension_errors";
@@ -36,31 +39,29 @@ const char kStoreExtensionErrorsPref[] = "store_extension_errors";
 // The default mask (for the time being) is to report everything.
 const int32 kDefaultMask = (1 << ExtensionError::MANIFEST_ERROR) |
                            (1 << ExtensionError::RUNTIME_ERROR);
-}
+
+const char kAppsDeveloperToolsExtensionId[] =
+    "ohmmkhmmmpcnpikjeljgnaoabkaalbgc";
+
+}  // namespace
 
 void ErrorConsole::Observer::OnErrorConsoleDestroyed() {
 }
 
 ErrorConsole::ErrorConsole(Profile* profile,
                            ExtensionService* extension_service)
-     : enabled_(false), default_mask_(kDefaultMask), profile_(profile) {
+     : should_record_(false), default_mask_(kDefaultMask), profile_(profile) {

[not at google - send to devlin, 2014/03/28 22:05:32]

I thnk that enabled_ was better, sorry.

[Devlin, 2014/03/31 18:39:31]

Haha okay, done.

 // TODO(rdevlin.cronin): Remove once crbug.com/159265 is fixed.
 #if !defined(ENABLE_EXTENSIONS)
   return;
 #endif
 
-  // If we don't have the necessary FeatureSwitch enabled, then return
-  // immediately. Since we never register for any notifications, this ensures
-  // the ErrorConsole will never be enabled.
-  if (!FeatureSwitch::error_console()->IsEnabled())
-    return;
-
   pref_registrar_.Init(profile_->GetPrefs());
   pref_registrar_.Add(prefs::kExtensionsUIDeveloperMode,
                       base::Bind(&ErrorConsole::OnPrefChanged,
                                  base::Unretained(this)));
 
-  if (profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode))
+  if (IsEnabledForChromeExtensionsPage() || IsEnabledForAppsDeveloperTools())
     Enable(extension_service);
 }
 
@@ -77,7 +78,7 @@ void ErrorConsole::SetReportingForExtension(const std::string& extension_id,
                                             ExtensionError::Type type,
                                             bool enabled) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!enabled_ || !Extension::IdIsValid(extension_id))
+  if (!should_record_ || !Extension::IdIsValid(extension_id))
     return;
 
   ErrorPreferenceMap::iterator pref = pref_map_.find(extension_id);
@@ -99,7 +100,7 @@ void ErrorConsole::SetReportingForExtension(const std::string& extension_id,
 void ErrorConsole::UseDefaultReportingForExtension(
     const std::string& extension_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!enabled_ || !Extension::IdIsValid(extension_id))
+  if (!should_record_ || !Extension::IdIsValid(extension_id))
     return;
 
   pref_map_.erase(extension_id);
@@ -111,7 +112,7 @@ void ErrorConsole::UseDefaultReportingForExtension(
 
 void ErrorConsole::ReportError(scoped_ptr<ExtensionError> error) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!enabled_ || !Extension::IdIsValid(error->extension_id()))
+  if (!should_record_ || !Extension::IdIsValid(error->extension_id()))
     return;
 
   ErrorPreferenceMap::const_iterator pref =
@@ -144,17 +145,29 @@ void ErrorConsole::RemoveObserver(Observer* observer) {
 }
 
 void ErrorConsole::OnPrefChanged() {
-  bool developer_mode =
-      profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode);
+  bool should_be_enabled = IsEnabledForChromeExtensionsPage() ||
+                           IsEnabledForAppsDeveloperTools();
 
-  if (developer_mode && !enabled_)
+  if (should_be_enabled && !should_record_)
     Enable(ExtensionSystem::Get(profile_)->extension_service());
-  else if (!developer_mode && enabled_)
+  else if (!should_be_enabled && should_record_)
     Disable();
 }
 
+bool ErrorConsole::IsEnabledForChromeExtensionsPage() const {
+  return profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode) &&
+         (FeatureSwitch::error_console()->IsEnabled() ||
+          chrome::VersionInfo::GetChannel() <=
+              chrome::VersionInfo::CHANNEL_DEV);
+}
+
+bool ErrorConsole::IsEnabledForAppsDeveloperTools() const {
+  return ExtensionRegistry::Get(profile_)->GetExtensionById(
+      kAppsDeveloperToolsExtensionId, ExtensionRegistry::EVERYTHING) != NULL;

[not at google - send to devlin, 2014/03/28 22:05:32]

Yes I think we had better only do this while the ADT is enabled, and listen for
ExtensionRegistry events. The Registry should outlive this class if a dependency
is declared on ExtensionSystem, but I suggest making the Registry an explicit
dependency on this class from the constructor.

Apart from that it shouldn't be too bad. All that OnLoaded/OnUnloaded need to do
is call OnPrefChanged.

[Devlin, 2014/03/31 18:39:31]

ErrorConsole can't directly have dependencies - it's not a KeyedService. But as
long as ExtensionSystem stays dependent on ExtensionRegistry, it'll be fine. 
Implemented ADT enabled checks. 

+}
+
 void ErrorConsole::Enable(ExtensionService* extension_service) {
-  enabled_ = true;
+  should_record_ = true;
 
   notification_registrar_.Add(
       this,
@@ -188,7 +201,7 @@ void ErrorConsole::Enable(ExtensionService* extension_service) {
 void ErrorConsole::Disable() {
   notification_registrar_.RemoveAll();
   errors_.RemoveAllErrors();
-  enabled_ = false;
+  should_record_ = false;
 }
 
 void ErrorConsole::AddManifestErrorsForExtension(const Extension* extension) {