Skip to content

Commit

Permalink
Allow Extension-specific toggling for error reporting
Browse files Browse the repository at this point in the history 
Allow error reporting to be toggled on or off for specific extensions.
Allow type-specific enabling for extension errors (e.g., enabling only manifest or runtime errors).
Create an ErrorMap object for error storage management.

BUG=21734
TBR=finnur@chromium.org (c/b/ui/webui/extensions)

Review URL: https://codereview.chromium.org/58903008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@246756 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rdevlin.cronin@chromium.org committed Jan 24, 2014
1 parent 54ac189 commit 71c10c5
Show file tree
Hide file tree
Showing 12 changed files with 629 additions and 271 deletions.
166 changes: 67 additions & 99 deletions chrome/browser/extensions/error_console/error_console.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,49 +29,21 @@
namespace extensions {

namespace {
// The key into the Extension prefs for an Extension's specific reporting
// settings.
const char kStoreExtensionErrorsPref[] = "store_extension_errors";

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;
}
}
}

// Iterate through an error list and remove and delete all errors of a given
// |type|.
void DeleteErrorsOfTypeFromList(ErrorConsole::ErrorList* list,
ExtensionError::Type type) {
ErrorConsole::ErrorList::iterator iter = list->begin();
while (iter != list->end()) {
if ((*iter)->type() == type) {
delete *iter;
iter = list->erase(iter);
} else {
++iter;
}
}
// The default mask (for the time being) is to report everything.
const int32 kDefaultMask = (1 << ExtensionError::MANIFEST_ERROR) |
(1 << ExtensionError::RUNTIME_ERROR);
}

base::LazyInstance<ErrorConsole::ErrorList> g_empty_error_list =
LAZY_INSTANCE_INITIALIZER;

} // namespace

void ErrorConsole::Observer::OnErrorConsoleDestroyed() {
}

ErrorConsole::ErrorConsole(Profile* profile,
ExtensionService* extension_service)
: enabled_(false), profile_(profile) {
: enabled_(false), default_mask_(kDefaultMask), profile_(profile) {
// TODO(rdevlin.cronin): Remove once crbug.com/159265 is fixed.
#if !defined(ENABLE_EXTENSIONS)
return;
Expand All @@ -94,56 +66,71 @@ ErrorConsole::ErrorConsole(Profile* profile,

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<ExtensionError> error) {
void ErrorConsole::SetReportingForExtension(const std::string& extension_id,
ExtensionError::Type type,
bool enabled) {
DCHECK(thread_checker_.CalledOnValidThread());

if (!enabled_ || !Extension::IdIsValid(error->extension_id()))
if (!enabled_ || !Extension::IdIsValid(extension_id))
return;

ErrorList* extension_errors = &errors_[error->extension_id()];

// First, check if it's a duplicate.
for (ErrorList::iterator iter = extension_errors->begin();
iter != extension_errors->end(); ++iter) {
// If we find a duplicate error, remove the old error and add the new one,
// incrementing the occurrence count of the error. We use the new error
// for runtime errors, so we can link to the latest context, inspectable
// view, etc.
if (error->IsEqual(*iter)) {
error->set_occurrences((*iter)->occurrences() + 1);
delete *iter;
extension_errors->erase(iter);
break;
}
}
ErrorPreferenceMap::iterator pref = pref_map_.find(extension_id);

// 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();
if (pref == pref_map_.end()) {
pref = pref_map_.insert(
std::pair<std::string, int32>(extension_id, default_mask_)).first;
}

extension_errors->push_back(error.release());
pref->second =
enabled ? pref->second | (1 << type) : pref->second &~(1 << type);

ExtensionPrefs::Get(profile_)->UpdateExtensionPref(
extension_id,
kStoreExtensionErrorsPref,
base::Value::CreateIntegerValue(pref->second));
}

FOR_EACH_OBSERVER(
Observer, observers_, OnErrorAdded(extension_errors->back()));
void ErrorConsole::UseDefaultReportingForExtension(
const std::string& extension_id) {
DCHECK(thread_checker_.CalledOnValidThread());
if (!enabled_ || !Extension::IdIsValid(extension_id))
return;

pref_map_.erase(extension_id);
ExtensionPrefs::Get(profile_)->UpdateExtensionPref(
extension_id,
kStoreExtensionErrorsPref,
NULL);
}

const ErrorConsole::ErrorList& ErrorConsole::GetErrorsForExtension(
void ErrorConsole::ReportError(scoped_ptr<ExtensionError> error) {
DCHECK(thread_checker_.CalledOnValidThread());
if (!enabled_ || !Extension::IdIsValid(error->extension_id()))
return;

ErrorPreferenceMap::const_iterator pref =
pref_map_.find(error->extension_id());
// Check the mask to see if we report the error. If we don't have a specific
// entry, use the default mask.
if ((pref == pref_map_.end() &&
((default_mask_ & (1 << error->type())) == 0)) ||
(pref != pref_map_.end() && (pref->second & (1 << error->type())) == 0)) {
return;
}

const ExtensionError* weak_error = errors_.AddError(error.Pass());
FOR_EACH_OBSERVER(Observer, observers_, OnErrorAdded(weak_error));
}

const 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();
return errors_.GetErrorsForExtension(extension_id);
}

void ErrorConsole::AddObserver(Observer* observer) {
Expand Down Expand Up @@ -183,18 +170,24 @@ void ErrorConsole::Enable(ExtensionService* extension_service) {
content::Source<Profile>(profile_));

if (extension_service) {
// Get manifest errors for extensions already installed.
const ExtensionSet* extensions = extension_service->extensions();
ExtensionPrefs* prefs = ExtensionPrefs::Get(profile_);
for (ExtensionSet::const_iterator iter = extensions->begin();
iter != extensions->end(); ++iter) {
int mask = 0;
if (prefs->ReadPrefAsInteger(iter->get()->id(),
kStoreExtensionErrorsPref,
&mask)) {
pref_map_[iter->get()->id()] = mask;
}
AddManifestErrorsForExtension(iter->get());
}
}
}

void ErrorConsole::Disable() {
notification_registrar_.RemoveAll();
RemoveAllErrors();
errors_.RemoveAllErrors();
enabled_ = false;
}

Expand All @@ -211,27 +204,6 @@ void ErrorConsole::AddManifestErrorsForExtension(const Extension* extension) {
}
}

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()) {
STLDeleteContainerPointers(iter->second.begin(), iter->second.end());
errors_.erase(iter);
}
}

void ErrorConsole::RemoveAllErrors() {
for (ErrorMap::iterator iter = errors_.begin(); iter != errors_.end(); ++iter)
STLDeleteContainerPointers(iter->second.begin(), iter->second.end());
errors_.clear();
}

void ErrorConsole::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
Expand All @@ -241,14 +213,13 @@ void ErrorConsole::Observe(int type,
// If incognito profile which we are associated with is destroyed, also
// destroy all incognito errors.
if (profile->IsOffTheRecord() && profile_->IsSameProfile(profile))
RemoveIncognitoErrors();
errors_.RemoveIncognitoErrors();
break;
}
case chrome::NOTIFICATION_EXTENSION_UNINSTALLED:
// No need to check the profile here, since we registered to only receive
// notifications from our own.
RemoveErrorsForExtension(
content::Details<Extension>(details).ptr()->id());
errors_.Remove(content::Details<Extension>(details).ptr()->id());
break;
case chrome::NOTIFICATION_EXTENSION_INSTALLED: {
const InstalledExtensionInfo* info =
Expand All @@ -258,11 +229,8 @@ void ErrorConsole::Observe(int type,
// to keep runtime errors, though, because extensions are reloaded on a
// refresh of chrome:extensions, and we don't want to wipe our history
// whenever that happens.
ErrorMap::iterator iter = errors_.find(info->extension->id());
if (iter != errors_.end()) {
DeleteErrorsOfTypeFromList(&(iter->second),
ExtensionError::MANIFEST_ERROR);
}
errors_.RemoveErrorsForExtensionOfType(info->extension->id(),
ExtensionError::MANIFEST_ERROR);

AddManifestErrorsForExtension(info->extension);
break;
Expand Down
45 changes: 30 additions & 15 deletions chrome/browser/extensions/error_console/error_console.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/threading/thread_checker.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "extensions/browser/error_map.h"
#include "extensions/browser/extension_error.h"

namespace content {
Expand All @@ -38,9 +39,6 @@ class Extension;
// BrowserContext-keyed service.
class ErrorConsole : public content::NotificationObserver {
public:
typedef std::deque<const ExtensionError*> ErrorList;
typedef std::map<std::string, ErrorList> ErrorMap;

class Observer {
public:
// Sent when a new error is reported to the error console.
Expand All @@ -57,6 +55,16 @@ class ErrorConsole : public content::NotificationObserver {
// Convenience method to return the ErrorConsole for a given profile.
static ErrorConsole* Get(Profile* profile);

// Set whether or not errors of the specified |type| are stored for the
// extension with the given |extension_id|. This will be stored in the
// preferences.
void SetReportingForExtension(const std::string& extension_id,
ExtensionError::Type type,
bool enabled);

// Restore default reporting to the given extension.
void UseDefaultReportingForExtension(const std::string& extension_id);

// Report an extension error, and add it to the list.
void ReportError(scoped_ptr<ExtensionError> error);

Expand All @@ -70,14 +78,25 @@ class ErrorConsole : public content::NotificationObserver {
void RemoveObserver(Observer* observer);

bool enabled() const { return enabled_; }
const ErrorMap& errors() const { return errors_; }

// Return the number of entries (extensions) in the error map.
size_t get_num_entries_for_test() const { return errors_.size(); }

// Set the default reporting for all extensions.
void set_default_reporting_for_test(ExtensionError::Type type, bool enabled) {
default_mask_ =
enabled ? default_mask_ | (1 << type) : default_mask_ & ~(1 << type);
}

private:
FRIEND_TEST_ALL_PREFIXES(ErrorConsoleUnitTest, AddAndRemoveErrors);
// A map which stores the reporting preferences for each Extension. If there
// is no entry in the map, it signals that the |default_mask_| should be used.
typedef std::map<std::string, int32> ErrorPreferenceMap;

// Enable the error console for error collection and retention. This involves
// subscribing to the appropriate notifications and fetching manifest errors.
void Enable(ExtensionService* extension_service);

// Disable the error console, removing the subscriptions to notifications and
// removing all current errors.
void Disable();
Expand All @@ -90,16 +109,6 @@ class ErrorConsole : public content::NotificationObserver {
// Add manifest errors from an extension's install warnings.
void AddManifestErrorsForExtension(const Extension* extension);

// Remove all errors which happened while incognito; we have to do this once
// the incognito profile is destroyed.
void RemoveIncognitoErrors();

// Remove all errors relating to a particular |extension_id|.
void RemoveErrorsForExtension(const std::string& extension_id);

// Remove all errors for all extensions.
void RemoveAllErrors();

// content::NotificationObserver implementation.
virtual void Observe(int type,
const content::NotificationSource& source,
Expand All @@ -119,6 +128,12 @@ class ErrorConsole : public content::NotificationObserver {
// The errors which we have received so far.
ErrorMap errors_;

// The mapping of Extension's error-reporting preferences.
ErrorPreferenceMap pref_map_;

// The default mask to use if an Extension does not have specific settings.
int32 default_mask_;

// The profile with which the ErrorConsole is associated. Only collect errors
// from extensions and RenderViews associated with this Profile (and it's
// incognito fellow).
Expand Down
13 changes: 7 additions & 6 deletions chrome/browser/extensions/error_console/error_console_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ class ErrorConsoleBrowserTest : public ExtensionBrowserTest {

// We should only have errors for a single extension, or should have no
// entries, if no errors were expected.
ASSERT_EQ(errors_expected > 0 ? 1u : 0u, error_console()->errors().size());
ASSERT_EQ(errors_expected > 0 ? 1u : 0u,
error_console()->get_num_entries_for_test());
ASSERT_EQ(
errors_expected,
error_console()->GetErrorsForExtension((*extension)->id()).size());
Expand All @@ -294,7 +295,7 @@ IN_PROC_BROWSER_TEST_F(ErrorConsoleBrowserTest, ReportManifestErrors) {
ACTION_NONE,
&extension);

const ErrorConsole::ErrorList& errors =
const ErrorList& errors =
error_console()->GetErrorsForExtension(extension->id());

// Unfortunately, there's not always a hard guarantee of order in parsing the
Expand Down Expand Up @@ -373,7 +374,7 @@ IN_PROC_BROWSER_TEST_F(ErrorConsoleBrowserTest,

std::string script_url = extension->url().Resolve("content_script.js").spec();

const ErrorConsole::ErrorList& errors =
const ErrorList& errors =
error_console()->GetErrorsForExtension(extension->id());

// The first error should be a console log.
Expand Down Expand Up @@ -431,7 +432,7 @@ IN_PROC_BROWSER_TEST_F(ErrorConsoleBrowserTest, BrowserActionRuntimeError) {

std::string script_url = extension->url().Resolve("browser_action.js").spec();

const ErrorConsole::ErrorList& errors =
const ErrorList& errors =
error_console()->GetErrorsForExtension(extension->id());

std::string event_bindings_str =
Expand Down Expand Up @@ -471,7 +472,7 @@ IN_PROC_BROWSER_TEST_F(ErrorConsoleBrowserTest, BadAPIArgumentsRuntimeError) {
ACTION_NONE,
&extension);

const ErrorConsole::ErrorList& errors =
const ErrorList& errors =
error_console()->GetErrorsForExtension(extension->id());

std::string schema_utils_str =
Expand Down Expand Up @@ -510,7 +511,7 @@ IN_PROC_BROWSER_TEST_F(ErrorConsoleBrowserTest, BadAPIPermissionsRuntimeError) {

std::string script_url = extension->url().Resolve("background.js").spec();

const ErrorConsole::ErrorList& errors =
const ErrorList& errors =
error_console()->GetErrorsForExtension(extension->id());

CheckRuntimeError(
Expand Down
Loading

0 comments on commit 71c10c5

Please sign in to comment.