Change extension icon load errors to warnings

chrome/browser/extensions/api/extension_action/browser_action_unittest.cc

+// Copyright 2015 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 "base/files/file_util.h"
+#include "chrome/browser/extensions/extension_service_test_with_install.h"
+#include "chrome/common/extensions/api/extension_action/action_info.h"
+
+namespace extensions {
+namespace {
+
+class BrowserActionUnitTest : public ExtensionServiceTestWithInstall {
+};
+
+TEST_F(BrowserActionUnitTest, MultiIcons) {
+  InitializeEmptyExtensionService();
+  base::FilePath path =
+      data_dir().AppendASCII("api_test/browser_action/multi_icons");
+  ASSERT_TRUE(base::PathExists(path));
+
+  const Extension* extension = PackAndInstallCRX(path, INSTALL_NEW);
+
+  EXPECT_EQ(0U, extension->install_warnings().size());
+  const ActionInfo* browser_action_info =
+      ActionInfo::GetBrowserActionInfo(extension);
+  ASSERT_TRUE(browser_action_info);
+
+  const ExtensionIconSet& icons = browser_action_info->default_icon;
+
+  // Extension can provide arbitrary sizes.
+  EXPECT_EQ(4u, icons.map().size());
+  EXPECT_EQ("icon19.png", icons.Get(19, ExtensionIconSet::MATCH_EXACTLY));
+  EXPECT_EQ("icon24.png", icons.Get(24, ExtensionIconSet::MATCH_EXACTLY));
+  EXPECT_EQ("icon24.png", icons.Get(31, ExtensionIconSet::MATCH_EXACTLY));

[Devlin, 2015/12/19 01:43:19]

This looks a little weird, even though it's correct in the manifest file.  Mind
just copying icon24.png to icon31.png so that it doesn't look like we're just
matching nearest?

(If you like, fine to do in a followup)

[Evan Stade, 2015/12/19 01:49:58]

I could do that but I thought we were going for ultimate oddballer status. We
don't want to imply we care about the name of the icon or that we couldn't have
one icon be pointed at two times... maybe?

[Devlin, 2015/12/19 02:06:11]

Unittests are cheap.  EXPECT_EQs in a unittest are practically free.  Let's
check the following oddballs. ;)
- Size that isn't one of the main sizes with a normal key (e.g., "31":
"icon31.png")
- Size that uses the same icon twice - let's even make it deliberately not the
closest icon (e.g., "38": "icon24.png")
- Size that uses a random name (e.g., "40": "an_unrelated_name.png")

+Any others you can think of.

+  EXPECT_EQ("icon38.png", icons.Get(38, ExtensionIconSet::MATCH_EXACTLY));
+}
+
+TEST_F(BrowserActionUnitTest, MissingIconInWebstoreCrx) {
+  InitializeEmptyExtensionService();
+  base::FilePath path =
+      data_dir().AppendASCII("api_test/browser_action/missing_icon");
+  ASSERT_TRUE(base::PathExists(path));
+
+  const Extension* extension = PackAndInstallCRX(path, INSTALL_NEW);
+
+  ASSERT_EQ(1U, extension->install_warnings().size());
+  EXPECT_EQ("Could not load extension icon 'icon19.png'.",
+            extension->install_warnings().front().message);
+  const ActionInfo* browser_action_info =
+      ActionInfo::GetBrowserActionInfo(extension);
+  ASSERT_TRUE(browser_action_info);
+
+  const ExtensionIconSet& icons = browser_action_info->default_icon;
+
+  EXPECT_EQ(2u, icons.map().size());
+  EXPECT_EQ("icon24.png", icons.Get(24, ExtensionIconSet::MATCH_EXACTLY));
+  EXPECT_EQ("icon38.png", icons.Get(38, ExtensionIconSet::MATCH_EXACTLY));
+}
+
+TEST_F(BrowserActionUnitTest, MissingIconInCommandLine) {
+  InitializeEmptyExtensionService();
+  base::FilePath path =
+      data_dir().AppendASCII("api_test/browser_action/missing_icon");
+  ASSERT_TRUE(base::PathExists(path));
+
+  PackAndInstallCRXWithLocation(path, Manifest::COMMAND_LINE, INSTALL_FAILED);
+}
+
+} // namespace
+} // namespace extensions