Update page action manifest parsing to match the new format.

Preserves backwards compatability with the old manifest format.

BUG=24635

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

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

6 files changed, 146 insertions, 115 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 8b04577..1d34c97d 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -104,7 +104,6 @@ const size_t Extension::kNumPermissions =
     arraysize(Extension::kPermissionNames);
 
 Extension::~Extension() {
-  STLDeleteValues(&page_actions_);
 }
 
 const std::string Extension::VersionString() const {
@@ -146,20 +145,6 @@ GURL Extension::GetResourceURL(const GURL& extension_url,
   return ret_val;
 }
 
-const ExtensionAction* Extension::GetExtensionAction(
-    std::string id, ExtensionAction::ExtensionActionType action_type) const {
-  if (action_type == ExtensionAction::BROWSER_ACTION) {
-    DCHECK(id.empty());  // Multiple browser actions are not allowed.
-    return browser_action_.get();
-  } else {
-    ExtensionActionMap::const_iterator it = page_actions_.find(id);
-    if (it == page_actions_.end())
-      return NULL;
-
-    return it->second;
-  }
-}
-
 Extension::Location Extension::ExternalExtensionInstallType(
     std::string registry_path) {
 #if defined(OS_WIN)
@@ -310,73 +295,66 @@ bool Extension::LoadUserScriptHelper(const DictionaryValue* content_script,
 // 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, int definition_index,
-    std::string* error, ExtensionAction::ExtensionActionType action_type) {
+    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;
-  // Read the page action |icons|.
-  if (!page_action->HasKey(keys::kPageActionIcons) ||
-      !page_action->GetList(keys::kPageActionIcons, &icons) ||
-      icons->GetSize() == 0) {
-    // Icons are only required for page actions.
-    if (action_type == ExtensionAction::PAGE_ACTION) {
-      *error = ExtensionErrorUtils::FormatErrorMessage(
-          errors::kInvalidPageActionIconPaths, IntToString(definition_index));
-      return NULL;
-    }
-  }
-
-  int icon_count = 0;
-  if (icons) {
+  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 = ExtensionErrorUtils::FormatErrorMessage(
-            errors::kInvalidPageActionIconPath,
-            IntToString(definition_index), IntToString(icon_count));
+        *error = errors::kInvalidPageActionIconPath;
         return NULL;
       }
 
       result->AddIconPath(path);
-      ++icon_count;
     }
   }
 
-  if (action_type == ExtensionAction::BROWSER_ACTION) {
-    result->set_id("");  // Not needed (only 1 browser action per extension).
-  } else {
-    // Read the page action |id|.
-    std::string id;
-    if (!page_action->GetString(keys::kPageActionId, &id)) {
-      *error = ExtensionErrorUtils::FormatErrorMessage(
-          errors::kInvalidPageActionId, IntToString(definition_index));
+  // 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;
     }
-    result->set_id(id);
+    // TODO(EXTENSIONS_DEPRECATED): one icon.
+    result->AddIconPath(default_icon);
   }
 
-  // Read the page action |name|.
-  std::string name;
-  if (!page_action->GetString(keys::kName, &name)) {
-    *error = ExtensionErrorUtils::FormatErrorMessage(errors::kInvalidName,
-        IntToString(definition_index));
+  // 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_name(name);
+  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->GetDictionary(keys::kPageActionPopup, &popup) &&
+     !page_action->GetString(keys::kPageActionPopup, &url_str)) {
     *error = errors::kInvalidPageActionPopup;
     return NULL;
   }
   if (popup) {
-    std::string url_str;
+    // TODO(EXTENSIONS_DEPRECATED): popup is a string only
     if (!popup->GetString(keys::kPageActionPopupPath, &url_str)) {
       *error = ExtensionErrorUtils::FormatErrorMessage(
           errors::kInvalidPageActionPopupPath, "<missing>");
@@ -397,6 +375,17 @@ ExtensionAction* Extension::LoadExtensionActionHelper(
       return NULL;
     }
     result->set_popup_height(height);
+  } 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);
+    // TODO(erikkay): Need dynamic sizing of popups.
+    // http://code.google.com/p/chromium/issues/detail?id=24471
+    result->set_popup_height(100);
   }
 
   return result.release();
@@ -929,7 +918,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
     }
   }
 
-  // Initialize page actions (optional).
+  // Initialize page action (optional).
   if (source.HasKey(keys::kPageActions)) {
     ListValue* list_value;
     if (!source.GetList(keys::kPageActions, &list_value)) {
@@ -937,39 +926,52 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
       return false;
     }
 
-    for (size_t i = 0; i < list_value->GetSize(); ++i) {
-      DictionaryValue* page_action_value;
-      if (!list_value->GetDictionary(i, &page_action_value)) {
-        *error = ExtensionErrorUtils::FormatErrorMessage(
-            errors::kInvalidPageAction, IntToString(i));
-        return false;
-      }
+    if (list_value->GetSize() != 1u) {
+      *error = errors::kInvalidPageActionsListSize;
+      return false;
+    }
+
+    DictionaryValue* page_action_value;
+    if (!list_value->GetDictionary(0, &page_action_value)) {
+      *error = errors::kInvalidPageAction;
+      return false;
+    }
 
-      ExtensionAction* contextual_action =
-          LoadExtensionActionHelper(page_action_value, i, error,
-                                    ExtensionAction::PAGE_ACTION);
-      if (!contextual_action)
-        return false;  // Failed to parse page action definition.
-      page_actions_[contextual_action->id()] = contextual_action;
+    page_action_.reset(
+        LoadExtensionActionHelper(page_action_value, error,
+                                  ExtensionAction::PAGE_ACTION));
+    if (!page_action_.get())
+      return false;  // Failed to parse page action definition.
+  } else if (source.HasKey(keys::kPageAction)) {
+    DictionaryValue* page_action_value;
+    if (!source.GetDictionary(keys::kPageAction, &page_action_value)) {
+      *error = errors::kInvalidPageAction;
+      return false;
     }
+
+    page_action_.reset(
+        LoadExtensionActionHelper(page_action_value, error,
+                                  ExtensionAction::PAGE_ACTION));
+    if (!page_action_.get())
+      return false;  // Failed to parse page action definition.
   }
 
+  // Initialize browser action (optional).
   if (source.HasKey(keys::kBrowserAction)) {
     DictionaryValue* browser_action_value;
     if (!source.GetDictionary(keys::kBrowserAction, &browser_action_value)) {
-      *error = ExtensionErrorUtils::FormatErrorMessage(
-          errors::kInvalidBrowserAction, "");
+      *error = errors::kInvalidBrowserAction;
       return false;
     }
 
     browser_action_.reset(
-        LoadExtensionActionHelper(browser_action_value, 0, error,
+        LoadExtensionActionHelper(browser_action_value, error,
                                   ExtensionAction::BROWSER_ACTION));
     if (!browser_action_.get())
       return false;  // Failed to parse browser action definition.
 
     browser_action_state_.reset(
-        new ExtensionActionState(browser_action_->name(), 0));
+        new ExtensionActionState(browser_action_->title(), 0));
   }
 
   // Initialize the permissions (optional).
@@ -1075,9 +1077,17 @@ std::set<FilePath> Extension::GetBrowserImages() {
   }
 
   // page action icons
-  for (ExtensionActionMap::const_iterator it = page_actions().begin();
-       it != page_actions().end(); ++it) {
-    const std::vector<std::string>& icon_paths = it->second->icon_paths();
+  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) {
+      image_paths.insert(FilePath::FromWStringHack(UTF8ToWide(*iter)));
+    }
+  }
+
+  // browser action icons
+  if (browser_action_.get()) {
+    const std::vector<std::string>& icon_paths = browser_action_->icon_paths();
     for (std::vector<std::string>::const_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 57337de..0cb120b 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -185,7 +185,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_; }
-  const ExtensionActionMap& page_actions() const { return page_actions_; }
+  ExtensionAction* page_action() const { return page_action_.get(); }
   ExtensionAction* browser_action() const { return browser_action_.get(); }
   ExtensionActionState* browser_action_state() {
     return browser_action_state_.get();
@@ -220,10 +220,6 @@ class Extension {
   const GURL& update_url() const { return update_url_; }
   const std::map<int, std::string>& icons() { return icons_; }
 
-  // Retrieves a page action or browser action by |id|.
-  const ExtensionAction* GetExtensionAction(
-    std::string id, ExtensionAction::ExtensionActionType action_type) const;
-
   // Returns the origin of this extension. This function takes a |registry_path|
   // so that the registry location can be overwritten during testing.
   Location ExternalExtensionInstallType(std::string registry_path);
@@ -283,7 +279,6 @@ class Extension {
   // dictionary in the page_action or browser_action section of the manifest.
   ExtensionAction* LoadExtensionActionHelper(
       const DictionaryValue* contextual_action,
-      int definition_index,
       std::string* error,
       ExtensionAction::ExtensionActionType action_type);
 
@@ -320,8 +315,8 @@ class Extension {
   // Paths to the content scripts the extension contains.
   UserScriptList content_scripts_;
 
-  // A list of page actions.
-  ExtensionActionMap page_actions_;
+  // The extension's page action, if any.
+  scoped_ptr<ExtensionAction> page_action_;
 
   // The extension's browser action, if any.
   scoped_ptr<ExtensionAction> browser_action_;
diff --git a/chrome/common/extensions/extension_action.h b/chrome/common/extensions/extension_action.h
index 74c22d9..046ab63 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -38,8 +38,8 @@ class ExtensionAction {
     extension_id_ = extension_id;
   }
 
-  std::string name() const { return name_; }
-  void set_name(const std::string& name) { name_ = name; }
+  std::string title() const { return title_; }
+  void set_title(const std::string& title) { title_ = title; }
 
   const std::vector<std::string>& icon_paths() const { return icon_paths_; }
   void AddIconPath(const std::string& icon_path) {
@@ -68,8 +68,8 @@ class ExtensionAction {
   // the extension manifest).
   std::string extension_id_;
 
-  // The name of the ExtensionAction.
-  std::string name_;
+  // The title text of the ExtensionAction.
+  std::string title_;
 
   // The paths to the icons that this PageIcon can show.
   std::vector<std::string> icon_paths_;
@@ -116,7 +116,7 @@ class ExtensionActionState {
   void set_icon(SkBitmap* icon) { icon_.reset(icon); }
 
  private:
-  // The title to use.
+  // The title text to use for tooltips and labels.
   std::string title_;
 
   // The icon to use.
diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc
index b269848..db37ee5 100644
--- a/chrome/common/extensions/extension_constants.cc
+++ b/chrome/common/extensions/extension_constants.cc
@@ -18,8 +18,11 @@ const wchar_t* kJs = L"js";
 const wchar_t* kMatches = L"matches";
 const wchar_t* kName = L"name";
 const wchar_t* kPageActionId = L"id";
+const wchar_t* kPageAction = L"page_action";
 const wchar_t* kPageActions = L"page_actions";
 const wchar_t* kPageActionIcons = L"icons";
+const wchar_t* kPageActionDefaultIcon = L"default_icon";
+const wchar_t* kPageActionDefaultTitle = L"default_title";
 const wchar_t* kPageActionPopup = L"popup";
 const wchar_t* kPageActionPopupHeight = L"height";
 const wchar_t* kPageActionPopupPath = L"path";
@@ -93,15 +96,17 @@ const char* kInvalidMatches =
 const char* kInvalidName =
     "Required value 'name' is missing or invalid.";
 const char* kInvalidPageAction =
-    "Invalid value for 'page_actions[*]'.";
+    "Invalid value for 'page_action'.";
 const char* kInvalidPageActionIconPath =
-    "Invalid value for 'page_actions[*].icons[*]'.";
+    "Invalid value for 'page_action.default_icon'.";
 const char* kInvalidPageActionsList =
     "Invalid value for 'page_actions'.";
-const char* kInvalidPageActionIconPaths =
-    "Required value 'page_actions[*].icons' is missing or invalid.";
+const char* kInvalidPageActionsListSize =
+    "Invalid value for 'page_actions'. There can be only one.";
 const char* kInvalidPageActionId =
     "Required value 'id' is missing or invalid.";
+const char* kInvalidPageActionDefaultTitle =
+    "Required value 'default_title' is missing or invalid.";
 const char* kInvalidPageActionPopup =
     "Invalid type for page action popup.";
 const char* kInvalidPageActionPopupHeight =
diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h
index 61f8092..1282995 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/chrome/common/extensions/extension_constants.h
@@ -19,8 +19,11 @@ namespace extension_manifest_keys {
   extern const wchar_t* kMatches;
   extern const wchar_t* kName;
   extern const wchar_t* kPageActionId;
+  extern const wchar_t* kPageAction;
   extern const wchar_t* kPageActions;
   extern const wchar_t* kPageActionIcons;
+  extern const wchar_t* kPageActionDefaultIcon;
+  extern const wchar_t* kPageActionDefaultTitle;
   extern const wchar_t* kPageActionPopup;
   extern const wchar_t* kPageActionPopupHeight;
   extern const wchar_t* kPageActionPopupPath;
@@ -87,9 +90,10 @@ namespace extension_manifest_errors {
   extern const char* kInvalidVersion;
   extern const char* kInvalidPageAction;
   extern const char* kInvalidPageActionsList;
+  extern const char* kInvalidPageActionsListSize;
   extern const char* kInvalidPageActionIconPath;
-  extern const char* kInvalidPageActionIconPaths;
   extern const char* kInvalidPageActionId;
+  extern const char* kInvalidPageActionDefaultTitle;
   extern const char* kInvalidPageActionPopup;
   extern const char* kInvalidPageActionPopupHeight;
   extern const char* kInvalidPageActionPopupPath;
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 26cf648..55a8139 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -277,13 +277,13 @@ TEST(ExtensionTest, LoadPageActionHelper) {
 
   // First try with an empty dictionary. We should get nothing back.
   ASSERT_TRUE(extension.LoadExtensionActionHelper(
-      &input, 0, &error_msg, ExtensionAction::PAGE_ACTION) == NULL);
+      &input, &error_msg, ExtensionAction::PAGE_ACTION) == 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, 0, &error_msg, ExtensionAction::BROWSER_ACTION) == NULL);
+      &input, &error_msg, ExtensionAction::BROWSER_ACTION) == NULL);
   ASSERT_STRNE("", error_msg.c_str());
   error_msg = "";
 
@@ -303,11 +303,11 @@ TEST(ExtensionTest, LoadPageActionHelper) {
 
   // Parse as page action and read back the values from the object.
   action.reset(extension.LoadExtensionActionHelper(
-      &input, 0, &error_msg, ExtensionAction::PAGE_ACTION));
+      &input, &error_msg, ExtensionAction::PAGE_ACTION));
   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->name().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());
@@ -315,12 +315,12 @@ TEST(ExtensionTest, LoadPageActionHelper) {
 
   // Now try the same, but as a browser action.
   action.reset(extension.LoadExtensionActionHelper(
-      &input, 0, &error_msg, ExtensionAction::BROWSER_ACTION));
+      &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->name().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());
@@ -329,7 +329,7 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   // Explicitly set the same type and parse again.
   input.SetString(keys::kType, values::kPageActionTypeTab);
   action.reset(extension.LoadExtensionActionHelper(
-      &input, 0, &error_msg, ExtensionAction::BROWSER_ACTION));
+      &input, &error_msg, ExtensionAction::BROWSER_ACTION));
   ASSERT_TRUE(NULL != action.get());
   ASSERT_STREQ("", error_msg.c_str());
   ASSERT_EQ(ExtensionAction::BROWSER_ACTION, action->type());
@@ -337,7 +337,7 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   // Explicitly set the PAGE_ACTION type and parse again.
   input.SetString(keys::kType, values::kPageActionTypePermanent);
   action.reset(extension.LoadExtensionActionHelper(
-      &input, 0, &error_msg, ExtensionAction::PAGE_ACTION));
+      &input, &error_msg, ExtensionAction::PAGE_ACTION));
   ASSERT_TRUE(NULL != action.get());
   ASSERT_STREQ("", error_msg.c_str());
   ASSERT_EQ(ExtensionAction::PAGE_ACTION, action->type());
@@ -350,17 +350,16 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kPageActionId, NULL);
   action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), 0, &error_msg, ExtensionAction::PAGE_ACTION));
-  ASSERT_TRUE(NULL == action.get());
-  ASSERT_TRUE(MatchPattern(error_msg.c_str(),
-                           errors::kInvalidPageActionId));
+      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(), 0, &error_msg, ExtensionAction::BROWSER_ACTION));
+      copy.get(), &error_msg, ExtensionAction::BROWSER_ACTION));
   // Having no id is valid for browser actions.
   ASSERT_TRUE(NULL != action.get());
   ASSERT_STREQ("", error_msg.c_str());
@@ -370,39 +369,57 @@ TEST(ExtensionTest, LoadPageActionHelper) {
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kName, NULL);
   action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), 0, &error_msg, ExtensionAction::PAGE_ACTION));
+      copy.get(), &error_msg, ExtensionAction::PAGE_ACTION));
   ASSERT_TRUE(NULL == action.get());
   ASSERT_TRUE(MatchPattern(error_msg.c_str(),
-                           errors::kInvalidName));
+                           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(), 0, &error_msg, ExtensionAction::BROWSER_ACTION));
+      copy.get(), &error_msg, ExtensionAction::BROWSER_ACTION));
   ASSERT_TRUE(NULL == action.get());
   ASSERT_TRUE(MatchPattern(error_msg.c_str(),
-                           errors::kInvalidName));
+                           errors::kInvalidPageActionDefaultTitle));
   error_msg = "";
 
   // Then remove the icon paths key.
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kPageActionIcons, NULL);
   action.reset(extension.LoadExtensionActionHelper(
-      copy.get(), 0, &error_msg, ExtensionAction::PAGE_ACTION));
-  ASSERT_TRUE(NULL == action.get());
-  ASSERT_TRUE(MatchPattern(error_msg.c_str(),
-                           errors::kInvalidPageActionIconPaths));
+      copy.get(), &error_msg, ExtensionAction::PAGE_ACTION));
+  ASSERT_TRUE(NULL != action.get());
   error_msg = "";
 
-  // Same test (name key), but with browser action (icons are not required for
-  // browser actions).
+  // 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(), 0, &error_msg, ExtensionAction::BROWSER_ACTION));
+      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.
+  const std::string kTitle("MyExtensionActionTitle");
+  const std::string kIcon("image1.png");
+
+  // Add the dictionary for the contextual action.
+  input.Clear();
+  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));
+  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());
 }
 
 TEST(ExtensionTest, IdIsValid) {