Use high resolution icons where possible for streamlined hosted app icons.

chrome/browser/ui/web_applications/hosted_app_tab_helper_unittest.cc

Index: chrome/browser/ui/web_applications/hosted_app_tab_helper_unittest.cc
diff --git a/chrome/browser/ui/web_applications/hosted_app_tab_helper_unittest.cc b/chrome/browser/ui/web_applications/hosted_app_tab_helper_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..02dd0bfc79068c2d72e9891d9df93e3a5e2af893
--- /dev/null
+++ b/chrome/browser/ui/web_applications/hosted_app_tab_helper_unittest.cc
@@ -0,0 +1,221 @@
+// Copyright 2013 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/browser/ui/web_applications/hosted_app_tab_helper.h"
+
+#include "base/command_line.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/extensions/crx_installer.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/test_extension_service.h"
+#include "chrome/browser/extensions/test_extension_system.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/manifest_handlers/icons_handler.h"
+#include "chrome/common/web_application_info.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/common/favicon_url.h"
+#include "content/public/test/test_renderer_host.h"
+#include "content/public/test/test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
+using content::RenderViewHostTester;
+
+namespace {
+
+// Creates valid SkBitmaps of the dimensions found in |sizes| and pushes them
+// into |bitmaps|.
+std::vector<SkBitmap> CreateTestBitmaps(const std::vector<gfx::Size>& sizes) {
+  std::vector<SkBitmap> bitmaps(sizes.size());
+  for (size_t i = 0; i < sizes.size(); ++i) {
+    SkBitmap& bitmap = bitmaps[i];
+    bitmap.setConfig(SkBitmap::kARGB_8888_Config,
+                     sizes[i].width(),
+                     sizes[i].height());
+    bitmap.allocPixels();
+    bitmap.eraseColor(SK_ColorRED);
+  }
+  return bitmaps;
+}
+
+class TestHostedAppTabHelper : public HostedAppTabHelper {
+ public:
+  explicit TestHostedAppTabHelper(content::WebContents* web_contents)
+      : HostedAppTabHelper(web_contents),
+        id_counter_(0) {
+  }
+  virtual ~TestHostedAppTabHelper() {}
+
+  virtual int DownloadImage(const GURL& url) OVERRIDE {
+    return id_counter_++;
+  }
+
+ private:
+  int id_counter_;
+  DISALLOW_COPY_AND_ASSIGN(TestHostedAppTabHelper);
+};
+
+class HostedAppTabHelperTest : public ChromeRenderViewHostTestHarness,
+                               public content::NotificationObserver {
+ protected:
+  HostedAppTabHelperTest() : extension_(NULL) {
+  }
+
+  virtual ~HostedAppTabHelperTest() {
+  }
+
+  virtual void SetUp() OVERRIDE {
+    ChromeRenderViewHostTestHarness::SetUp();
+
+    web_app_info_.app_url = GURL("http://www.google.com");
+    web_app_info_.title = ASCIIToUTF16("Hosted App");
+    extension_ = NULL;
+
+    // A real extension service is needed for the CrxInstaller.
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    static_cast<extensions::TestExtensionSystem*>(
+        extensions::ExtensionSystem::Get(profile()))->
+        CreateExtensionService(
+            CommandLine::ForCurrentProcess(),
+            temp_dir_.path(),
+            false);
+
+    HostedAppTabHelper::CreateForWebContents(web_contents());

[benwells, 2013/11/25 00:40:16]

Why is this created here, when there are Test ones created in the tests?

[calamity, 2013/11/29 05:45:36]

Removed.

+
+    // Listen for notifications which indicate the hosted app installation has
+    // finished.
+    registrar_.Add(this,
+                   chrome::NOTIFICATION_CRX_INSTALLER_DONE,
+                   content::Source<extensions::CrxInstaller>(NULL));
+
+    registrar_.Add(this,
+                   chrome::NOTIFICATION_EXTENSION_INSTALL_ERROR,
+                   content::NotificationService::AllSources());
+  }
+
+  // Overriden from content::NotificationObserver:
+  void Observe(int type,
+               const content::NotificationSource& source,
+               const content::NotificationDetails& details) OVERRIDE {
+    if (type == chrome::NOTIFICATION_CRX_INSTALLER_DONE)
+      extension_ = content::Details<const extensions::Extension>(details).ptr();
+
+    base::MessageLoop::current()->Quit();
+  }
+
+  const extensions::Extension* extension() {
+    return extension_;
+  }
+
+  WebApplicationInfo* web_app_info() {
+    return &web_app_info_;
+  }
+
+ private:
+  const extensions::Extension* extension_;
+  WebApplicationInfo web_app_info_;
+  content::NotificationRegistrar registrar_;
+  base::ScopedTempDir temp_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostedAppTabHelperTest);
+};
+
+}  // namespace
+
+TEST_F(HostedAppTabHelperTest, CreateHostedApp) {
+  TestHostedAppTabHelper helper(web_contents());
+
+  std::vector<content::FaviconURL> favicon_urls;
+  favicon_urls.push_back(
+      content::FaviconURL(GURL("http://www.google.com/favicon.ico"),
+                          content::FaviconURL::FAVICON));
+
+  // Download should not be initiated because |fetch_icons_immediately| is not

[benwells, 2013/11/25 00:40:16]

This comment is out of date.

[calamity, 2013/11/29 05:45:36]

Done.

+  // set.
+  helper.DidUpdateFaviconURL(0, favicon_urls);
+  EXPECT_EQ(0, helper.pending_requests());
+
+  // Create hosted app which will initiate download.
+  helper.CreateHostedApp(*web_app_info());
+  EXPECT_EQ(1, helper.pending_requests());
+
+  std::vector<gfx::Size> sizes(1, gfx::Size(32, 32));
+  helper.DidDownloadFavicon(0, 200, favicon_urls[0].icon_url,
+                            CreateTestBitmaps(sizes), sizes);
+
+  // Create and install the hosted app.
+  content::RunMessageLoop();
+
+  EXPECT_TRUE(extension());
+  EXPECT_TRUE(extension()->from_bookmark());
+
+  const ExtensionIconSet& icons = extensions::IconsInfo::GetIcons(extension());
+  EXPECT_EQ(1, icons.map().size());
+  EXPECT_FALSE(icons.Get(32, ExtensionIconSet::MATCH_EXACTLY).empty());
+}
+
+TEST_F(HostedAppTabHelperTest, CreateHostedAppBeforeFaviconURLUpdate) {
+  TestHostedAppTabHelper helper(web_contents());
+  const GURL empty_favicon = GURL("http://www.google.com/empty_favicon.ico");
+
+  // This should get downloaded.
+  web_app_info()->icons.push_back(WebApplicationInfo::IconInfo());
+  web_app_info()->icons[0].url = GURL("http://www.google.com/favicon2.ico");
+
+  // This is duplicated in the favicon urls and should only be downloaded once.
+  web_app_info()->icons.push_back(WebApplicationInfo::IconInfo());
+  web_app_info()->icons[1].url = empty_favicon;
+
+  // Initiate creation of hosted app before favicon URLs are loaded.
+  helper.CreateHostedApp(*web_app_info());
+
+  std::vector<content::FaviconURL> favicon_urls;
+  favicon_urls.push_back(
+      content::FaviconURL(GURL("http://www.google.com/favicon.ico"),
+                          content::FaviconURL::FAVICON));
+  // This is duplicated in the favicon urls and should only be downloaded once.
+  favicon_urls.push_back(
+      content::FaviconURL(empty_favicon,
+                          content::FaviconURL::FAVICON));
+  // Invalid icons shouldn't get put into the download queue.
+  favicon_urls.push_back(
+      content::FaviconURL(GURL("http://www.google.com/invalid.ico"),
+                          content::FaviconURL::INVALID_ICON));
+  helper.DidUpdateFaviconURL(0, favicon_urls);

[benwells, 2013/11/25 00:40:16]

I think it would be clearer to check the number of pending requests here.

[calamity, 2013/11/29 05:45:36]

Done.

+
+  std::vector<gfx::Size> sizes_1(1, gfx::Size(16, 16));
+  helper.DidDownloadFavicon(0, 200, favicon_urls[0].icon_url,
+                            CreateTestBitmaps(sizes_1), sizes_1);
+
+  std::vector<gfx::Size> sizes_2;
+  sizes_2.push_back(gfx::Size(32, 32));
+  sizes_2.push_back(gfx::Size(64, 64));
+  helper.DidDownloadFavicon(1, 200, web_app_info()->icons[0].url,
+                            CreateTestBitmaps(sizes_2), sizes_2);
+
+  // Only 1 download should have been initiated for |empty_favicon| even though
+  // the URL was in both the web app info and the favicon urls.
+  helper.DidDownloadFavicon(2, 200, empty_favicon,
+                            std::vector<SkBitmap>(), std::vector<gfx::Size>());
+
+  // Create and install the hosted app.
+  content::RunMessageLoop();
+
+  EXPECT_TRUE(extension());
+  EXPECT_TRUE(extension()->from_bookmark());
+
+  const ExtensionIconSet& icons = extensions::IconsInfo::GetIcons(extension());
+  EXPECT_EQ(3, icons.map().size());
+  EXPECT_FALSE(icons.Get(16, ExtensionIconSet::MATCH_EXACTLY).empty());
+  EXPECT_FALSE(icons.Get(32, ExtensionIconSet::MATCH_EXACTLY).empty());
+  EXPECT_FALSE(icons.Get(64, ExtensionIconSet::MATCH_EXACTLY).empty());
+}