ErrorConsole Switch, Duplicate Error Detection, Dev Mode Check

chrome/browser/extensions/error_console/error_console.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/error_console/error_console.h"
 
 #include <list>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/command_line.h"
 #include "base/lazy_instance.h"
+#include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/extension.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/common/constants.h"
+#include "extensions/common/switches.h"
 
 namespace extensions {
 
 namespace {
 
 const size_t kMaxErrorsPerExtension = 100;
 
 // Iterate through an error list and remove and delete all errors which were
 // from an incognito context.
 void DeleteIncognitoErrorsFromList(ErrorConsole::ErrorList* list) {
   ErrorConsole::ErrorList::iterator iter = list->begin();
   while (iter != list->end()) {
     if ((*iter)->from_incognito()) {
       delete *iter;
       iter = list->erase(iter);
     } else {
       ++iter;
     }
   }
 }
 
 base::LazyInstance<ErrorConsole::ErrorList> g_empty_error_list =
     LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 void ErrorConsole::Observer::OnErrorConsoleDestroyed() {
 }
 
-ErrorConsole::ErrorConsole(Profile* profile) : profile_(profile) {
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_PROFILE_DESTROYED,
-                 content::NotificationService::AllBrowserContextsAndSources());
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
-                 content::Source<Profile>(profile_));
+ErrorConsole::ErrorConsole(Profile* profile) : enabled_(false),
+                                               profile_(profile) {
+  // If we don't have the necessary CommandLine switch enabled, then return

[Yoyo Zhou, 2013/08/27 00:41:07]

As I mentioned earlier today, take a look at FeatureSwitch:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/exte...

One of the benefits that didn't come to my mind at the time is it greatly
simplifies what you have to do when the switch goes from default-off to
default-on.

[Devlin, 2013/08/27 17:13:56]

Done.

+  // immediately. Since we never register for any notifications, this ensures
+  // the ErrorConsole will never be enabled.
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableErrorConsole)) {
+    return;
+  }
+
+  pref_registrar_.Init(profile_->GetPrefs());
+  pref_registrar_.Add(prefs::kExtensionsUIDeveloperMode,
+                      base::Bind(&ErrorConsole::OnPrefChanged,
+                                 base::Unretained(this)));
+
+  if (profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode))
+    Enable();
 }
 
 ErrorConsole::~ErrorConsole() {
   FOR_EACH_OBSERVER(Observer, observers_, OnErrorConsoleDestroyed());
   RemoveAllErrors();
 }
 
 // static
 ErrorConsole* ErrorConsole::Get(Profile* profile) {
   return ExtensionSystem::Get(profile)->error_console();
 }
 
-void ErrorConsole::ReportError(scoped_ptr<const ExtensionError> scoped_error) {
+void ErrorConsole::ReportError(scoped_ptr<const ExtensionError> error) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  const ExtensionError* error = scoped_error.release();
-  // If there are too many errors for an extension already, limit ourselves to
-  // the most recent ones.
-  ErrorList* error_list = &errors_[error->extension_id()];
-  if (error_list->size() >= kMaxErrorsPerExtension) {
-    delete error_list->front();
-    error_list->pop_front();
+  if (!enabled_ || !Extension::IdIsValid(error->extension_id()))
+    return;
+
+  // Keep a pointer to the error so we can reference it after |error|'s release
+  // without keeping track of an index in a list.
+  const ExtensionError* weak_error = error.get();
+  ErrorList* extension_errors = &errors_[error->extension_id()];
+
+  // First, check if it's a duplicate.
+  for (size_t i = 0; i < extension_errors->size(); ++i) {
+    // If we find a duplicate error, replace the error with the new version.

[Yoyo Zhou, 2013/08/27 00:41:07]

There's no way to see how often a duplicate error has occurred, right? It might
be nice to show a count, although I'm not sure what the UI should look like.

[Devlin, 2013/08/27 17:13:56]

Originally, I was holding off on any of the implementation until I had an idea
what the UI would be, but it's a simple enough addition to add in. Occurrence
count added.

+    // This can be useful for runtime errors, where we can link to the latest
+    // context, inspectable view, etc.
+    if (error->IsEqual(extension_errors->at(i))) {
+      delete extension_errors->at(i);
+      extension_errors->at(i) = error.release();

[Yoyo Zhou, 2013/08/27 00:41:07]

Could this violate our expectation that the error list contains the most recent
errors? e.g. say we're just showing the 3 most recent errors. We might not be
showing an error that repeated an older error for some reason. It seems to me
that adding the new error to the end would be preferable.

[Devlin, 2013/08/27 17:13:56]

Ooh, good point. Fixed.

+      break;
+    }
   }
-  error_list->push_back(error);
 
-  FOR_EACH_OBSERVER(Observer, observers_, OnErrorAdded(error));
+  // If we didn't replace a previous error, then we add the error to the end of
+  // the list.
+  if (error.get()) {
+    // If there are too many errors for an extension already, limit ourselves to
+    // the most recent ones.
+    if (extension_errors->size() >= kMaxErrorsPerExtension) {
+      delete extension_errors->front();
+      extension_errors->pop_front();
+    }
+    extension_errors->push_back(error.release());
+  }
+
+  FOR_EACH_OBSERVER(Observer, observers_, OnErrorAdded(weak_error));
 }
 
 const ErrorConsole::ErrorList& ErrorConsole::GetErrorsForExtension(
     const std::string& extension_id) const {
   ErrorMap::const_iterator iter = errors_.find(extension_id);
   if (iter != errors_.end())
     return iter->second;
   return g_empty_error_list.Get();
 }
 
 void ErrorConsole::AddObserver(Observer* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   observers_.AddObserver(observer);
 }
 
 void ErrorConsole::RemoveObserver(Observer* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   observers_.RemoveObserver(observer);
 }
 
+void ErrorConsole::OnPrefChanged() {
+  bool developer_mode =
+      profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode);
+
+  if (developer_mode && !enabled_)
+    Enable();
+  else if (!developer_mode && enabled_)
+    Disable();
+}
+
+void ErrorConsole::Enable() {
+  enabled_ = true;
+
+  notification_registrar_.Add(
+      this,
+      chrome::NOTIFICATION_PROFILE_DESTROYED,
+      content::NotificationService::AllBrowserContextsAndSources());
+  notification_registrar_.Add(
+      this,
+      chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
+      content::Source<Profile>(profile_));
+}
+
+void ErrorConsole::Disable() {
+  notification_registrar_.RemoveAll();
+  RemoveAllErrors();
+  enabled_ = false;
+}
+
 void ErrorConsole::RemoveIncognitoErrors() {
   for (ErrorMap::iterator iter = errors_.begin();
        iter != errors_.end(); ++iter) {
     DeleteIncognitoErrorsFromList(&(iter->second));
   }
 }
 
 void ErrorConsole::RemoveErrorsForExtension(const std::string& extension_id) {
   ErrorMap::iterator iter = errors_.find(extension_id);
   if (iter != errors_.end()) {
@@ skipping 25 matching lines @@
       // notifications from our own.
       RemoveErrorsForExtension(
           content::Details<Extension>(details).ptr()->id());
       break;
     default:
       NOTREACHED();
   }
 }
 
 }  // namespace extensions