Move page actions over to ExtensionAction2 and get rid of

extension_action.*.

Final bit of refactor will be to rename ExtensionAction2 to
ExtensionAction will be the next CL.

BUG=24472,25844

Review URL: http://codereview.chromium.org/332021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30133 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 238 insertions, 241 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 202a2dc..2fae2ab 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -297,99 +297,15 @@ bool Extension::LoadUserScriptHelper(const DictionaryValue* content_script,
   return true;
 }
 
-// Helper method that loads a PageAction or BrowserAction object from a
-// dictionary in the page_actions list or browser_action key of the manifest.
-ExtensionAction* Extension::LoadExtensionActionHelper(
-    const DictionaryValue* page_action, std::string* error,
-    ExtensionAction::ExtensionActionType action_type) {
-  scoped_ptr<ExtensionAction> result(new ExtensionAction());
-  result->set_extension_id(id());
-  result->set_type(action_type);
-
-  // TODO(EXTENSIONS_DEPRECATED): icons list is obsolete.
-  ListValue* icons = NULL;
-  if (page_action->HasKey(keys::kPageActionIcons) &&
-      page_action->GetList(keys::kPageActionIcons, &icons)) {
-    for (ListValue::const_iterator iter = icons->begin();
-         iter != icons->end(); ++iter) {
-      std::string path;
-      if (!(*iter)->GetAsString(&path) || path.empty()) {
-        *error = errors::kInvalidPageActionIconPath;
-        return NULL;
-      }
-
-      result->AddIconPath(path);
-    }
-  }
-
-  // TODO(EXTENSIONS_DEPRECATED): Read the page action |id| (optional).
-  std::string id;
-  if (action_type == ExtensionAction::PAGE_ACTION)
-    page_action->GetString(keys::kPageActionId, &id);
-  result->set_id(id);
-
-  std::string default_icon;
-  // Read the page action |default_icon| (optional).
-  if (page_action->HasKey(keys::kPageActionDefaultIcon)) {
-    if (!page_action->GetString(keys::kPageActionDefaultIcon, &default_icon) ||
-        default_icon.empty()) {
-      *error = errors::kInvalidPageActionIconPath;
-      return NULL;
-    }
-    // TODO(EXTENSIONS_DEPRECATED): one icon.
-    result->AddIconPath(default_icon);
-  }
-
-  // Read the page action |default_title|.
-  std::string title;
-  if (!page_action->GetString(keys::kName, &title) &&
-      !page_action->GetString(keys::kPageActionDefaultTitle, &title)) {
-    *error = errors::kInvalidPageActionDefaultTitle;
-    return NULL;
-  }
-  result->set_title(title);
-
-  // Read the action's |popup| (optional).
-  DictionaryValue* popup = NULL;
-  std::string url_str;
-  if (page_action->HasKey(keys::kPageActionPopup) &&
-     !page_action->GetDictionary(keys::kPageActionPopup, &popup) &&
-     !page_action->GetString(keys::kPageActionPopup, &url_str)) {
-    *error = errors::kInvalidPageActionPopup;
-    return NULL;
-  }
-  if (popup) {
-    // TODO(EXTENSIONS_DEPRECATED): popup is a string only
-    if (!popup->GetString(keys::kPageActionPopupPath, &url_str)) {
-      *error = ExtensionErrorUtils::FormatErrorMessage(
-          errors::kInvalidPageActionPopupPath, "<missing>");
-      return NULL;
-    }
-    GURL url = GetResourceURL(url_str);
-    if (!url.is_valid()) {
-      *error = ExtensionErrorUtils::FormatErrorMessage(
-          errors::kInvalidPageActionPopupPath, url_str);
-      return NULL;
-    }
-    result->set_popup_url(url);
-  } else if (!url_str.empty()) {
-    GURL url = GetResourceURL(url_str);
-    if (!url.is_valid()) {
-      *error = ExtensionErrorUtils::FormatErrorMessage(
-          errors::kInvalidPageActionPopupPath, url_str);
-      return NULL;
-    }
-    result->set_popup_url(url);
-  }
-
-  return result.release();
-}
-
 ExtensionAction2* Extension::LoadExtensionAction2Helper(
     const DictionaryValue* extension_action, std::string* error) {
   scoped_ptr<ExtensionAction2> result(new ExtensionAction2());
   result->set_extension_id(id());
 
+  // Page actions are hidden by default, and browser actions ignore
+  // visibility.
+  result->SetIsVisible(ExtensionAction2::kDefaultTabId, false);
+
   // TODO(EXTENSIONS_DEPRECATED): icons list is obsolete.
   ListValue* icons = NULL;
   if (extension_action->HasKey(keys::kPageActionIcons) &&
@@ -403,7 +319,6 @@ ExtensionAction2* Extension::LoadExtensionAction2Helper(
       }
 
       result->icon_paths()->push_back(path);
-      result->SetDefaultIcon(path);
     }
   }
 
@@ -426,7 +341,7 @@ ExtensionAction2* Extension::LoadExtensionAction2Helper(
       *error = errors::kInvalidPageActionIconPath;
       return NULL;
     }
-    result->SetDefaultIcon(default_icon);
+    result->set_default_icon_path(default_icon);
   }
 
   // Read the page action |default_title|.
@@ -1031,8 +946,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
     }
 
     page_action_.reset(
-        LoadExtensionActionHelper(page_action_value, error,
-                                  ExtensionAction::PAGE_ACTION));
+        LoadExtensionAction2Helper(page_action_value, error));
     if (!page_action_.get())
       return false;  // Failed to parse page action definition.
   } else if (source.HasKey(keys::kPageAction)) {
@@ -1043,8 +957,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
     }
 
     page_action_.reset(
-        LoadExtensionActionHelper(page_action_value, error,
-                                  ExtensionAction::PAGE_ACTION));
+        LoadExtensionAction2Helper(page_action_value, error));
     if (!page_action_.get())
       return false;  // Failed to parse page action definition.
   }
@@ -1179,9 +1092,9 @@ std::set<FilePath> Extension::GetBrowserImages() {
 
   // page action icons
   if (page_action_.get()) {
-    const std::vector<std::string>& icon_paths = page_action_->icon_paths();
-    for (std::vector<std::string>::const_iterator iter = icon_paths.begin();
-         iter != icon_paths.end(); ++iter) {
+    std::vector<std::string>* icon_paths = page_action_->icon_paths();
+    for (std::vector<std::string>::iterator iter = icon_paths->begin();
+         iter != icon_paths->end(); ++iter) {
       image_paths.insert(FilePath::FromWStringHack(UTF8ToWide(*iter)));
     }
   }
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 4a9fcd9..4677e2d 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -191,7 +191,7 @@ class Extension {
   const std::string& public_key() const { return public_key_; }
   const std::string& description() const { return description_; }
   const UserScriptList& content_scripts() const { return content_scripts_; }
-  ExtensionAction* page_action() const { return page_action_.get(); }
+  ExtensionAction2* page_action() const { return page_action_.get(); }
   ExtensionAction2* browser_action() const { return browser_action_.get(); }
   const std::vector<PrivacyBlacklistInfo>& privacy_blacklists() const {
     return privacy_blacklists_;
@@ -281,7 +281,7 @@ class Extension {
 
   // Helper method that loads a ExtensionAction object from a
   // dictionary in the page_action or browser_action section of the manifest.
-  ExtensionAction* LoadExtensionActionHelper(
+  ExtensionAction2* LoadExtensionActionHelper(
       const DictionaryValue* contextual_action,
       std::string* error,
       ExtensionAction::ExtensionActionType action_type);
@@ -326,7 +326,7 @@ class Extension {
   UserScriptList content_scripts_;
 
   // The extension's page action, if any.
-  scoped_ptr<ExtensionAction> page_action_;
+  scoped_ptr<ExtensionAction2> page_action_;
 
   // The extension's browser action, if any.
   scoped_ptr<ExtensionAction2> browser_action_;
diff --git a/chrome/common/extensions/extension_action2.cc b/chrome/common/extensions/extension_action2.cc
index 653de95..b36ef44 100755
--- a/chrome/common/extensions/extension_action2.cc
+++ b/chrome/common/extensions/extension_action2.cc
@@ -4,28 +4,129 @@
 
 #include "chrome/common/extensions/extension_action2.h"
 
-#include "base/logging.h"
+#include "app/gfx/canvas.h"
+#include "app/resource_bundle.h"
+#include "base/gfx/rect.h"
+#include "chrome/app/chrome_dll_resource.h"
+#include "grit/app_resources.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkTypeface.h"
+#include "third_party/skia/include/effects/SkGradientShader.h"
 
 const int ExtensionAction2::kDefaultTabId = -1;
 
-void ExtensionAction2::SetDefaultIcon(const std::string& path) {
-  default_icon_path_ = path;
-  icon_.erase(kDefaultTabId);
+void ExtensionAction2::ClearAllValuesForTab(int tab_id) {
+  title_.erase(tab_id);
+  icon_.erase(tab_id);
+  icon_index_.erase(tab_id);
+  badge_text_.erase(tab_id);
+  badge_text_color_.erase(tab_id);
+  badge_background_color_.erase(tab_id);
+  visible_.erase(tab_id);
 }
 
-void ExtensionAction2::SetDefaultIcon(int icon_index) {
-  if (static_cast<size_t>(icon_index) >= icon_paths_.size()) {
-    NOTREACHED();
+void ExtensionAction2::PaintBadge(gfx::Canvas* canvas,
+                                  const gfx::Rect& bounds,
+                                  int tab_id) {
+  std::string text = GetBadgeText(tab_id);
+  if (text.empty())
     return;
+
+  SkColor text_color = GetBadgeTextColor(tab_id);
+  SkColor background_color = GetBadgeBackgroundColor(tab_id);
+
+  if (SkColorGetA(text_color) == 0x00)
+    text_color = SK_ColorWHITE;
+
+  if (SkColorGetA(background_color) == 0x00)
+    background_color = SkColorSetARGB(255, 218, 0, 24);  // default badge color
+
+  // Different platforms need slightly different constants to look good.
+#if defined(OS_LINUX)
+  const int kTextSize = 9;
+  const int kBottomMargin = 4;
+  const int kPadding = 2;
+  const int kBadgeHeight = 12;
+  const int kMaxTextWidth = 23;
+  // The minimum width for center-aligning the badge.
+  const int kCenterAlignThreshold = 20;
+#else
+  const int kTextSize = 8;
+  const int kBottomMargin = 5;
+  const int kPadding = 2;
+  const int kBadgeHeight = 11;
+  const int kMaxTextWidth = 23;
+  // The minimum width for center-aligning the badge.
+  const int kCenterAlignThreshold = 20;
+#endif
+
+  canvas->save();
+
+  SkTypeface* typeface = SkTypeface::CreateFromName("Arial", SkTypeface::kBold);
+  SkPaint text_paint;
+  text_paint.setAntiAlias(true);
+  text_paint.setColor(text_color);
+  text_paint.setFakeBoldText(true);
+  text_paint.setTextAlign(SkPaint::kLeft_Align);
+  text_paint.setTextSize(SkIntToScalar(kTextSize));
+  text_paint.setTypeface(typeface);
+
+  // Calculate text width. We clamp it to a max size.
+  SkScalar text_width = text_paint.measureText(text.c_str(), text.size());
+  text_width = SkIntToScalar(
+      std::min(kMaxTextWidth, SkScalarFloor(text_width)));
+
+  // Cacluate badge size. It is clamped to a min width just because it looks
+  // silly if it is too skinny.
+  int badge_width = SkScalarFloor(text_width) + kPadding * 2;
+  badge_width = std::max(kBadgeHeight, badge_width);
+
+  // Paint the badge background color in the right location. It is usually
+  // right-aligned, but it can also be center-aligned if it is large.
+  SkRect rect;
+  rect.fBottom = SkIntToScalar(bounds.bottom() - kBottomMargin);
+  rect.fTop = rect.fBottom - SkIntToScalar(kBadgeHeight);
+  if (badge_width >= kCenterAlignThreshold) {
+    rect.fLeft = SkIntToScalar((bounds.right() - badge_width) / 2);
+    rect.fRight = rect.fLeft + SkIntToScalar(badge_width);
+  } else {
+    rect.fRight = SkIntToScalar(bounds.right());
+    rect.fLeft = rect.fRight - badge_width;
   }
 
-  SetDefaultIcon(icon_paths_[icon_index]);
-}
+  SkPaint rect_paint;
+  rect_paint.setStyle(SkPaint::kFill_Style);
+  rect_paint.setAntiAlias(true);
+  rect_paint.setColor(background_color);
+  canvas->drawRoundRect(rect, SkIntToScalar(2), SkIntToScalar(2), rect_paint);
 
-void ExtensionAction2::ClearAllValuesForTab(int tab_id) {
-  title_.erase(tab_id);
-  icon_.erase(tab_id);
-  badge_text_.erase(tab_id);
-  badge_background_color_.erase(tab_id);
-  badge_text_color_.erase(tab_id);
+  // Overlay the gradient. It is stretchy, so we do this in three parts.
+  ResourceBundle& resource_bundle = ResourceBundle::GetSharedInstance();
+  SkBitmap* gradient_left = resource_bundle.GetBitmapNamed(
+      IDR_BROWSER_ACTION_BADGE_LEFT);
+  SkBitmap* gradient_right = resource_bundle.GetBitmapNamed(
+      IDR_BROWSER_ACTION_BADGE_RIGHT);
+  SkBitmap* gradient_center = resource_bundle.GetBitmapNamed(
+      IDR_BROWSER_ACTION_BADGE_CENTER);
+
+  canvas->drawBitmap(*gradient_left, rect.fLeft, rect.fTop);
+  canvas->TileImageInt(*gradient_center,
+      SkScalarFloor(rect.fLeft) + gradient_left->width(),
+      SkScalarFloor(rect.fTop),
+      SkScalarFloor(rect.width()) - gradient_left->width() -
+                    gradient_right->width(),
+      SkScalarFloor(rect.height()));
+  canvas->drawBitmap(*gradient_right,
+      rect.fRight - SkIntToScalar(gradient_right->width()), rect.fTop);
+
+  // Finally, draw the text centered within the badge. We set a clip in case the
+  // text was too large.
+  rect.fLeft += kPadding;
+  rect.fRight -= kPadding;
+  canvas->clipRect(rect);
+  canvas->drawText(text.c_str(), text.size(),
+                   rect.fLeft + (rect.width() - text_width) / 2,
+                   rect.fTop + kTextSize + 1,
+                   text_paint);
+  canvas->restore();
 }
diff --git a/chrome/common/extensions/extension_action2.h b/chrome/common/extensions/extension_action2.h
index ae4cf34..53c62bc 100755
--- a/chrome/common/extensions/extension_action2.h
+++ b/chrome/common/extensions/extension_action2.h
@@ -10,11 +10,17 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/logging.h"
 #include "base/scoped_ptr.h"
 #include "googleurl/src/gurl.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColor.h"
 
+namespace gfx {
+class Canvas;
+class Rect;
+}
+
 // ExtensionAction2 encapsulates the state of a browser or page action.
 // Instances can have both global and per-tab state. If a property does not have
 // a per-tab value, the global value is used instead.
@@ -37,6 +43,13 @@ class ExtensionAction2 {
   void set_popup_url(const GURL& url) { popup_url_ = url; }
   bool has_popup() const { return !popup_url_.is_empty(); }
 
+  // action id -- only used with legacy page actions API
+  std::string id() const { return id_; }
+  void set_id(const std::string& id) { id_ = id; }
+
+  // static icon paths from manifest -- only used with legacy page actions API.
+  std::vector<std::string>* icon_paths() { return &icon_paths_; }
+
   // title
   void SetTitle(int tab_id, const std::string& title) {
     SetValue(&title_, tab_id, title);
@@ -50,20 +63,30 @@ class ExtensionAction2 {
   // To get the default icon, first check for the bitmap. If it is null, check
   // for the path.
 
-  // icon bitmap
+  // Icon bitmap.
   void SetIcon(int tab_id, const SkBitmap& bitmap) {
     SetValue(&icon_, tab_id, bitmap);
-    if (tab_id == kDefaultTabId)
-      default_icon_path_.clear();
   }
   SkBitmap GetIcon(int tab_id) { return GetValue(&icon_, tab_id); }
 
-  // icon path (relative to extension_id()'s root)
-  // For legacy code, we also support setting the path as an index into
-  // icon_paths().
-  void SetDefaultIcon(const std::string& path);
-  void SetDefaultIcon(int icon_index);
-  std::string GetDefaultIconPath() {
+  // Icon index -- for use with icon_paths(), only used in page actions.
+  void SetIconIndex(int tab_id, int index) {
+    if (static_cast<size_t>(index) >= icon_paths_.size()) {
+      NOTREACHED();
+      return;
+    }
+    SetValue(&icon_index_, tab_id, index);
+  }
+  int GetIconIndex(int tab_id) {
+    return GetValue(&icon_index_, tab_id);
+  }
+
+  // Non-tab-specific icon path. This is used to support the default_icon key of
+  // page and browser actions.
+  void set_default_icon_path(const std::string& path) {
+    default_icon_path_ = path;
+  }
+  std::string default_icon_path() {
     return default_icon_path_;
   }
 
@@ -89,16 +112,19 @@ class ExtensionAction2 {
     return GetValue(&badge_background_color_, tab_id);
   }
 
+  // visibility
+  void SetIsVisible(int tab_id, bool value) {
+    SetValue(&visible_, tab_id, value);
+  }
+  bool GetIsVisible(int tab_id) {
+    return GetValue(&visible_, tab_id);
+  }
+
   // Remove all tab-specific state.
   void ClearAllValuesForTab(int tab_id);
 
-  //---------------------------------------------------------------------------
-  // Legacy support
-
-  std::string id() const { return id_; }
-  void set_id(const std::string& id) { id_ = id; }
-
-  std::vector<std::string>* icon_paths() { return &icon_paths_; }
+  // If the specified tab has a badge, paint it into the provided bounds.
+  void PaintBadge(gfx::Canvas* canvas, const gfx::Rect& bounds, int tab_id);
 
  private:
   template <class T>
@@ -132,31 +158,30 @@ class ExtensionAction2 {
   // kDefaultTabId), or tab-specific state (stored with the tab_id as the key).
   std::map<int, std::string> title_;
   std::map<int, SkBitmap> icon_;
+  std::map<int, int> icon_index_;  // index into icon_paths_
   std::map<int, std::string> badge_text_;
   std::map<int, SkColor> badge_background_color_;
   std::map<int, SkColor> badge_text_color_;
+  std::map<int, bool> visible_;
 
   std::string default_icon_path_;
 
   // If the action has a popup, it has a URL and a height.
   GURL popup_url_;
 
-  //---------------------------------------------------------------------------
-  // Legacy support
-
   // The id for the ExtensionAction2, for example: "RssPageAction". This is
   // needed for compat with an older version of the page actions API.
   std::string id_;
 
   // A list of paths to icons this action might show. This is needed to support
-  // the setIcon({iconIndex:...} method.
+  // the legacy setIcon({iconIndex:...} method of the page actions API.
   std::vector<std::string> icon_paths_;
 };
 
-template <>
-struct ExtensionAction2::ValueTraits<SkColor> {
-  static SkColor CreateEmpty() {
-    return 0x00000000;
+template<>
+struct ExtensionAction2::ValueTraits<int> {
+  static int CreateEmpty() {
+    return -1;
   }
 };
 
diff --git a/chrome/common/extensions/extension_action2_unittest.cc b/chrome/common/extensions/extension_action2_unittest.cc
index 7f1d78b..47a32d7 100644
--- a/chrome/common/extensions/extension_action2_unittest.cc
+++ b/chrome/common/extensions/extension_action2_unittest.cc
@@ -65,6 +65,35 @@ TEST(ExtensionAction2Test, TabSpecificState) {
   ASSERT_TRUE(BitmapsAreEqual(icon1, action.GetIcon(1)));
   ASSERT_TRUE(BitmapsAreEqual(icon2, action.GetIcon(100)));
 
+  // icon index
+  ASSERT_EQ(-1, action.GetIconIndex(1));
+  action.icon_paths()->push_back("foo.png");
+  action.icon_paths()->push_back("bar.png");
+  action.SetIconIndex(ExtensionAction2::kDefaultTabId, 1);
+  ASSERT_EQ(1, action.GetIconIndex(1));
+  ASSERT_EQ(1, action.GetIconIndex(100));
+  action.SetIconIndex(100, 0);
+  ASSERT_EQ(0, action.GetIconIndex(100));
+  ASSERT_EQ(1, action.GetIconIndex(1));
+  action.ClearAllValuesForTab(100);
+  ASSERT_EQ(1, action.GetIconIndex(100));
+  ASSERT_EQ(1, action.GetIconIndex(1));
+
+  // visibility
+  ASSERT_EQ(false, action.GetIsVisible(1));
+  action.SetIsVisible(ExtensionAction2::kDefaultTabId, true);
+  ASSERT_EQ(true, action.GetIsVisible(1));
+  ASSERT_EQ(true, action.GetIsVisible(100));
+  action.SetIsVisible(ExtensionAction2::kDefaultTabId, false);
+  ASSERT_EQ(false, action.GetIsVisible(1));
+  ASSERT_EQ(false, action.GetIsVisible(100));
+  action.SetIsVisible(100, true);
+  ASSERT_EQ(false, action.GetIsVisible(1));
+  ASSERT_EQ(true, action.GetIsVisible(100));
+  action.ClearAllValuesForTab(100);
+  ASSERT_EQ(false, action.GetIsVisible(1));
+  ASSERT_EQ(false, action.GetIsVisible(100));
+
   // badge text
   ASSERT_EQ("", action.GetBadgeText(1));
   action.SetBadgeText(ExtensionAction2::kDefaultTabId, "foo");
@@ -80,48 +109,29 @@ TEST(ExtensionAction2Test, TabSpecificState) {
 
   // badge text color
   ASSERT_EQ(0x00000000u, action.GetBadgeTextColor(1));
-  action.SetBadgeTextColor(ExtensionAction2::kDefaultTabId, 0xFFFF0000);
+  action.SetBadgeTextColor(ExtensionAction2::kDefaultTabId, 0xFFFF0000u);
   ASSERT_EQ(0xFFFF0000u, action.GetBadgeTextColor(1));
   ASSERT_EQ(0xFFFF0000u, action.GetBadgeTextColor(100));
   action.SetBadgeTextColor(100, 0xFF00FF00);
   ASSERT_EQ(0xFFFF0000u, action.GetBadgeTextColor(1));
   ASSERT_EQ(0xFF00FF00u, action.GetBadgeTextColor(100));
-  action.SetBadgeTextColor(ExtensionAction2::kDefaultTabId, 0xFF0000FF);
+  action.SetBadgeTextColor(ExtensionAction2::kDefaultTabId, 0xFF0000FFu);
   ASSERT_EQ(0xFF0000FFu, action.GetBadgeTextColor(1));
   action.ClearAllValuesForTab(100);
   ASSERT_EQ(0xFF0000FFu, action.GetBadgeTextColor(100));
 
   // badge background color
   ASSERT_EQ(0x00000000u, action.GetBadgeBackgroundColor(1));
-  action.SetBadgeBackgroundColor(ExtensionAction2::kDefaultTabId, 0xFFFF0000);
+  action.SetBadgeBackgroundColor(ExtensionAction2::kDefaultTabId,
+                                 0xFFFF0000u);
   ASSERT_EQ(0xFFFF0000u, action.GetBadgeBackgroundColor(1));
   ASSERT_EQ(0xFFFF0000u, action.GetBadgeBackgroundColor(100));
   action.SetBadgeBackgroundColor(100, 0xFF00FF00);
   ASSERT_EQ(0xFFFF0000u, action.GetBadgeBackgroundColor(1));
   ASSERT_EQ(0xFF00FF00u, action.GetBadgeBackgroundColor(100));
-  action.SetBadgeBackgroundColor(ExtensionAction2::kDefaultTabId, 0xFF0000FF);
+  action.SetBadgeBackgroundColor(ExtensionAction2::kDefaultTabId,
+                                 0xFF0000FFu);
   ASSERT_EQ(0xFF0000FFu, action.GetBadgeBackgroundColor(1));
   action.ClearAllValuesForTab(100);
   ASSERT_EQ(0xFF0000FFu, action.GetBadgeBackgroundColor(100));
 }
-
-TEST(ExtensionAction2Test, IconOddCases) {
-  ExtensionAction2 action;
-
-  action.SetIcon(ExtensionAction2::kDefaultTabId, LoadIcon("icon1.png"));
-  action.SetDefaultIcon("foo.png");
-  ASSERT_TRUE(action.GetIcon(1).isNull());
-  ASSERT_EQ("foo.png", action.GetDefaultIconPath());
-
-  action.icon_paths()->push_back("a.png");
-  action.icon_paths()->push_back("b.png");
-  action.SetDefaultIcon(1);
-  ASSERT_TRUE(action.GetIcon(1).isNull());
-  ASSERT_EQ("b.png", action.GetDefaultIconPath());
-
-  action.SetIcon(100, LoadIcon("icon1.png"));
-  ASSERT_TRUE(!action.GetIcon(100).isNull());
-  action.SetIcon(ExtensionAction2::kDefaultTabId, LoadIcon("icon1.png"));
-  ASSERT_TRUE(!action.GetIcon(1).isNull());
-  ASSERT_EQ("", action.GetDefaultIconPath());
-}
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index eeb4499..dd366fb 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -297,18 +297,18 @@ TEST(ExtensionTest, LoadPageActionHelper) {
 #endif
   Extension extension(path);
   std::string error_msg;
-  scoped_ptr<ExtensionAction> action;
+  scoped_ptr<ExtensionAction2> action;
   DictionaryValue input;
 
   // First try with an empty dictionary. We should get nothing back.
-  ASSERT_TRUE(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::PAGE_ACTION) == NULL);
+  ASSERT_TRUE(extension.LoadExtensionAction2Helper(
+      &input, &error_msg) == NULL);
   ASSERT_STRNE("", error_msg.c_str());
   error_msg = "";
 
   // Now try the same, but as a browser action. Ensure same results.
-  ASSERT_TRUE(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::BROWSER_ACTION) == NULL);
+  ASSERT_TRUE(extension.LoadExtensionAction2Helper(
+      &input, &error_msg) == NULL);
   ASSERT_STRNE("", error_msg.c_str());
   error_msg = "";
 
@@ -326,46 +326,23 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   icons->Set(1, Value::CreateStringValue(img2));
   input.Set(keys::kPageActionIcons, icons);
 
-  // Parse as page action and read back the values from the object.
-  action.reset(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::PAGE_ACTION));
+  // Parse and read back the values from the object.
+  action.reset(extension.LoadExtensionAction2Helper(
+      &input, &error_msg));
   ASSERT_TRUE(NULL != action.get());
   ASSERT_STREQ("", error_msg.c_str());
   ASSERT_STREQ(id.c_str(), action->id().c_str());
-  ASSERT_STREQ(name.c_str(), action->title().c_str());
-  ASSERT_EQ(2u, action->icon_paths().size());
-  ASSERT_STREQ(img1.c_str(), action->icon_paths()[0].c_str());
-  ASSERT_STREQ(img2.c_str(), action->icon_paths()[1].c_str());
-  ASSERT_EQ(ExtensionAction::PAGE_ACTION, action->type());
-
-  // Now try the same, but as a browser action.
-  action.reset(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::BROWSER_ACTION));
-  ASSERT_TRUE(NULL != action.get());
-  ASSERT_STREQ("", error_msg.c_str());
-  // Browser actions don't have an id, page actions do.
-  ASSERT_STREQ("", action->id().c_str());
-  ASSERT_STREQ(name.c_str(), action->title().c_str());
-  ASSERT_EQ(2u, action->icon_paths().size());
-  ASSERT_STREQ(img1.c_str(), action->icon_paths()[0].c_str());
-  ASSERT_STREQ(img2.c_str(), action->icon_paths()[1].c_str());
-  ASSERT_EQ(ExtensionAction::BROWSER_ACTION, action->type());
+  ASSERT_STREQ(name.c_str(), action->GetTitle(1).c_str());
+  ASSERT_EQ(2u, action->icon_paths()->size());
+  ASSERT_STREQ(img1.c_str(), action->icon_paths()->at(0).c_str());
+  ASSERT_STREQ(img2.c_str(), action->icon_paths()->at(1).c_str());
 
   // Explicitly set the same type and parse again.
   input.SetString(keys::kType, values::kPageActionTypeTab);
-  action.reset(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::BROWSER_ACTION));
-  ASSERT_TRUE(NULL != action.get());
-  ASSERT_STREQ("", error_msg.c_str());
-  ASSERT_EQ(ExtensionAction::BROWSER_ACTION, action->type());
-
-  // Explicitly set the PAGE_ACTION type and parse again.
-  input.SetString(keys::kType, values::kPageActionTypePermanent);
-  action.reset(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::PAGE_ACTION));
+  action.reset(extension.LoadExtensionAction2Helper(
+      &input, &error_msg));
   ASSERT_TRUE(NULL != action.get());
   ASSERT_STREQ("", error_msg.c_str());
-  ASSERT_EQ(ExtensionAction::PAGE_ACTION, action->type());
 
   // Make a deep copy of the input and remove one key at a time and see if we
   // get the right error.
@@ -374,18 +351,8 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   // First remove id key.
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kPageActionId, NULL);
-  action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), &error_msg, ExtensionAction::PAGE_ACTION));
-  ASSERT_TRUE(NULL != action.get());
-  ASSERT_STREQ("", error_msg.c_str());
-  error_msg = "";
-
-  // Same test (id key), but with browser action.
-  copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
-  copy->Remove(keys::kPageActionId, NULL);
-  action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), &error_msg, ExtensionAction::BROWSER_ACTION));
-  // Having no id is valid for browser actions.
+  action.reset(extension.LoadExtensionAction2Helper(
+      copy.get(), &error_msg));
   ASSERT_TRUE(NULL != action.get());
   ASSERT_STREQ("", error_msg.c_str());
   error_msg = "";
@@ -393,18 +360,8 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   // Then remove the name key.
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kName, NULL);
-  action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), &error_msg, ExtensionAction::PAGE_ACTION));
-  ASSERT_TRUE(NULL == action.get());
-  ASSERT_TRUE(MatchPattern(error_msg.c_str(),
-                           errors::kInvalidPageActionDefaultTitle));
-  error_msg = "";
-
-  // Same test (name key), but with browser action.
-  copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
-  copy->Remove(keys::kName, NULL);
-  action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), &error_msg, ExtensionAction::BROWSER_ACTION));
+  action.reset(extension.LoadExtensionAction2Helper(
+      copy.get(), &error_msg));
   ASSERT_TRUE(NULL == action.get());
   ASSERT_TRUE(MatchPattern(error_msg.c_str(),
                            errors::kInvalidPageActionDefaultTitle));
@@ -413,18 +370,11 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   // Then remove the icon paths key.
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kPageActionIcons, NULL);
-  action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), &error_msg, ExtensionAction::PAGE_ACTION));
+  action.reset(extension.LoadExtensionAction2Helper(
+      copy.get(), &error_msg));
   ASSERT_TRUE(NULL != action.get());
   error_msg = "";
 
-  // Same test (name key), but with browser action.
-  copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
-  copy->Remove(keys::kPageActionIcons, NULL);
-  action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), &error_msg, ExtensionAction::BROWSER_ACTION));
-  ASSERT_TRUE(NULL != action.get());
-
   // Now test that we can parse the new format for page actions.
 
   // Now setup some values to use in the page action.
@@ -436,15 +386,13 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   input.SetString(keys::kPageActionDefaultTitle, kTitle);
   input.SetString(keys::kPageActionDefaultIcon, kIcon);
 
-  // Parse as page action and read back the values from the object.
-  action.reset(extension.LoadExtensionActionHelper(
-      &input, &error_msg, ExtensionAction::PAGE_ACTION));
+  // Parse and read back the values from the object.
+  action.reset(extension.LoadExtensionAction2Helper(
+      &input, &error_msg));
   ASSERT_TRUE(action.get());
   ASSERT_STREQ("", error_msg.c_str());
-  ASSERT_EQ(kTitle, action->title());
-  ASSERT_EQ(1u, action->icon_paths().size());
-  ASSERT_EQ(kIcon, action->icon_paths()[0]);
-  ASSERT_EQ(ExtensionAction::PAGE_ACTION, action->type());
+  ASSERT_EQ(kTitle, action->GetTitle(1));
+  ASSERT_EQ(0u, action->icon_paths()->size());
 }
 
 TEST(ExtensionTest, IdIsValid) {