Warn user in case extension delays network traffic too much.

chrome/browser/ui/webui/options/extension_settings_handler.cc

Index: chrome/browser/ui/webui/options/extension_settings_handler.cc
diff --git a/chrome/browser/ui/webui/options/extension_settings_handler.cc b/chrome/browser/ui/webui/options/extension_settings_handler.cc
index 9943a750c34b902e90e2476a8faca5c05732a802..dc2e55f21bcbb4afd0d90a0b15463621e4266953 100644
--- a/chrome/browser/ui/webui/options/extension_settings_handler.cc
+++ b/chrome/browser/ui/webui/options/extension_settings_handler.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/extensions/extension_disabled_infobar_delegate.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_updater.h"
+#include "chrome/browser/extensions/extension_warning_set.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/background_contents.h"
@@ -142,6 +143,8 @@ void ExtensionSettingsHandler::HandleRequestExtensionsData(
   // Add the extensions to the results structure.
   ListValue *extensions_list = new ListValue();
 
+  ExtensionWarningSet* warnings = extension_service_->extension_warnings();
+
   const ExtensionList* extensions = extension_service_->extensions();
   for (ExtensionList::const_iterator extension = extensions->begin();
        extension != extensions->end(); ++extension) {
@@ -150,6 +153,7 @@ void ExtensionSettingsHandler::HandleRequestExtensionsData(
           extension_service_,
           *extension,
           GetActivePagesForExtension(*extension),
+          warnings,
           true, false));  // enabled, terminated
     }
   }
@@ -161,6 +165,7 @@ void ExtensionSettingsHandler::HandleRequestExtensionsData(
           extension_service_,
           *extension,
           GetActivePagesForExtension(*extension),
+          warnings,
           false, false));  // enabled, terminated
     }
   }
@@ -173,6 +178,7 @@ void ExtensionSettingsHandler::HandleRequestExtensionsData(
           extension_service_,
           *extension,
           empty_pages,  // Terminated process has no active pages.
+          warnings,
           false, true));  // enabled, terminated
     }
   }
@@ -204,6 +210,8 @@ void ExtensionSettingsHandler::MaybeRegisterForNotifications() {
       NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UPDATE_DISABLED,
       NotificationService::AllSources());
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_WARNING_CHANGED,
+      NotificationService::AllSources());

[Finnur, 2011/10/10 09:59:24]

Shouldn't this just check for notifications from its own profile?

[Finnur, 2011/10/10 10:37:27]

See http://codereview.chromium.org/8199022/ for reference.

On 2011/10/10 09:59:24, Finnur wrote:

[battre, 2011/10/10 13:16:36]

Done.

   registrar_.Add(this,
       content::NOTIFICATION_NAV_ENTRY_COMMITTED,
       NotificationService::AllSources());
@@ -554,6 +562,8 @@ void ExtensionSettingsHandler::GetLocalizedValues(
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_CRASHED_EXTENSION));
   localized_strings->SetString("extensionSettingsInDevelopment",
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_IN_DEVELOPMENT));
+  localized_strings->SetString("extensionSettingsWarningsTitle",
+      l10n_util::GetStringUTF16(IDS_EXTENSION_WARNINGS_TITLE));
 }
 
 void ExtensionSettingsHandler::Initialize() {
@@ -600,6 +610,7 @@ void ExtensionSettingsHandler::Observe(int type,
     case chrome::NOTIFICATION_EXTENSION_PROCESS_CREATED:
     case chrome::NOTIFICATION_EXTENSION_UNLOADED:
     case chrome::NOTIFICATION_EXTENSION_UPDATE_DISABLED:
+    case chrome::NOTIFICATION_EXTENSION_WARNING_CHANGED:
     case content::NOTIFICATION_RENDER_VIEW_HOST_CREATED:
     case content::NOTIFICATION_NAV_ENTRY_COMMITTED:
     case chrome::NOTIFICATION_BACKGROUND_CONTENTS_NAVIGATED:
@@ -625,9 +636,27 @@ void ExtensionSettingsHandler::MaybeUpdateAfterNotification() {
 }
 
 // Static
+string16 ExtensionSettingsHandler::GetLocalizedWarning(
+    ExtensionWarning::WarningType warning_type) {
+  switch (warning_type) {
+  case ExtensionWarning::kInvalid:

[Finnur, 2011/10/10 09:59:24]

nit: indentation is off.

[battre, 2011/10/10 13:16:36]

Done.

+  case ExtensionWarning::kMaxWarningType:
+    NOTREACHED();
+    return string16();
+  case ExtensionWarning::kNetworkDelay:
+    return l10n_util::GetStringFUTF16(
+        IDS_EXTENSION_WARNINGS_NETWORK_DELAY,
+        l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));

[battre, 2011/10/10 13:16:36]

please note that there is no default case here. Adding new enumeration values to
the type without a new case branch here creates a compiler error.

[Finnur, 2011/10/10 15:06:56]

Hmm... This is a little too subtle for my taste, but it comes with the added
benefit of a compile-time error. Can't argue with that.

Can you at least add a NOTREACHED() on line 651 (before the return), for the
benefit of Windows developers?

On 2011/10/10 13:16:36, battre wrote:

[battre, 2011/10/10 15:55:57]

Done.

+  }
+  return string16();
+}
+
+// Static
 DictionaryValue* ExtensionSettingsHandler::CreateExtensionDetailValue(
     ExtensionService* service, const Extension* extension,
-    const std::vector<ExtensionPage>& pages, bool enabled, bool terminated) {
+    const std::vector<ExtensionPage>& pages,
+    const ExtensionWarningSet* warnings_set,
+    bool enabled, bool terminated) {
   DictionaryValue* extension_data = new DictionaryValue();
   GURL icon =
       ExtensionIconSource::GetIconURL(extension,
@@ -691,6 +720,22 @@ DictionaryValue* ExtensionSettingsHandler::CreateExtensionDetailValue(
       extension->browser_action() || extension->page_action());
   extension_data->SetString("homepageUrl", extension->GetHomepageURL().spec());
 
+  // Add warnings.
+  ListValue* warnings_list = new ListValue;
+  if (warnings_set) {
+    std::set<ExtensionWarning::WarningType> warnings;
+    warnings_set->GetWarningsAffectingExtension(extension->id(), &warnings);
+
+    for (std::set<ExtensionWarning::WarningType>::const_iterator iter =
+             warnings.begin();
+         iter != warnings.end();
+         ++iter) {
+      warnings_list->Append(
+          Value::CreateStringValue(GetLocalizedWarning(*iter)));

[Finnur, 2011/10/10 09:59:24]

Hmm... For all warnings we add in the future (besides 'network slowdown'), we'll
pass an empty string down to the page...

[battre, 2011/10/10 13:16:36]

No, we'll trigger a compile error (on GCC and Clang, not for MSVC) that forces
authors of new warnings to add new messages.

+    }
+  }
+  extension_data->Set("warnings", warnings_list);
+
   return extension_data;
 }