Allow InstallableManager to fetch optional badge icon

chrome/browser/installable/installable_manager_browsertest.cc

 // Copyright 2016 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/installable/installable_manager.h"
 
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 
+using IconPurpose = content::Manifest::Icon::IconPurpose;
+
 namespace {
 
+const std::tuple<int, int, IconPurpose> kPrimaryIconParams{144, 144,
+                                                           IconPurpose::ANY};
+
 InstallableParams GetManifestParams() {
   InstallableParams params;
   params.check_installable = false;
   params.fetch_valid_primary_icon = false;
   return params;
 }
 
 InstallableParams GetWebAppParams() {
   InstallableParams params = GetManifestParams();
   params.ideal_primary_icon_size_in_px = 144;
   params.minimum_primary_icon_size_in_px = 144;
   params.check_installable = true;
   params.fetch_valid_primary_icon = true;
   return params;
 }
 
-InstallableParams GetIconParams() {
+InstallableParams GetPrimaryIconParams() {
   InstallableParams params = GetManifestParams();
   params.ideal_primary_icon_size_in_px = 144;
   params.minimum_primary_icon_size_in_px = 144;
   params.fetch_valid_primary_icon = true;
   return params;
 }
 
+InstallableParams GetPrimaryIconAndBadgeIconParams() {
+  InstallableParams params = GetManifestParams();
+  params.ideal_primary_icon_size_in_px = 144;
+  params.minimum_primary_icon_size_in_px = 144;
+  params.fetch_valid_primary_icon = true;
+  params.ideal_badge_icon_size_in_px = 72;
+  params.minimum_badge_icon_size_in_px = 72;
+  params.fetch_valid_badge_icon = true;
+  return params;
+}
+
 }  // anonymous namespace
 
 class CallbackTester {
  public:
   explicit CallbackTester(base::Closure quit_closure)
       : quit_closure_(quit_closure) { }
 
   void OnDidFinishInstallableCheck(const InstallableData& data) {
     error_code_ = data.error_code;
     manifest_url_ = data.manifest_url;
     manifest_ = data.manifest;
-    icon_url_ = data.primary_icon_url;
+    primary_icon_url_ = data.primary_icon_url;
     if (data.primary_icon)
-      icon_.reset(new SkBitmap(*data.primary_icon));
+      primary_icon_.reset(new SkBitmap(*data.primary_icon));
+    badge_icon_url_ = data.badge_icon_url;
+    if (data.badge_icon)
+      badge_icon_.reset(new SkBitmap(*data.badge_icon));
     is_installable_ = data.is_installable;
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, quit_closure_);
   }
 
   InstallableStatusCode error_code() const { return error_code_; }
   const GURL& manifest_url() const { return manifest_url_; }
   const content::Manifest& manifest() const { return manifest_; }
-  const GURL& icon_url() const { return icon_url_; }
-  const SkBitmap* icon() const { return icon_.get(); }
+  const GURL& primary_icon_url() const { return primary_icon_url_; }
+  const SkBitmap* primary_icon() const { return primary_icon_.get(); }
+  const GURL& badge_icon_url() const { return badge_icon_url_; }
+  const SkBitmap* badge_icon() const { return badge_icon_.get(); }
   bool is_installable() const { return is_installable_; }
 
  private:
   base::Closure quit_closure_;
   InstallableStatusCode error_code_;
   GURL manifest_url_;
   content::Manifest manifest_;
-  GURL icon_url_;
-  std::unique_ptr<SkBitmap> icon_;
+  GURL primary_icon_url_;
+  std::unique_ptr<SkBitmap> primary_icon_;
+  GURL badge_icon_url_;
+  std::unique_ptr<SkBitmap> badge_icon_;
   bool is_installable_;
 };
 
 class NestedCallbackTester {
  public:
   NestedCallbackTester(InstallableManager* manager,
                        const InstallableParams& params,
                        base::Closure quit_closure)
       : manager_(manager), params_(params), quit_closure_(quit_closure) {}
 
   void Run() {
     manager_->GetData(params_,
                       base::Bind(&NestedCallbackTester::OnDidFinishFirstCheck,
                                  base::Unretained(this)));
   }
 
   void OnDidFinishFirstCheck(const InstallableData& data) {
     error_code_ = data.error_code;
     manifest_url_ = data.manifest_url;
     manifest_ = data.manifest;
-    icon_url_ = data.primary_icon_url;
+    primary_icon_url_ = data.primary_icon_url;
     if (data.primary_icon)
-      icon_.reset(new SkBitmap(*data.primary_icon));
+      primary_icon_.reset(new SkBitmap(*data.primary_icon));
     is_installable_ = data.is_installable;
 
     manager_->GetData(params_,
                       base::Bind(&NestedCallbackTester::OnDidFinishSecondCheck,
                                  base::Unretained(this)));
   }
 
   void OnDidFinishSecondCheck(const InstallableData& data) {
     EXPECT_EQ(error_code_, data.error_code);
     EXPECT_EQ(manifest_url_, data.manifest_url);
-    EXPECT_EQ(icon_url_, data.primary_icon_url);
-    EXPECT_EQ(icon_.get(), data.primary_icon);
+    EXPECT_EQ(primary_icon_url_, data.primary_icon_url);
+    EXPECT_EQ(primary_icon_.get(), data.primary_icon);
     EXPECT_EQ(is_installable_, data.is_installable);
     EXPECT_EQ(manifest_.IsEmpty(), data.manifest.IsEmpty());
     EXPECT_EQ(manifest_.start_url, data.manifest.start_url);
     EXPECT_EQ(manifest_.display, data.manifest.display);
     EXPECT_EQ(manifest_.name, data.manifest.name);
     EXPECT_EQ(manifest_.short_name, data.manifest.short_name);
 
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, quit_closure_);
   }
 
  private:
   InstallableManager* manager_;
   InstallableParams params_;
   base::Closure quit_closure_;
   InstallableStatusCode error_code_;
   GURL manifest_url_;
   content::Manifest manifest_;
-  GURL icon_url_;
-  std::unique_ptr<SkBitmap> icon_;
+  GURL primary_icon_url_;
+  std::unique_ptr<SkBitmap> primary_icon_;
   bool is_installable_;
 };
 
 class InstallableManagerBrowserTest : public InProcessBrowserTest {
  public:
   void SetUpOnMainThread() override {
     InProcessBrowserTest::SetUpOnMainThread();
     ASSERT_TRUE(embedded_test_server()->Start());
   }
 
@@ skipping 62 matching lines @@
   std::unique_ptr<CallbackTester> tester(
       new CallbackTester(run_loop.QuitClosure()));
 
   NavigateAndRunInstallableManager(tester.get(), GetManifestParams(),
                                    "/banners/no_manifest_test_page.html");
   run_loop.Run();
 
   // If there is no manifest, everything should be empty.
   EXPECT_TRUE(tester->manifest().IsEmpty());
   EXPECT_TRUE(tester->manifest_url().is_empty());
-  EXPECT_TRUE(tester->icon_url().is_empty());
-  EXPECT_EQ(nullptr, tester->icon());
+  EXPECT_TRUE(tester->primary_icon_url().is_empty());

[dominickn, 2017/01/27 06:40:43]

Tests which don't fetch the badge should check that it is nullptr (many).

[F, 2017/01/27 20:11:17]

Done.

+  EXPECT_EQ(nullptr, tester->primary_icon());
   EXPECT_FALSE(tester->is_installable());
   EXPECT_EQ(NO_MANIFEST, tester->error_code());
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckManifest404) {
   base::RunLoop run_loop;
   std::unique_ptr<CallbackTester> tester(
       new CallbackTester(run_loop.QuitClosure()));
 
   NavigateAndRunInstallableManager(tester.get(), GetManifestParams(),
       GetURLOfPageWithServiceWorkerAndManifest(
           "/banners/manifest_missing.json"));
   run_loop.Run();
 
   // The installable manager should return a manifest URL even if it 404s.
   // However, the check should fail with a ManifestEmpty error.
   EXPECT_TRUE(tester->manifest().IsEmpty());
 
   EXPECT_FALSE(tester->manifest_url().is_empty());
-  EXPECT_TRUE(tester->icon_url().is_empty());
-  EXPECT_EQ(nullptr, tester->icon());
+  EXPECT_TRUE(tester->primary_icon_url().is_empty());
+  EXPECT_EQ(nullptr, tester->primary_icon());
   EXPECT_FALSE(tester->is_installable());
   EXPECT_EQ(MANIFEST_EMPTY, tester->error_code());
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckManifestOnly) {
   // Verify that asking for just the manifest works as expected.
   base::RunLoop run_loop;
   std::unique_ptr<CallbackTester> tester(
       new CallbackTester(run_loop.QuitClosure()));
 
   NavigateAndRunInstallableManager(tester.get(), GetManifestParams(),
                                    "/banners/manifest_test_page.html");
   run_loop.Run();
 
   EXPECT_FALSE(tester->manifest().IsEmpty());
   EXPECT_FALSE(tester->manifest_url().is_empty());
 
-  EXPECT_TRUE(tester->icon_url().is_empty());
-  EXPECT_EQ(nullptr, tester->icon());
+  EXPECT_TRUE(tester->primary_icon_url().is_empty());
+  EXPECT_EQ(nullptr, tester->primary_icon());
   EXPECT_FALSE(tester->is_installable());
   EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                        CheckInstallableParamsDefaultConstructor) {
   // Verify that using InstallableParams' default constructor is equivalent to
   // just asking for the manifest alone.
   base::RunLoop run_loop;
   std::unique_ptr<CallbackTester> tester(
       new CallbackTester(run_loop.QuitClosure()));
 
   InstallableParams params;
   NavigateAndRunInstallableManager(tester.get(), params,
                                    "/banners/manifest_test_page.html");
   run_loop.Run();
 
   EXPECT_FALSE(tester->manifest().IsEmpty());
   EXPECT_FALSE(tester->manifest_url().is_empty());
 
-  EXPECT_TRUE(tester->icon_url().is_empty());
-  EXPECT_EQ(nullptr, tester->icon());
+  EXPECT_TRUE(tester->primary_icon_url().is_empty());
+  EXPECT_EQ(nullptr, tester->primary_icon());
   EXPECT_FALSE(tester->is_installable());
   EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                        CheckManifestWithOnlyRelatedApplications) {
   // This page has a manifest with only related applications specified. Asking
   // for just the manifest should succeed.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     NavigateAndRunInstallableManager(tester.get(), GetManifestParams(),
                                      "/banners/play_app_test_page.html");
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_TRUE(tester->manifest().prefer_related_applications);
 
-    EXPECT_TRUE(tester->icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->icon());
+    EXPECT_TRUE(tester->primary_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->primary_icon());
     EXPECT_FALSE(tester->is_installable());
     EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
   }
 
-  // Ask for an icon (but don't navigate). This should fail with
+  // Ask for a primary icon (but don't navigate). This should fail with
   // NO_ACCEPTABLE_ICON.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
-    RunInstallableManager(tester.get(), GetIconParams());
+    RunInstallableManager(tester.get(), GetPrimaryIconParams());
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_TRUE(tester->manifest().prefer_related_applications);
 
-    EXPECT_TRUE(tester->icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->icon());
+    EXPECT_TRUE(tester->primary_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->primary_icon());
     EXPECT_FALSE(tester->is_installable());
     EXPECT_EQ(NO_ACCEPTABLE_ICON, tester->error_code());
   }
 
-  // Ask for everything. This should fail with NO_ACCEPTABLE_ICON - the icon
-  // fetch has already failed, so that cached error stops the installable check
-  // from being performed.
+  // Ask for everything except badge icon. This should fail with
+  // NO_ACCEPTABLE_ICON - the primary icon fetch has already failed, so that
+  // cached error stops the installable check from being performed.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     RunInstallableManager(tester.get(), GetWebAppParams());
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_TRUE(tester->manifest().prefer_related_applications);
 
-    EXPECT_TRUE(tester->icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->icon());
+    EXPECT_TRUE(tester->primary_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->primary_icon());
     EXPECT_FALSE(tester->is_installable());
     EXPECT_EQ(NO_ACCEPTABLE_ICON, tester->error_code());
   }
 
-  // Ask for a different size icon. This should fail with START_URL_NOT_VALID
-  // since we won't have a cached icon error.
+  // Ask for a different size primary icon. This should fail with
+  // START_URL_NOT_VALID since we won't have a cached icon error.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     InstallableParams params = GetWebAppParams();
     params.ideal_primary_icon_size_in_px = 96;
     params.minimum_primary_icon_size_in_px = 96;
     RunInstallableManager(tester.get(), params);
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_TRUE(tester->manifest().prefer_related_applications);
 
-    EXPECT_TRUE(tester->icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->icon());
+    EXPECT_TRUE(tester->primary_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->primary_icon());
     EXPECT_FALSE(tester->is_installable());
     EXPECT_EQ(START_URL_NOT_VALID, tester->error_code());
   }
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckManifestAndIcon) {
-  // Add to homescreen checks for manifest + icon.
-  base::RunLoop run_loop;
-  std::unique_ptr<CallbackTester> tester(
-      new CallbackTester(run_loop.QuitClosure()));
+  // Add to homescreen checks for manifest + primary icon.
+  {
+    base::RunLoop run_loop;
+    std::unique_ptr<CallbackTester> tester(
+        new CallbackTester(run_loop.QuitClosure()));
 
-  NavigateAndRunInstallableManager(tester.get(), GetIconParams(),
-                                   "/banners/manifest_test_page.html");
-  run_loop.Run();
+    NavigateAndRunInstallableManager(tester.get(), GetPrimaryIconParams(),
+                                     "/banners/manifest_test_page.html");
+    run_loop.Run();
 
-  EXPECT_FALSE(tester->manifest().IsEmpty());
-  EXPECT_FALSE(tester->manifest_url().is_empty());
+    EXPECT_FALSE(tester->manifest().IsEmpty());
+    EXPECT_FALSE(tester->manifest_url().is_empty());
 
-  EXPECT_FALSE(tester->icon_url().is_empty());
-  EXPECT_NE(nullptr, tester->icon());
+    EXPECT_FALSE(tester->primary_icon_url().is_empty());
+    EXPECT_NE(nullptr, tester->primary_icon());
 
-  EXPECT_FALSE(tester->is_installable());
-  EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
+    EXPECT_FALSE(tester->is_installable());
+    EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
+  }
+
+  // Add to homescreen checks for manifest + primary icon + badge icon.
+  {
+    base::RunLoop run_loop;
+    std::unique_ptr<CallbackTester> tester(
+        new CallbackTester(run_loop.QuitClosure()));
+
+    RunInstallableManager(tester.get(), GetPrimaryIconAndBadgeIconParams());
+    run_loop.Run();
+
+    EXPECT_FALSE(tester->manifest().IsEmpty());
+    EXPECT_FALSE(tester->manifest_url().is_empty());
+
+    EXPECT_FALSE(tester->primary_icon_url().is_empty());
+    EXPECT_NE(nullptr, tester->primary_icon());
+
+    EXPECT_FALSE(tester->badge_icon_url().is_empty());
+    EXPECT_NE(nullptr, tester->badge_icon());
+
+    EXPECT_FALSE(tester->is_installable());
+    EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
+  }
+
+  // Request an oversized badge icon. This should should only primary icon, but

[dominickn, 2017/01/27 06:40:43]

This comment is a bit weird

[F, 2017/01/27 20:11:17]

Done. Thanks!

+  // without errors.
+  {
+    base::RunLoop run_loop;
+    std::unique_ptr<CallbackTester> tester(
+        new CallbackTester(run_loop.QuitClosure()));
+
+    InstallableParams params = GetPrimaryIconAndBadgeIconParams();
+    params.ideal_badge_icon_size_in_px = 2000;
+    params.minimum_badge_icon_size_in_px = 2000;
+    RunInstallableManager(tester.get(), params);
+    run_loop.Run();
+
+    EXPECT_FALSE(tester->manifest().IsEmpty());
+    EXPECT_FALSE(tester->manifest_url().is_empty());
+
+    EXPECT_FALSE(tester->primary_icon_url().is_empty());
+    EXPECT_NE(nullptr, tester->primary_icon());
+
+    EXPECT_TRUE(tester->badge_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->badge_icon());
+
+    EXPECT_FALSE(tester->is_installable());
+    EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckWebapp) {
-  // Request everything.
+  // Request everything except badge icon.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     NavigateAndRunInstallableManager(tester.get(), GetWebAppParams(),
                                      "/banners/manifest_test_page.html");
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_TRUE(tester->is_installable());
-    EXPECT_FALSE(tester->icon_url().is_empty());
-    EXPECT_NE(nullptr, tester->icon());
+    EXPECT_FALSE(tester->primary_icon_url().is_empty());
+    EXPECT_NE(nullptr, tester->primary_icon());
     EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
 
     // Verify that the returned state matches manager internal state.
     InstallableManager* manager = GetManager();
 
     EXPECT_FALSE(manager->manifest().IsEmpty());
     EXPECT_FALSE(manager->manifest_url().is_empty());
     EXPECT_TRUE(manager->is_installable());
     EXPECT_EQ(1u, manager->icons_.size());
-    EXPECT_FALSE((manager->icon_url({144,144}).is_empty()));
-    EXPECT_NE(nullptr, (manager->icon({144,144})));
+    EXPECT_FALSE((manager->icon_url(kPrimaryIconParams).is_empty()));
+    EXPECT_NE(nullptr, (manager->icon(kPrimaryIconParams)));
     EXPECT_EQ(NO_ERROR_DETECTED, manager->manifest_error());
     EXPECT_EQ(NO_ERROR_DETECTED, manager->installable_error());
-    EXPECT_EQ(NO_ERROR_DETECTED, (manager->icon_error({144,144})));
+    EXPECT_EQ(NO_ERROR_DETECTED, (manager->icon_error(kPrimaryIconParams)));
     EXPECT_TRUE(manager->tasks_.empty());
   }
 
-  // Request everything again without navigating away. This should work fine.
+  // Request everything except badge icon again without navigating away. This
+  // should work fine.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     RunInstallableManager(tester.get(), GetWebAppParams());
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_TRUE(tester->is_installable());
-    EXPECT_FALSE(tester->icon_url().is_empty());
-    EXPECT_NE(nullptr, tester->icon());
+    EXPECT_FALSE(tester->primary_icon_url().is_empty());
+    EXPECT_NE(nullptr, tester->primary_icon());
     EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
 
     // Verify that the returned state matches manager internal state.
     InstallableManager* manager = GetManager();
 
     EXPECT_FALSE(manager->manifest().IsEmpty());
     EXPECT_FALSE(manager->manifest_url().is_empty());
     EXPECT_TRUE(manager->is_installable());
     EXPECT_EQ(1u, manager->icons_.size());
-    EXPECT_FALSE((manager->icon_url({144,144}).is_empty()));
-    EXPECT_NE(nullptr, (manager->icon({144,144})));
+    EXPECT_FALSE((manager->icon_url(kPrimaryIconParams).is_empty()));
+    EXPECT_NE(nullptr, (manager->icon(kPrimaryIconParams)));
     EXPECT_EQ(NO_ERROR_DETECTED, manager->manifest_error());
     EXPECT_EQ(NO_ERROR_DETECTED, manager->installable_error());
-    EXPECT_EQ(NO_ERROR_DETECTED, (manager->icon_error({144,144})));
+    EXPECT_EQ(NO_ERROR_DETECTED, (manager->icon_error(kPrimaryIconParams)));
     EXPECT_TRUE(manager->tasks_.empty());
   }
 
   {
     // Check that a subsequent navigation resets state.
     ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
     InstallableManager* manager = GetManager();
 
     EXPECT_TRUE(manager->manifest().IsEmpty());
     EXPECT_TRUE(manager->manifest_url().is_empty());
@@ skipping 11 matching lines @@
       new CallbackTester(run_loop.QuitClosure()));
 
   NavigateAndRunInstallableManager(tester.get(), GetWebAppParams(),
                                    "/banners/iframe_test_page.html");
   run_loop.Run();
 
   // The installable manager should only retrieve items in the main frame;
   // everything should be empty here.
   EXPECT_TRUE(tester->manifest().IsEmpty());
   EXPECT_TRUE(tester->manifest_url().is_empty());
-  EXPECT_TRUE(tester->icon_url().is_empty());
-  EXPECT_EQ(nullptr, tester->icon());
+  EXPECT_TRUE(tester->primary_icon_url().is_empty());
+  EXPECT_EQ(nullptr, tester->primary_icon());
   EXPECT_FALSE(tester->is_installable());
   EXPECT_EQ(NO_MANIFEST, tester->error_code());
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                        CheckPageWithManifestAndNoServiceWorker) {
   // Just fetch the manifest. This should have no error.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     NavigateAndRunInstallableManager(
         tester.get(), GetManifestParams(),
         "/banners/manifest_no_service_worker.html");
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
 
-    EXPECT_TRUE(tester->icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->icon());
+    EXPECT_TRUE(tester->primary_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->primary_icon());
     EXPECT_FALSE(tester->is_installable());
     EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
   }
 
   // Fetch the full criteria should fail.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     RunInstallableManager(tester.get(), GetWebAppParams());
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
 
-    EXPECT_TRUE(tester->icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->icon());
+    EXPECT_TRUE(tester->primary_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->primary_icon());
     EXPECT_FALSE(tester->is_installable());
     EXPECT_EQ(NO_MATCHING_SERVICE_WORKER, tester->error_code());
   }
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                        CheckManifestCorruptedIcon) {
-  // Verify that the returned InstallableData::icon is null if the web manifest
-  // points to a corrupt icon.
+  // Verify that the returned InstallableData::primary_icon is null if the web
+  // manifest points to a corrupt primary icon.
   base::RunLoop run_loop;
   std::unique_ptr<CallbackTester> tester(
       new CallbackTester(run_loop.QuitClosure()));
 
-  NavigateAndRunInstallableManager(tester.get(), GetIconParams(),
+  NavigateAndRunInstallableManager(tester.get(), GetPrimaryIconParams(),
                                    GetURLOfPageWithServiceWorkerAndManifest(
                                        "/banners/manifest_bad_icon.json"));
   run_loop.Run();
 
   EXPECT_FALSE(tester->manifest().IsEmpty());
   EXPECT_FALSE(tester->manifest_url().is_empty());
-  EXPECT_TRUE(tester->icon_url().is_empty());
-  EXPECT_EQ(nullptr, tester->icon());
+  EXPECT_TRUE(tester->primary_icon_url().is_empty());
+  EXPECT_EQ(nullptr, tester->primary_icon());
   EXPECT_FALSE(tester->is_installable());
   EXPECT_EQ(NO_ICON_AVAILABLE, tester->error_code());
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                        CheckChangeInIconDimensions) {
-  // Verify that a follow-up request for an icon with a different size works.
+  // Verify that a follow-up request for a primary icon with a different size
+  // works.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     NavigateAndRunInstallableManager(tester.get(), GetWebAppParams(),
                                      "/banners/manifest_test_page.html");
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_TRUE(tester->is_installable());
-    EXPECT_FALSE(tester->icon_url().is_empty());
-    EXPECT_NE(nullptr, tester->icon());
+    EXPECT_FALSE(tester->primary_icon_url().is_empty());
+    EXPECT_NE(nullptr, tester->primary_icon());
     EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
   }
 
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
-    // Dial up the icon size requirements to something that isn't available.
-    // This should now fail with NoIconMatchingRequirements.
+    // Dial up the primary icon size requirements to something that isn't
+    // available. This should now fail with NoIconMatchingRequirements.
     InstallableParams params = GetWebAppParams();
     params.ideal_primary_icon_size_in_px = 2000;
     params.minimum_primary_icon_size_in_px = 2000;
     RunInstallableManager(tester.get(), params);
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_TRUE(tester->is_installable());
-    EXPECT_TRUE(tester->icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->icon());
+    EXPECT_TRUE(tester->primary_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->primary_icon());
     EXPECT_EQ(NO_ACCEPTABLE_ICON, tester->error_code());
   }
 
-  // Navigate and verify the reverse: an overly large icon requested first
-  // fails, but a smaller icon requested second passes.
+  // Navigate and verify the reverse: an overly large primary icon requested
+  // first fails, but a smaller primary icon requested second passes.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     // This should fail with NoIconMatchingRequirements.
     InstallableParams params = GetWebAppParams();
     params.ideal_primary_icon_size_in_px = 2000;
     params.minimum_primary_icon_size_in_px = 2000;
     NavigateAndRunInstallableManager(tester.get(), params,
                                      "/banners/manifest_test_page.html");
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_TRUE(tester->is_installable());
-    EXPECT_TRUE(tester->icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->icon());
+    EXPECT_TRUE(tester->primary_icon_url().is_empty());
+    EXPECT_EQ(nullptr, tester->primary_icon());
     EXPECT_EQ(NO_ACCEPTABLE_ICON, tester->error_code());
   }
 
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
     RunInstallableManager(tester.get(), GetWebAppParams());
 
     run_loop.Run();
 
-    // The smaller icon requirements should allow this to pass.
+    // The smaller primary icon requirements should allow this to pass.
     EXPECT_FALSE(tester->manifest_url().is_empty());
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_TRUE(tester->is_installable());
-    EXPECT_FALSE(tester->icon_url().is_empty());
-    EXPECT_NE(nullptr, tester->icon());
+    EXPECT_FALSE(tester->primary_icon_url().is_empty());
+    EXPECT_NE(nullptr, tester->primary_icon());
     EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
   }
 }
 
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                        CheckNestedCallsToGetData) {
   // Verify that we can call GetData while in a callback from GetData.
   base::RunLoop run_loop;
   InstallableParams params = GetWebAppParams();
   std::unique_ptr<NestedCallbackTester> tester(
       new NestedCallbackTester(GetManager(), params, run_loop.QuitClosure()));
 
   tester->Run();
   run_loop.Run();
 }