Parse the script_badge manifest section.

chrome/common/extensions/extension.cc

Index: chrome/common/extensions/extension.cc
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 7db7ca6aefc4ca0839a434beeac07cc7cd8badab..efa151ec46f1c3f1ca485f772e5adcefe151da45 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/common/extensions/extension.h"
 
+#include <ostream>
+
 #include "base/base64.h"
 #include "base/basictypes.h"
 #include "base/command_line.h"
@@ -415,6 +417,13 @@ void Extension::AddInstallWarnings(
                            new_warnings.begin(), new_warnings.end());
 }
 
+void Extension::AddDeveloperInstallWarning(
+    InstallWarning::Format format,
+    const std::string& message) {
+  if (location() == LOAD)

[not at google - send to devlin, 2012/06/18 22:59:41]

Install warnings only show up when developer mode has been turned on anyway...
as in, the "developer" is a tautology.

(ny all means have a function for adding a single warning, but only needs to be
called AddInstallWarning)

[Jeffrey Yasskin, 2012/06/18 23:58:42]

My idea here was that it would show up for extensions that are under development
but not extensions that were merely installed. There's another example of this
for the manifest_version_==1 warning, but I don't see any others.

If it shouldn't filter on location()==LOAD, I don't think we need a helper
function for this, so I've removed the filter.

[not at google - send to devlin, 2012/06/19 00:27:32]

Yeah, makes sense. I'm just saying that this happens automatically kind of,
because most people don't have developer mode turned on.

+    install_warnings_.push_back(InstallWarning(format, message));
+}
+
 // static
 bool Extension::IsExtension(const FilePath& file_name) {
   return file_name.MatchesExtension(chrome::kExtensionFileExtension);
@@ -801,7 +810,9 @@ bool Extension::LoadGlobsHelper(
 }
 
 scoped_ptr<ExtensionAction> Extension::LoadExtensionActionHelper(
-    const DictionaryValue* extension_action, string16* error) {
+    const DictionaryValue* extension_action,
+    ExtensionAction::Type action_type,
+    string16* error) {
   scoped_ptr<ExtensionAction> result(new ExtensionAction(id()));
 
   // Page actions are hidden by default, and browser actions ignore
@@ -837,13 +848,18 @@ scoped_ptr<ExtensionAction> Extension::LoadExtensionActionHelper(
   std::string default_icon;
   // Read the page action |default_icon| (optional).
   if (extension_action->HasKey(keys::kPageActionDefaultIcon)) {
-    if (!extension_action->GetString(keys::kPageActionDefaultIcon,
-                                     &default_icon) ||
-        default_icon.empty()) {
-      *error = ASCIIToUTF16(errors::kInvalidPageActionIconPath);
-      return scoped_ptr<ExtensionAction>();
+    if (action_type == ExtensionAction::TYPE_SCRIPT_BADGE) {
+      AddDeveloperInstallWarning(InstallWarning::FORMAT_TEXT,
+                                 errors::kScriptBadgeIconIgnored);
+    } else {
+      if (!extension_action->GetString(keys::kPageActionDefaultIcon,
+                                       &default_icon) ||
+          default_icon.empty()) {
+        *error = ASCIIToUTF16(errors::kInvalidPageActionIconPath);
+        return scoped_ptr<ExtensionAction>();
+      }
+      result->set_default_icon_path(default_icon);
     }
-    result->set_default_icon_path(default_icon);
   }
 
   // Read the page action title from |default_title| if present, |name| if not
@@ -2026,6 +2042,7 @@ bool Extension::LoadExtensionFeatures(
       !LoadContentScripts(error) ||
       !LoadPageAction(error) ||
       !LoadBrowserAction(error) ||
+      !LoadScriptBadge(error) ||
       !LoadFileBrowserHandlers(error) ||
       !LoadChromeURLOverrides(error) ||
       !LoadOmnibox(error) ||
@@ -2246,7 +2263,8 @@ bool Extension::LoadPageAction(string16* error) {
 
   // If page_action_value is not NULL, then there was a valid page action.
   if (page_action_value) {
-    page_action_ = LoadExtensionActionHelper(page_action_value, error);
+    page_action_ = LoadExtensionActionHelper(
+        page_action_value, ExtensionAction::TYPE_PAGE, error);
     if (!page_action_.get())
       return false;  // Failed to parse page action definition.
     declared_action_type_ = ExtensionAction::TYPE_PAGE;
@@ -2271,13 +2289,57 @@ bool Extension::LoadBrowserAction(string16* error) {
     return false;
   }
 
-  browser_action_ = LoadExtensionActionHelper(browser_action_value, error);
+  browser_action_ = LoadExtensionActionHelper(
+      browser_action_value, ExtensionAction::TYPE_BROWSER, error);
   if (!browser_action_.get())
     return false;  // Failed to parse browser action definition.
   declared_action_type_ = ExtensionAction::TYPE_BROWSER;
   return true;
 }
 
+bool Extension::LoadScriptBadge(string16* error) {
+  DictionaryValue* script_badge_value = NULL;
+
+  if (!switch_utils::IsActionBoxEnabled()) {

[not at google - send to devlin, 2012/06/18 22:59:41]

(has been submitted now btw)

[Jeffrey Yasskin, 2012/06/18 23:58:42]

And I've now synced past it.

+    if (manifest_->HasKey(keys::kScriptBadge)) {
+      AddDeveloperInstallWarning(InstallWarning::FORMAT_TEXT,
+                                 errors::kScriptBadgeRequiresActionBox);
+    }
+    return true;

[not at google - send to devlin, 2012/06/18 22:59:41]

Could we just generate a warning here and not early-exit? I know that it would
result in redundantly generating a script badge if the flag is off, but I don't
think that's a big deal (it's just going to be turned on by default soon anyway)
and it means that we have less logic that's divergent on command line flags (and
logic that's divergent on command line flags is confusing and increasingly hard
to follow). It also means no setting up command line flag stuff for unit tests
as I mentioned earlier.

E.g. how about just:

// So as to not confuse developers if they specify a script badge
// section in the manifest, show a warning if the script badge
// declaration isn't going to have any effect.
if (!switch_utils::AreScriptBadgeEnabled() &&
     manifest_HasKey(keys::kScriptBadge)) {
  AddInstallWarning();
}

[Jeffrey Yasskin, 2012/06/18 23:58:42]

Sure. Then script_badge_ should never be null.

+  }
+
+  if (manifest_->HasKey(keys::kScriptBadge)) {
+    if (!manifest_->GetDictionary(keys::kScriptBadge, &script_badge_value)) {
+      *error = ASCIIToUTF16(errors::kInvalidScriptBadge);
+      return false;
+    }
+    script_badge_ = LoadExtensionActionHelper(
+        script_badge_value, ExtensionAction::TYPE_SCRIPT_BADGE, error);
+    if (!script_badge_.get())
+      return false;  // Failed to parse script badge definition.
+    declared_action_type_ = ExtensionAction::TYPE_SCRIPT_BADGE;
+  } else {
+    script_badge_.reset(new ExtensionAction(id()));
+
+    // Make sure there is always a title.
+    script_badge_->SetTitle(ExtensionAction::kDefaultTabId, name());
+  }
+
+  // Script badges always use their extension's icon so users can rely on the
+  // visual appearance to know which extension is running.  This isn't
+  // bulletproof since an malicious extension could use a different 16x16 icon
+  // that matches the icon of a trusted extension, and users wouldn't be warned
+  // during installation.
+
+  script_badge_->set_default_icon_path(
+      icons().Get(ExtensionIconSet::EXTENSION_ICON_BITTY,
+                  ExtensionIconSet::MATCH_BIGGER));
+
+  script_badge_->SetIsVisible(ExtensionAction::kDefaultTabId, true);
+
+  return true;
+}
+
 bool Extension::LoadFileBrowserHandlers(string16* error) {
   if (!manifest_->HasKey(keys::kFileBrowserHandlers))
     return true;
@@ -3617,6 +3679,25 @@ bool Extension::ShouldDisplayInLauncher() const {
   return is_app() && id() != extension_misc::kCloudPrintAppId;
 }
 
+bool Extension::InstallWarning::operator==(const InstallWarning& other) const {
+  return format == other.format && message == other.message;
+}
+
+void PrintTo(const Extension::InstallWarning& warning, ::std::ostream* os){
+  *os << "InstallWarning(";
+  switch (warning.format) {
+    case Extension::InstallWarning::FORMAT_TEXT:
+      *os << "FORMAT_TEXT, \"";
+      break;
+    case Extension::InstallWarning::FORMAT_HTML:
+      *os << "FORMAT_HTML, \"";
+      break;
+  }
+  // This is just for test error messages, so no need to escape '"'
+  // characters inside the message.
+  *os << warning.message << "\")";
+}
+
 ExtensionInfo::ExtensionInfo(const DictionaryValue* manifest,
                              const std::string& id,
                              const FilePath& path,
@@ -3664,46 +3745,6 @@ bool Extension::HasContentScriptAtURL(const GURL& url) const {
   return false;
 }
 
-ExtensionAction* Extension::GetScriptBadge() const {
-  if (!script_badge_.get()) {
-    script_badge_.reset(new ExtensionAction(id()));
-
-    // On initialization, copy the default icon path from the browser action,
-    // or generate a puzzle piece if there isn't one. Extensions may later
-    // overwrite this icon using the browserAction API.
-    if (browser_action() && !browser_action()->default_icon_path().empty()) {
-      script_badge_->set_default_icon_path(
-          browser_action()->default_icon_path());
-    } else {
-      script_badge_->SetIcon(
-          ExtensionAction::kDefaultTabId,
-          *ui::ResourceBundle::GetSharedInstance().GetImageNamed(
-              IDR_EXTENSIONS_FAVICON).ToSkBitmap());
-    }
-
-    // Likewise, make sure there is always a title.
-    script_badge_->SetTitle(ExtensionAction::kDefaultTabId, name());
-  }
-
-  // Every time, re-initialize the script badge based on the current state of
-  // the browser action.
-  int kDefaultTabId = ExtensionAction::kDefaultTabId;
-
-  script_badge_->SetIsVisible(kDefaultTabId, true);
-
-  if (browser_action()) {
-    SkBitmap icon = browser_action()->GetIcon(kDefaultTabId);
-    if (!icon.isNull())
-      script_badge_->SetIcon(kDefaultTabId, icon);
-
-    std::string title = browser_action()->GetTitle(kDefaultTabId);
-    if (!title.empty())
-      script_badge_->SetTitle(kDefaultTabId, title);
-  }
-
-  return script_badge_.get();
-}
-
 const URLPatternSet* Extension::GetTabSpecificHostPermissions(
     int tab_id) const {
   base::AutoLock auto_lock(runtime_data_lock_);