Move BrowserAction out of Extension

BUG=159265

Review URL: https://chromiumcodereview.appspot.com/11644057

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

7 files changed, 255 insertions, 9 deletions

diff --git a/chrome/common/extensions/api/extension_action/action_info.cc b/chrome/common/extensions/api/extension_action/action_info.cc
index 7d7ac10..5201575 100644
--- a/chrome/common/extensions/api/extension_action/action_info.cc
+++ b/chrome/common/extensions/api/extension_action/action_info.cc
@@ -29,6 +29,13 @@ ActionInfoData::ActionInfoData(ActionInfo* info) : action_info(info) {
 ActionInfoData::~ActionInfoData() {
 }
 
+static const ActionInfo* GetActionInfo(const Extension* extension,
+                                       const std::string& key) {
+  ActionInfoData* data = static_cast<ActionInfoData*>(
+      extension->GetManifestData(key));
+  return data ? data->action_info.get() : NULL;
+}
+
 }  // namespace
 
 ActionInfo::ActionInfo() {
@@ -38,10 +45,19 @@ ActionInfo::~ActionInfo() {
 }
 
 // static
+const ActionInfo* ActionInfo::GetBrowserActionInfo(const Extension* extension) {
+  return GetActionInfo(extension, extension_manifest_keys::kBrowserAction);
+}
+
+// static
 const ActionInfo* ActionInfo::GetScriptBadgeInfo(const Extension* extension) {
-  ActionInfoData* data = static_cast<ActionInfoData*>(
-      extension->GetManifestData(extension_manifest_keys::kScriptBadge));
-  return data ? data->action_info.get() : NULL;
+  return GetActionInfo(extension, extension_manifest_keys::kScriptBadge);
+}
+
+// static
+void ActionInfo::SetBrowserActionInfo(Extension* extension, ActionInfo* info) {
+  extension->SetManifestData(extension_manifest_keys::kBrowserAction,
+                             new ActionInfoData(info));
 }
 
 // static
diff --git a/chrome/common/extensions/api/extension_action/action_info.h b/chrome/common/extensions/api/extension_action/action_info.h
index 09e7ecc..c17759a 100644
--- a/chrome/common/extensions/api/extension_action/action_info.h
+++ b/chrome/common/extensions/api/extension_action/action_info.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_icon_set.h"
 #include "googleurl/src/gurl.h"
 
@@ -26,12 +27,17 @@ struct ActionInfo {
     TYPE_SYSTEM_INDICATOR,
   };
 
-  // Returns the appropriate ActionInfo for the given |extension|.
-  static const ActionInfo* GetScriptBadgeInfo(const Extension* extension);
+  // Returns the extension's browser action, if any.
+  static const ActionInfo* GetBrowserActionInfo(const Extension* extension);
 
-  // Sets the appropriate ActionInfo as ManifestData for the given |extension|.
-  // This is static since |extension| takes ownership of |info|.
-  static void SetScriptBadgeInfo(Extension* extension, ActionInfo* info);
+  // Returns the extension's script badge.
+  static const ActionInfo* GetScriptBadgeInfo(const Extension* etxension);
+
+  // Sets the extension's browser action. |extension| takes ownership of |info|.
+  static void SetBrowserActionInfo(Extension* extension, ActionInfo* info);
+
+  // Sets the extension's script badge. |extension| takes ownership of |info|.
+  static void SetScriptBadgeInfo(Extension* etxension, ActionInfo* info);
 
   // Empty implies the key wasn't present.
   ExtensionIconSet default_icon;
diff --git a/chrome/common/extensions/api/extension_action/browser_action_handler.cc b/chrome/common/extensions/api/extension_action/browser_action_handler.cc
new file mode 100644
index 0000000..45ec188
--- /dev/null
+++ b/chrome/common/extensions/api/extension_action/browser_action_handler.cc
@@ -0,0 +1,43 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/extensions/api/extension_action/browser_action_handler.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "base/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_manifest_constants.h"
+#include "chrome/common/extensions/feature_switch.h"
+#include "chrome/common/extensions/manifest_handler_helpers.h"
+
+namespace extensions {
+
+BrowserActionHandler::BrowserActionHandler() {
+}
+
+BrowserActionHandler::~BrowserActionHandler() {
+}
+
+bool BrowserActionHandler::Parse(const base::Value* value,
+                                 Extension* extension,
+                                 string16* error) {
+  const DictionaryValue* dict = NULL;
+  if (!value->GetAsDictionary(&dict)) {
+    *error = ASCIIToUTF16(extension_manifest_errors::kInvalidBrowserAction);
+    return false;
+  }
+
+  scoped_ptr<ActionInfo> action_info =
+      manifest_handler_helpers::LoadActionInfo(extension, dict, error);
+  if (!action_info.get())
+    return false;  // Failed to parse browser action definition.
+
+  ActionInfo::SetBrowserActionInfo(extension, action_info.release());
+
+  return true;
+}
+
+}  // namespace extensions
diff --git a/chrome/common/extensions/api/extension_action/browser_action_handler.h b/chrome/common/extensions/api/extension_action/browser_action_handler.h
new file mode 100644
index 0000000..e979445
--- /dev/null
+++ b/chrome/common/extensions/api/extension_action/browser_action_handler.h
@@ -0,0 +1,29 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_COMMON_EXTENSIONS_API_EXTENSION_ACTION_BROWSER_ACTION_HANDLER_H_
+#define CHROME_COMMON_EXTENSIONS_API_EXTENSION_ACTION_BROWSER_ACTION_HANDLER_H_
+
+#include <string>
+
+#include "chrome/common/extensions/api/extension_action/action_info.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/manifest_handler.h"
+
+namespace extensions {
+
+// Parses the "browser_action" manifest key.
+class BrowserActionHandler : public ManifestHandler {
+ public:
+  BrowserActionHandler();
+  virtual ~BrowserActionHandler();
+
+  virtual bool Parse(const base::Value* value,
+                     Extension* extension,
+                     string16* error) OVERRIDE;
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_COMMON_EXTENSIONS_API_EXTENSION_ACTION_BROWSER_ACTION_HANDLER_H_
diff --git a/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc b/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc
new file mode 100644
index 0000000..01ead27
--- /dev/null
+++ b/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc
@@ -0,0 +1,122 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/extensions/api/extension_action/action_info.h"
+#include "chrome/common/extensions/api/extension_action/browser_action_handler.h"
+#include "chrome/common/extensions/extension_builder.h"
+#include "chrome/common/extensions/extension_icon_set.h"
+#include "chrome/common/extensions/extension_manifest_constants.h"
+#include "chrome/common/extensions/manifest_handler.h"
+#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
+#include "chrome/common/extensions/value_builder.h"
+#include "extensions/common/error_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace errors = extension_manifest_errors;
+
+namespace extensions {
+namespace {
+
+class BrowserActionManifestTest : public ExtensionManifestTest {
+ protected:
+  virtual void SetUp() OVERRIDE {
+    ExtensionManifestTest::SetUp();
+    ManifestHandler::Register(extension_manifest_keys::kBrowserAction,
+                              new BrowserActionHandler);
+  }
+};
+
+TEST_F(BrowserActionManifestTest,
+       BrowserActionManifestIcons_NoDefaultIcons) {
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder()
+      .SetManifest(DictionaryBuilder()
+                   .Set("name", "No default properties")
+                   .Set("version", "1.0.0")
+                   .Set("manifest_version", 2)
+                   .Set("browser_action", DictionaryBuilder()
+                       .Set("default_title", "Title")))
+      .Build();
+
+  ASSERT_TRUE(extension.get());
+  const ActionInfo* browser_action_info =
+      ActionInfo::GetBrowserActionInfo(extension);
+  ASSERT_TRUE(browser_action_info);
+  EXPECT_TRUE(browser_action_info->default_icon.empty());
+}
+
+TEST_F(BrowserActionManifestTest,
+       BrowserActionManifestIcons_StringDefaultIcon) {
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder()
+      .SetManifest(DictionaryBuilder()
+                   .Set("name", "String default icon")
+                   .Set("version", "1.0.0")
+                   .Set("manifest_version", 2)
+                   .Set("browser_action", DictionaryBuilder()
+                       .Set("default_icon", "icon.png")))
+      .Build();
+
+  ASSERT_TRUE(extension.get());
+  const ActionInfo* browser_action_info =
+      ActionInfo::GetBrowserActionInfo(extension);
+  ASSERT_TRUE(browser_action_info);
+  ASSERT_FALSE(browser_action_info->default_icon.empty());
+
+  const ExtensionIconSet& icons = browser_action_info->default_icon;
+
+  EXPECT_EQ(1u, icons.map().size());
+  EXPECT_EQ("icon.png", icons.Get(19, ExtensionIconSet::MATCH_EXACTLY));
+}
+
+TEST_F(BrowserActionManifestTest,
+       BrowserActionManifestIcons_DictDefaultIcon) {
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder()
+      .SetManifest(DictionaryBuilder()
+                   .Set("name", "Dictionary default icon")
+                   .Set("version", "1.0.0")
+                   .Set("manifest_version", 2)
+                   .Set("browser_action", DictionaryBuilder()
+                       .Set("default_icon", DictionaryBuilder()
+                           .Set("19", "icon19.png")
+                           .Set("24", "icon24.png")  // Should be ignored.
+                           .Set("38", "icon38.png"))))
+      .Build();
+
+  ASSERT_TRUE(extension.get());
+  const ActionInfo* browser_action_info =
+      ActionInfo::GetBrowserActionInfo(extension);
+  ASSERT_TRUE(browser_action_info);
+  ASSERT_FALSE(browser_action_info->default_icon.empty());
+
+  const ExtensionIconSet& icons = browser_action_info->default_icon;
+
+  // 24px icon should be ignored.
+  EXPECT_EQ(2u, icons.map().size());
+  EXPECT_EQ("icon19.png", icons.Get(19, ExtensionIconSet::MATCH_EXACTLY));
+  EXPECT_EQ("icon38.png", icons.Get(38, ExtensionIconSet::MATCH_EXACTLY));
+}
+
+TEST_F(BrowserActionManifestTest,
+       BrowserActionManifestIcons_InvalidDefaultIcon) {
+  scoped_ptr<DictionaryValue> manifest_value = DictionaryBuilder()
+      .Set("name", "Invalid default icon")
+      .Set("version", "1.0.0")
+      .Set("manifest_version", 2)
+      .Set("browser_action", DictionaryBuilder()
+          .Set("default_icon", DictionaryBuilder()
+              .Set("19", "")  // Invalid value.
+              .Set("24", "icon24.png")
+              .Set("38", "icon38.png")))
+     .Build();
+
+  string16 error = ErrorUtils::FormatErrorMessageUTF16(
+      errors::kInvalidIconPath, "19");
+  LoadAndExpectError(Manifest(manifest_value.get(), "Invalid default icon"),
+                     errors::kInvalidIconPath);
+}
+
+}  // namespace
+}  // namespace extensions
diff --git a/chrome/common/extensions/api/extension_action/page_action_manifest_unittest.cc b/chrome/common/extensions/api/extension_action/page_action_manifest_unittest.cc
new file mode 100644
index 0000000..f19e37e
--- /dev/null
+++ b/chrome/common/extensions/api/extension_action/page_action_manifest_unittest.cc
@@ -0,0 +1,29 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
+
+#include "chrome/common/extensions/api/extension_action/action_info.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_manifest_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+TEST_F(ExtensionManifestTest, PageActionManifestVersion2) {
+  scoped_refptr<Extension> extension(
+      LoadAndExpectSuccess("page_action_manifest_version_2.json"));
+  ASSERT_TRUE(extension.get());
+  ASSERT_TRUE(extension->page_action_info());
+
+  EXPECT_EQ("", extension->page_action_info()->id);
+  EXPECT_TRUE(extension->page_action_info()->default_icon.empty());
+  EXPECT_EQ("", extension->page_action_info()->default_title);
+  EXPECT_TRUE(extension->page_action_info()->default_popup_url.is_empty());
+
+  LoadAndExpectError("page_action_manifest_version_2b.json",
+                     extension_manifest_errors::kInvalidPageActionPopup);
+}
+
+}  // namespace extensions
diff --git a/chrome/common/extensions/api/omnibox/omnibox_handler.cc b/chrome/common/extensions/api/omnibox/omnibox_handler.cc
index ece5388..287021dd 100644
--- a/chrome/common/extensions/api/omnibox/omnibox_handler.cc
+++ b/chrome/common/extensions/api/omnibox/omnibox_handler.cc
@@ -9,6 +9,7 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/common/extensions/api/commands/commands_handler.h"
+#include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_manifest_constants.h"
 
@@ -31,7 +32,7 @@ const std::string& OmniboxInfo::GetKeyword(const Extension* extension) {
 // static
 bool OmniboxInfo::IsVerboseInstallMessage(const Extension* extension) {
   return !GetKeyword(extension).empty() ||
-      extension->browser_action_info() ||
+      ActionInfo::GetBrowserActionInfo(extension) ||
       (extension->page_action_info() &&
        (CommandsInfo::GetPageActionCommand(extension) ||
         !extension->page_action_info()->default_icon.empty()));