Move ScriptBadge, ActionInfo out of Extension; preparation for BrowserAction

chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc

Index: chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc
diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc
index 736be658e57672309088f4767abbd57548abc22a..5e375ddfcc6b7dde106dbc6b3115d531277b0679 100644
--- a/chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc
+++ b/chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc
@@ -2,20 +2,20 @@
 // 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/script_badge_handler.h"
 #include "chrome/common/extensions/extension_builder.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_icon_set.h"
 #include "chrome/common/extensions/extension_manifest_constants.h"
 #include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
+#include "chrome/test/base/testing_profile.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace errors = extension_manifest_errors;
-using extensions::DictionaryBuilder;
-using extensions::Extension;
-using extensions::ExtensionBuilder;
 
-namespace {
+namespace extensions {
 
 std::vector<Extension::InstallWarning> StripMissingFlagWarning(
     const std::vector<Extension::InstallWarning>& install_warnings) {
@@ -28,6 +28,10 @@ std::vector<Extension::InstallWarning> StripMissingFlagWarning(
 }
 
 TEST_F(ExtensionManifestTest, ScriptBadgeBasic) {
+  // We need to make a profile so that the ProfileKeyedServices (some of which
+  // are used in manifest-parsing) are initialized.

[Yoyo Zhou, 2012/12/18 02:01:41]

It would be preferable to just register the manifest handlers directly here. But
additionally, you should subclass ExtensionManifestTest and do that in the SetUp
function.

I would also advocate for putting this test in
chrome/common/extensions/api/extension_action/script_badge_manifest_unittest.cc.
(I have no idea who pluralized manifests here.)

[Devlin, 2012/12/18 20:42:07]

File has been moved and ScriptBadgeManifestTest::SetUp has been added.  My only
concern with registering the handlers directly is that we'd have to register
_all_ the used handlers.  Typically, manifest tests appear to be small enough
that this isn't a big deal, but some features (e.g., icons) are used somewhat
frequently throughout multiple tests.  Would we want to instead just have
ExtensionManifestTest::SetUp which registers all existing handlers? (This would
be slower, but perhaps more stable, since we only ever have to modify that one
class; here, if we change icons, we'll also have to change the script badge
unittest, which is somewhat strange.)

I actually have no preference either way; just mentioning it as an option.

[Yoyo Zhou, 2012/12/19 01:38:50]

I think it's okay to make unit tests explicitly register the handlers they need.
Of course, if it gets to be unwieldy for some particular test then we can
reevaluate.

+  scoped_ptr<TestingProfile> profile(new TestingProfile());
+
   scoped_refptr<Extension> extension(
       ExtensionBuilder()
       .SetManifest(DictionaryBuilder()
@@ -45,12 +49,14 @@ TEST_F(ExtensionManifestTest, ScriptBadgeBasic) {
                         .Set("default_popup", "popup.html")))
       .Build());
   ASSERT_TRUE(extension.get());
-  ASSERT_TRUE(extension->script_badge_info());
+  const ActionInfo* script_badge_info =
+      ScriptBadgeInfo::GetScriptBadge(extension);
+  ASSERT_TRUE(script_badge_info);
   EXPECT_THAT(StripMissingFlagWarning(extension->install_warnings()),
               testing::ElementsAre(/*empty*/));
 
   const ExtensionIconSet& default_icon =
-      extension->script_badge_info()->default_icon;
+      script_badge_info->default_icon;
   // Should have a default icon set.
   ASSERT_FALSE(default_icon.empty());
 
@@ -63,11 +69,15 @@ TEST_F(ExtensionManifestTest, ScriptBadgeBasic) {
             default_icon.Get(2 * extension_misc::EXTENSION_ICON_BITTY,
                              ExtensionIconSet::MATCH_EXACTLY));
 
-  EXPECT_EQ("my extension", extension->script_badge_info()->default_title);
-  EXPECT_FALSE(extension->script_badge_info()->default_popup_url.is_empty());
+  EXPECT_EQ("my extension", script_badge_info->default_title);
+  EXPECT_FALSE(script_badge_info->default_popup_url.is_empty());
 }
 
 TEST_F(ExtensionManifestTest, ScriptBadgeExplicitTitleAndIconsIgnored) {
+  // We need to make a profile so that the ProfileKeyedServices (some of which
+  // are used in manifest-parsing) are initialized.
+  scoped_ptr<TestingProfile> profile(new TestingProfile());
+
   scoped_refptr<Extension> extension(
       ExtensionBuilder()
       .SetManifest(DictionaryBuilder()
@@ -83,7 +93,9 @@ TEST_F(ExtensionManifestTest, ScriptBadgeExplicitTitleAndIconsIgnored) {
                         .Set("default_icon", "malicious.png")))
       .Build());
   ASSERT_TRUE(extension.get());
-  ASSERT_TRUE(extension->script_badge_info());
+  const ActionInfo* script_badge_info =
+      ScriptBadgeInfo::GetScriptBadge(extension);
+  ASSERT_TRUE(script_badge_info);
 
   EXPECT_THAT(StripMissingFlagWarning(extension->install_warnings()),
               testing::ElementsAre(
@@ -95,7 +107,7 @@ TEST_F(ExtensionManifestTest, ScriptBadgeExplicitTitleAndIconsIgnored) {
                       errors::kScriptBadgeIconIgnored)));
 
   const ExtensionIconSet& default_icon =
-      extension->script_badge_info()->default_icon;
+      script_badge_info->default_icon;
   ASSERT_FALSE(default_icon.empty());
 
   EXPECT_EQ(1u, default_icon.map().size());
@@ -103,10 +115,14 @@ TEST_F(ExtensionManifestTest, ScriptBadgeExplicitTitleAndIconsIgnored) {
             default_icon.Get(extension_misc::EXTENSION_ICON_BITTY,
                              ExtensionIconSet::MATCH_EXACTLY));
 
-  EXPECT_EQ("my extension", extension->script_badge_info()->default_title);
+  EXPECT_EQ("my extension", script_badge_info->default_title);
 }
 
 TEST_F(ExtensionManifestTest, ScriptBadgeIconFallsBackToPuzzlePiece) {
+  // We need to make a profile so that the ProfileKeyedServices (some of which
+  // are used in manifest-parsing) are initialized.
+  scoped_ptr<TestingProfile> profile(new TestingProfile());
+
   scoped_refptr<Extension> extension(
       ExtensionBuilder()
       .SetManifest(DictionaryBuilder()
@@ -119,12 +135,14 @@ TEST_F(ExtensionManifestTest, ScriptBadgeIconFallsBackToPuzzlePiece) {
                         .Set("128", "icon128.png")))
       .Build());
   ASSERT_TRUE(extension.get());
-  ASSERT_TRUE(extension->script_badge_info());
+  const ActionInfo* script_badge_info =
+      ScriptBadgeInfo::GetScriptBadge(extension);
+  ASSERT_TRUE(script_badge_info);
   EXPECT_THAT(extension->install_warnings(),
               testing::ElementsAre(/*empty*/));
 
   const ExtensionIconSet& default_icon =
-      extension->script_badge_info()->default_icon;
+      script_badge_info->default_icon;
   ASSERT_FALSE(default_icon.empty()) << "Should fall back to the 128px icon.";
   EXPECT_EQ(2u, default_icon.map().size());
   EXPECT_EQ("icon128.png",
@@ -135,4 +153,4 @@ TEST_F(ExtensionManifestTest, ScriptBadgeIconFallsBackToPuzzlePiece) {
                              ExtensionIconSet::MATCH_EXACTLY));
 }
 
-}  // namespace
+}  // namespace extensions