Change browser/page action default icon defined in manifest to support hidpi.

chrome/common/extensions/manifest_tests/extension_manifests_browseraction_unittest.cc

+// 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/extension_action.h"
+#include "chrome/common/extensions/extension_error_utils.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 "testing/gtest/include/gtest/gtest.h"
+
+namespace errors = extension_manifest_errors;
+
+TEST_F(ExtensionManifestTest, BrowserActionManifestIcons) {
+  {

[Jeffrey Yasskin, 2012/09/17 20:18:39]

Make each of these blocks its own TEST_F. That gives you more assurance that
they don't affect each other and allows each section to run even if other
sections fail.

[tbarzic, 2012/09/17 22:06:10]

Done.

+    scoped_refptr<extensions::Extension> extension(
+        LoadAndExpectSuccess("browser_action_no_default_properties.json"));

[Jeffrey Yasskin, 2012/09/17 20:18:39]

Instead of storing the json you want to load separately, where it's hard to
compare it against the test's assertions, use
chrome/common/extensions/value_builder.h and possibly .../extension_builder.h to
write the manifest inline.

[tbarzic, 2012/09/17 22:06:10]

Done.

+    ASSERT_TRUE(extension.get());
+    ASSERT_TRUE(extension->browser_action());
+    EXPECT_FALSE(extension->browser_action()->default_icon());
+  }
+
+  {
+    scoped_refptr<extensions::Extension> extension(
+        LoadAndExpectSuccess("browser_action_with_string_default_icon.json"));
+    ASSERT_TRUE(extension.get());
+    ASSERT_TRUE(extension->browser_action());
+    ASSERT_TRUE(extension->browser_action()->default_icon());
+
+    const ExtensionIconSet* icons = extension->browser_action()->default_icon();
+
+    EXPECT_EQ(1u, icons->map().size());
+    EXPECT_EQ("icon.png", icons->Get(19, ExtensionIconSet::MATCH_EXACTLY));
+  }
+
+  {
+    scoped_refptr<extensions::Extension> extension(LoadAndExpectSuccess(
+        "browser_action_with_dictionary_default_icon.json"));
+    ASSERT_TRUE(extension.get());
+    ASSERT_TRUE(extension->browser_action());
+    ASSERT_TRUE(extension->browser_action()->default_icon());
+
+    const ExtensionIconSet* icons = extension->browser_action()->default_icon();
+
+    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));

[Jeffrey Yasskin, 2012/09/17 20:18:39]

Please comment that you're testing that the 24px icon is ignored.

[tbarzic, 2012/09/17 22:06:10]

Done.

+  }
+
+  {
+    string16 error = ExtensionErrorUtils::FormatErrorMessageUTF16(
+        errors::kInvalidIconPath, "19");
+    LoadAndExpectError("browser_action_invalid_default_icons.json",
+                       errors::kInvalidIconPath);
+  }
+}