Add support to process favicons from Web Manifests

components/favicon/core/favicon_handler_unittest.cc

Index: components/favicon/core/favicon_handler_unittest.cc
diff --git a/components/favicon/core/favicon_handler_unittest.cc b/components/favicon/core/favicon_handler_unittest.cc
index 4ff91269be35d7c93368afa3522ec5163bf3b088..f10ea262a7d809e9718fbcf15c5aefb5aa37a030 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -16,6 +16,7 @@
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/test/test_simple_task_runner.h"
 #include "components/favicon/core/favicon_driver.h"
@@ -37,10 +38,12 @@ using favicon_base::FaviconRawBitmapResult;
 using favicon_base::TOUCH_ICON;
 using favicon_base::TOUCH_PRECOMPOSED_ICON;
 using testing::Assign;
+using testing::Contains;
 using testing::ElementsAre;
 using testing::InSequence;
 using testing::Invoke;
 using testing::IsEmpty;
+using testing::Not;
 using testing::Return;
 using testing::_;
 
@@ -117,14 +120,16 @@ class FakeImageDownloader {
     SizeVector original_bitmap_sizes;
   };
 
-  FakeImageDownloader() : next_download_id_(1) {}
+  // |downloads| must not be nullptr and must outlive this object.
+  FakeImageDownloader(URLVector* downloads)
+      : downloads_(downloads), next_download_id_(1) {}
 
   // Implementation of FaviconHalder::Delegate's DownloadImage(). If a given
   // URL is not known (i.e. not previously added via Add()), it produces 404s.
   int DownloadImage(const GURL& url,
                     int max_image_size,
                     FaviconHandler::Delegate::ImageDownloadCallback callback) {
-    downloads_.push_back(url);
+    downloads_->push_back(url);
 
     const Response& response = responses_[url];
     int download_id = next_download_id_++;
@@ -184,17 +189,10 @@ class FakeImageDownloader {
     return true;
   }
 
-  // Returns pending and completed download URLs.
-  const URLVector& downloads() const { return downloads_; }
-
-  void ClearDownloads() { downloads_.clear(); }
-
  private:
+  URLVector* downloads_;
   int next_download_id_;
 
-  // Pending and completed download URLs.
-  URLVector downloads_;
-
   // URL to disable automatic callbacks for.
   GURL manual_callback_url_;
 
@@ -207,19 +205,105 @@ class FakeImageDownloader {
   DISALLOW_COPY_AND_ASSIGN(FakeImageDownloader);
 };
 
+// Fake that implements the calls to FaviconHandler::Delegate's
+// DownloadManifest(), delegated to this class through MockDelegate.
+class FakeManifestDownloader {
+ public:
+  struct Response {
+    int http_status_code = 404;
+    std::vector<favicon::FaviconURL> favicon_urls;
+  };
+
+  // |downloads| must not be nullptr and must outlive this object.
+  FakeManifestDownloader(URLVector* downloads) : downloads_(downloads) {}
+
+  // Implementation of FaviconHalder::Delegate's DownloadManifest(). If a given
+  // URL is not known (i.e. not previously added via Add()), it produces 404s.
+  void DownloadManifest(
+      const GURL& url,
+      FaviconHandler::Delegate::ManifestDownloadCallback callback) {
+    downloads_->push_back(url);
+
+    const Response& response = responses_[url];
+    base::Closure bound_callback =
+        base::Bind(callback, response.http_status_code, response.favicon_urls);
+    if (url == manual_callback_url_)
+      manual_callback_ = bound_callback;
+    else
+      base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, bound_callback);
+  }
+
+  void Add(const GURL& manifest_url,
+           const std::vector<favicon::FaviconURL>& favicon_urls) {
+    Response response;
+    response.http_status_code = 200;
+    response.favicon_urls = favicon_urls;
+    responses_[manifest_url] = response;
+  }
+
+  void AddError(const GURL& manifest_url, int http_status_code) {
+    Response response;
+    response.http_status_code = http_status_code;
+    responses_[manifest_url] = response;
+  }
+
+  // Disables automatic callback for |url|. This is useful for emulating a
+  // download taking a long time. The callback for DownloadManifest() will be
+  // stored in |manual_callback_|.
+  void SetRunCallbackManuallyForUrl(const GURL& url) {
+    manual_callback_url_ = url;
+  }
+
+  // Returns whether an ongoing download exists for a url previously selected
+  // via SetRunCallbackManuallyForUrl().
+  bool HasPendingManualCallback() { return !manual_callback_.is_null(); }
+
+  // Triggers the response for a download previously selected for manual
+  // triggering via SetRunCallbackManuallyForUrl().
+  bool RunCallbackManually() {
+    if (!HasPendingManualCallback())
+      return false;
+    manual_callback_.Run();
+    manual_callback_.Reset();
+    return true;
+  }
+
+ private:
+  URLVector* downloads_;
+
+  // URL to disable automatic callbacks for.
+  GURL manual_callback_url_;
+
+  // Callback for DownloadManifest() request for |manual_callback_url_|.
+  base::Closure manual_callback_;
+
+  // Registered responses.
+  std::map<GURL, Response> responses_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeManifestDownloader);
+};
+
 class MockDelegate : public FaviconHandler::Delegate {
  public:
-  MockDelegate() {
+  MockDelegate()
+      : fake_image_downloader_(&downloads_),
+        fake_manifest_downloader_(&downloads_) {
     // Delegate image downloading to FakeImageDownloader.
     ON_CALL(*this, DownloadImage(_, _, _))
         .WillByDefault(Invoke(&fake_image_downloader_,
                               &FakeImageDownloader::DownloadImage));
+    // Delegate manifest downloading to FakeManifestDownloader.
+    ON_CALL(*this, DownloadManifest(_, _))
+        .WillByDefault(Invoke(&fake_manifest_downloader_,
+                              &FakeManifestDownloader::DownloadManifest));
   }
 
   MOCK_METHOD3(DownloadImage,
                int(const GURL& url,
                    int max_image_size,
                    ImageDownloadCallback callback));
+  MOCK_METHOD2(DownloadManifest,
+               void(const GURL& url, ManifestDownloadCallback callback));
   MOCK_METHOD0(IsOffTheRecord, bool());
   MOCK_METHOD1(IsBookmarked, bool(const GURL& url));
   MOCK_METHOD5(OnFaviconUpdated,
@@ -233,14 +317,20 @@ class MockDelegate : public FaviconHandler::Delegate {
     return fake_image_downloader_;
   }
 
-  // Convenience getter for test readability. Returns pending and completed
-  // download URLs.
-  const URLVector& downloads() const {
-    return fake_image_downloader_.downloads();
+  FakeManifestDownloader& fake_manifest_downloader() {
+    return fake_manifest_downloader_;
   }
 
+  // Returns pending and completed download URLs.
+  const URLVector& downloads() const { return downloads_; }
+
+  void ClearDownloads() { downloads_.clear(); }
+
  private:
+  // Pending and completed download URLs.
+  URLVector downloads_;
   FakeImageDownloader fake_image_downloader_;
+  FakeManifestDownloader fake_manifest_downloader_;
 };
 
 // FakeFaviconService mimics a FaviconService backend that allows setting up
@@ -404,7 +494,7 @@ class FaviconHandlerTest : public testing::Test {
   bool VerifyAndClearExpectations() {
     base::RunLoop().RunUntilIdle();
     favicon_service_.fake()->ClearDbRequests();
-    delegate_.fake_image_downloader().ClearDownloads();
+    delegate_.ClearDownloads();
     return testing::Mock::VerifyAndClearExpectations(&favicon_service_) &&
            testing::Mock::VerifyAndClearExpectations(&delegate_);
   }
@@ -413,14 +503,15 @@ class FaviconHandlerTest : public testing::Test {
   // Returns the handler in case tests want to exercise further steps.
   std::unique_ptr<FaviconHandler> RunHandlerWithCandidates(
       FaviconDriverObserver::NotificationIconType handler_type,
-      const std::vector<favicon::FaviconURL>& candidates) {
+      const std::vector<favicon::FaviconURL>& candidates,
+      const base::Optional<GURL>& manifest_url = base::nullopt) {
     auto handler = base::MakeUnique<FaviconHandler>(&favicon_service_,
                                                     &delegate_, handler_type);
     handler->FetchFavicon(kPageURL);
     // The first RunUntilIdle() causes the FaviconService lookups be faster than
     // OnUpdateCandidates(), which is the most likely scenario.
     base::RunLoop().RunUntilIdle();
-    handler->OnUpdateCandidates(kPageURL, candidates);
+    handler->OnUpdateCandidates(kPageURL, candidates, manifest_url);
     base::RunLoop().RunUntilIdle();
     return handler;
   }
@@ -428,13 +519,14 @@ class FaviconHandlerTest : public testing::Test {
   // Same as above, but for the simplest case where all types are FAVICON and
   // no sizes are provided, using a FaviconHandler of type NON_TOUCH_16_DIP.
   std::unique_ptr<FaviconHandler> RunHandlerWithSimpleFaviconCandidates(
-      const std::vector<GURL>& urls) {
+      const std::vector<GURL>& urls,
+      const base::Optional<GURL>& manifest_url = base::nullopt) {
     std::vector<favicon::FaviconURL> candidates;
     for (const GURL& url : urls) {
       candidates.emplace_back(url, FAVICON, kEmptySizes);
     }
     return RunHandlerWithCandidates(FaviconDriverObserver::NON_TOUCH_16_DIP,
-                                    candidates);
+                                    candidates, manifest_url);
   }
 
   base::test::ScopedTaskEnvironment scoped_task_environment_;
@@ -504,7 +596,8 @@ TEST_F(FaviconHandlerTest, DownloadUnknownFaviconIfCandidatesSlower) {
                              kIconURL16x16, /*icon_url_changed=*/true, _));
   // Feed in favicons now that the database lookup is completed.
   handler.OnUpdateCandidates(kPageURL,
-                             {FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)});
+                             {FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)},
+                             base::nullopt);
   base::RunLoop().RunUntilIdle();
 
   EXPECT_THAT(favicon_service_.fake()->db_requests(),
@@ -530,7 +623,8 @@ TEST_F(FaviconHandlerTest, DownloadUnknownFaviconIfCandidatesFaster) {
   base::RunLoop().RunUntilIdle();
   // Feed in favicons before completing the database lookup.
   handler.OnUpdateCandidates(kPageURL,
-                             {FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)});
+                             {FaviconURL(kIconURL16x16, FAVICON, kEmptySizes)},
+                             base::nullopt);
 
   ASSERT_TRUE(VerifyAndClearExpectations());
   // Database lookup for |kPageURL| is ongoing.
@@ -744,8 +838,8 @@ TEST_F(FaviconHandlerTest, UpdateDuringDownloading) {
   EXPECT_CALL(favicon_service_, SetFavicons(_, kIconURL3, _, _));
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL3, _, _));
 
-  handler->OnUpdateCandidates(kPageURL,
-                              {FaviconURL(kIconURL3, FAVICON, kEmptySizes)});
+  handler->OnUpdateCandidates(
+      kPageURL, {FaviconURL(kIconURL3, FAVICON, kEmptySizes)}, base::nullopt);
 
   // Finalizes download, which should be thrown away as the favicon URLs were
   // updated.
@@ -781,8 +875,8 @@ TEST_F(FaviconHandlerTest, UpdateDuringDatabaseLookup) {
   EXPECT_CALL(favicon_service_, SetFavicons(_, kIconURL2, _, _));
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL2, _, _));
 
-  handler->OnUpdateCandidates(kPageURL,
-                              {FaviconURL(kIconURL2, FAVICON, kEmptySizes)});
+  handler->OnUpdateCandidates(
+      kPageURL, {FaviconURL(kIconURL2, FAVICON, kEmptySizes)}, base::nullopt);
 
   // Finalizes the DB lookup, which should be thrown away as the favicon URLs
   // were updated.
@@ -819,7 +913,7 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsWhileDownloadingShouldBeNoop) {
 
   // Calling OnUpdateCandidates() with the same icon URLs should have no effect,
   // despite the ongoing download.
-  handler->OnUpdateCandidates(kPageURL, favicon_urls);
+  handler->OnUpdateCandidates(kPageURL, favicon_urls, base::nullopt);
   base::RunLoop().RunUntilIdle();
   EXPECT_THAT(favicon_service_.fake()->db_requests(), IsEmpty());
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
@@ -853,7 +947,7 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsWhileDatabaseLookupShouldBeNoop) {
 
   // Calling OnUpdateCandidates() with the same icon URLs should have no effect,
   // despite the ongoing DB lookup.
-  handler->OnUpdateCandidates(kPageURL, favicon_urls);
+  handler->OnUpdateCandidates(kPageURL, favicon_urls, base::nullopt);
   base::RunLoop().RunUntilIdle();
   EXPECT_THAT(favicon_service_.fake()->db_requests(), IsEmpty());
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
@@ -884,7 +978,7 @@ TEST_F(FaviconHandlerTest, UpdateSameIconURLsAfterFinishedShouldBeNoop) {
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, _, _, _)).Times(0);
   EXPECT_CALL(favicon_service_, SetFavicons(_, _, _, _)).Times(0);
 
-  handler->OnUpdateCandidates(kPageURL, favicon_urls);
+  handler->OnUpdateCandidates(kPageURL, favicon_urls, base::nullopt);
   base::RunLoop().RunUntilIdle();
   EXPECT_THAT(favicon_service_.fake()->db_requests(), IsEmpty());
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
@@ -926,8 +1020,8 @@ TEST_F(FaviconHandlerTest,
   // Simulate the page changing it's icon URL to just |kIconURL2| via
   // Javascript.
   EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL2, _, _));
-  handler->OnUpdateCandidates(kPageURL,
-                              {FaviconURL(kIconURL2, FAVICON, kEmptySizes)});
+  handler->OnUpdateCandidates(
+      kPageURL, {FaviconURL(kIconURL2, FAVICON, kEmptySizes)}, base::nullopt);
   base::RunLoop().RunUntilIdle();
 }
 
@@ -1138,7 +1232,6 @@ TEST_F(FaviconHandlerTest, TestDownloadLargestFavicon) {
                   {gfx::Size(16, 16), gfx::Size(512, 512)}),
        FaviconURL(kIconURLWithoutSize1, FAVICON, kEmptySizes),
        FaviconURL(kIconURLWithoutSize2, FAVICON, kEmptySizes)});
-
   // Icon URLs are not registered and hence 404s will be produced, which
   // allows checking whether the icons were requested according to their size.
   // The favicons should have been requested in decreasing order of their sizes.
@@ -1370,5 +1463,394 @@ TEST_F(FaviconHandlerTest, TestRecordSkippedDownloadForKnownFailingUrl) {
                                /*expected_count=*/1)));
 }
 
+// Test that the support for Web Manifest is disabled by default, unless the
+// feature is enabled.
+TEST_F(FaviconHandlerTest, IgnoreWebManifestByDefault) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL16x16}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              Not(Contains(kManifestURL)));
+  EXPECT_THAT(delegate_.downloads(), Not(Contains(kManifestURL)));
+}
+
+class FaviconHandlerManifestsEnabledTest : public FaviconHandlerTest {
+ protected:
+  FaviconHandlerManifestsEnabledTest() {
+    override_features_.InitAndEnableFeature(kFaviconsFromWebManifest);
+  }
+
+ private:
+  base::test::ScopedFeatureList override_features_;
+};
+
+// Test that a favicon corresponding to a web manifest is reported when:
+// - There is data in the favicon database for the manifest URL.
+// AND
+// - FaviconService::OnFaviconDataForManifestFromFaviconService() runs before
+//   FaviconHandler::OnUpdateCandidates() is called.
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       GetFaviconFromManifestInHistoryIfCandidatesSlower) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+
+  favicon_service_.fake()->Store(kPageURL, kManifestURL,
+                                 CreateRawBitmapResult(kManifestURL));
+
+  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).Times(0);
+
+  EXPECT_CALL(favicon_service_,
+              UpdateFaviconMappingsAndFetch(_, kManifestURL, FAVICON,
+                                            /*desired_size_in_dip=*/16, _, _));
+  EXPECT_CALL(delegate_,
+              OnFaviconUpdated(_, FaviconDriverObserver::NON_TOUCH_16_DIP,
+                               kManifestURL, _, _))
+      .Times(2);
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL));
+  EXPECT_THAT(delegate_.downloads(), IsEmpty());
+}
+
+// Test that a favicon corresponding to a web manifest is reported when:
+// - There is data in the favicon database for the manifest URL.
+// AND
+// - FaviconHandler::OnUpdateCandidates() is called before
+//   FaviconService::OnFaviconDataForManifestFromFaviconService() runs.
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       GetFaviconFromManifestInHistoryIfCandidatesFaster) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+
+  favicon_service_.fake()->Store(kPageURL, kManifestURL,
+                                 CreateRawBitmapResult(kManifestURL));
+
+  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).Times(0);
+
+  EXPECT_CALL(favicon_service_,
+              UpdateFaviconMappingsAndFetch(_, kManifestURL, FAVICON,
+                                            /*desired_size_in_dip=*/16, _, _));
+  EXPECT_CALL(delegate_,
+              OnFaviconUpdated(_, FaviconDriverObserver::NON_TOUCH_16_DIP,
+                               kManifestURL, _, _))
+      .Times(2);
+
+  FaviconHandler handler(&favicon_service_, &delegate_,
+                         FaviconDriverObserver::NON_TOUCH_16_DIP);
+  handler.FetchFavicon(kPageURL);
+  // Feed in candidates without processing posted tasks (RunUntilIdle()).
+  handler.OnUpdateCandidates(kPageURL,
+                             {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)},
+                             kManifestURL);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL));
+  EXPECT_THAT(delegate_.downloads(), IsEmpty());
+}
+
+// Test that a favicon corresponding to a web manifest is reported when there is
+// data in the database for neither the page URL nor the manifest URL.
+TEST_F(FaviconHandlerManifestsEnabledTest, GetFaviconFromUnknownManifest) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+  const std::vector<favicon::FaviconURL> kManifestIcons = {
+      FaviconURL(kIconURL16x16, FAVICON, kEmptySizes),
+  };
+
+  delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons);
+
+  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).Times(0);
+
+  EXPECT_CALL(favicon_service_, SetFavicons(_, kManifestURL, FAVICON, _));
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL, _, _));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL16x16));
+}
+
+// Test that the manifest and icon are redownloaded if the icon cached for the
+// page URL expired.
+TEST_F(FaviconHandlerManifestsEnabledTest, GetFaviconFromExpiredManifest) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+  const std::vector<favicon::FaviconURL> kManifestIcons = {
+      FaviconURL(kIconURL64x64, FAVICON, kEmptySizes),
+  };
+
+  favicon_service_.fake()->Store(kPageURL, kManifestURL,
+                                 CreateRawBitmapResult(kManifestURL, FAVICON,
+                                                       /*expired=*/true));
+  delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons);
+
+  // The third notification is unnecessary, but allows simplifying the code.
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL, _, _)).Times(3);
+  EXPECT_CALL(favicon_service_, SetFavicons(_, kManifestURL, _, _));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL64x64));
+}
+
+// Test that the manifest and icon are redownloaded if the icon cached for the
+// manifest URL expired, which was observed during a visit to a different page
+// URL.
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       GetFaviconFromExpiredManifestLinkedFromOtherPage) {
+  const GURL kSomePreviousPageURL("https://www.google.com/previous");
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+  const std::vector<favicon::FaviconURL> kManifestIcons = {
+      FaviconURL(kIconURL64x64, FAVICON, kEmptySizes),
+  };
+
+  favicon_service_.fake()->Store(kSomePreviousPageURL, kManifestURL,
+                                 CreateRawBitmapResult(kManifestURL, FAVICON,
+                                                       /*expired=*/true));
+  delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons);
+
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL, _, _)).Times(2);
+  EXPECT_CALL(favicon_service_, SetFavicons(_, kManifestURL, _, _));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL64x64));
+}
+
+// Test that a favicon corresponding to a web manifest is reported when:
+// - There is data in the database for neither the page URL nor the manifest
+//   URL.
+// - There is data in the database for the icon URL listed in the manifest.
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       GetFaviconFromUnknownManifestButKnownIcon) {
+  const GURL kSomePreviousPageURL("https://www.google.com/previous");
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+  const std::vector<favicon::FaviconURL> kManifestIcons = {
+      FaviconURL(kIconURL16x16, FAVICON, kEmptySizes),
+  };
+
+  favicon_service_.fake()->Store(kSomePreviousPageURL, kIconURL16x16,
+                                 CreateRawBitmapResult(kIconURL16x16));
+  delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons);
+
+  EXPECT_CALL(favicon_service_, SetFavicons(_, kManifestURL, FAVICON, _));
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL, _, _));
+
+  RunHandlerWithSimpleFaviconCandidates(URLVector(), kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL));
+  // In the current implementation, the icon is downloaded although it's in the
+  // database. This is because the icon has been cached earlier using the icon
+  // URL instead of the manifest's URL.
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL16x16));
+}
+
+// Test that attempting to download a manifest that returns a 404 gets
+// blacklisted via UnableToDownloadFavicon() AND that the regular favicon is
+// selected as fallback.
+TEST_F(FaviconHandlerManifestsEnabledTest, UnknownManifestReturning404) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+
+  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kManifestURL));
+  EXPECT_CALL(favicon_service_, SetFavicons(_, kIconURL12x12, FAVICON, _));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL, kIconURL12x12));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL12x12));
+}
+
+// Test that attempting to download a manifest that returns a 503 does NOT get
+// blacklisted via UnableToDownloadFavicon() AND that the regular favicon is
+// selected as fallback.
+TEST_F(FaviconHandlerManifestsEnabledTest, UnknownManifestReturning503) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+
+  delegate_.fake_manifest_downloader().AddError(kManifestURL, 503);
+
+  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(_)).Times(0);
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+}
+
+// Test that a manifest that was previously blacklisted via
+// UnableToDownloadFavicon() is ignored and that the regular favicon is selected
+// as fallback.
+TEST_F(FaviconHandlerManifestsEnabledTest, IgnoreManifestWithPrior404) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+
+  ON_CALL(favicon_service_, WasUnableToDownloadFavicon(kManifestURL))
+      .WillByDefault(Return(true));
+
+  EXPECT_CALL(favicon_service_, SetFavicons(_, kIconURL12x12, FAVICON, _));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kIconURL12x12));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL12x12));
+}
+
+// Test that the regular favicon is selected when:
+// - The page links to a Web Manifest.
+// - The Web Manifest does not contain any icon URLs (it is not a 404).
+// - The page has an icon URL provided via a <link rel="icon"> tag.
+// - The database does not know about the page URL, manifest URL or icon URL.
+TEST_F(FaviconHandlerManifestsEnabledTest, UnknownManifestWithoutIcons) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+
+  delegate_.fake_manifest_downloader().Add(kManifestURL,
+                                           std::vector<favicon::FaviconURL>());
+
+  // UnableToDownloadFavicon() is expected to prevent repeated downloads of the
+  // same manifest (which is not otherwise cached, since it doesn't contain
+  // icons).
+  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kManifestURL));
+  EXPECT_CALL(favicon_service_, SetFavicons(_, kIconURL12x12, FAVICON, _));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL, kIconURL12x12));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL12x12));
+}
+
+// Test that the regular favicon is selected when:
+// - The page links to a Web Manifest.
+// - The Web Manifest does not contain any icon URLs (it is not a 404).
+// - The page has an icon URL provided via a <link rel="icon"> tag.
+// - The database does not know about the page URL.
+// - The database does not know about the manifest URL.
+// - The database knows about the icon URL.
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       UnknownManifestWithoutIconsAndKnownRegularIcons) {
+  const GURL kSomePreviousPageURL("https://www.google.com/previous");
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+
+  delegate_.fake_manifest_downloader().Add(kManifestURL,
+                                           std::vector<favicon::FaviconURL>());
+  favicon_service_.fake()->Store(kSomePreviousPageURL, kIconURL12x12,
+                                 CreateRawBitmapResult(kIconURL12x12));
+
+  EXPECT_CALL(favicon_service_, SetFavicons(_, _, _, _)).Times(0);
+
+  // UnableToDownloadFavicon() is expected to prevent repeated downloads of the
+  // same manifest (which is not otherwise cached, since it doesn't contain
+  // icons).
+  EXPECT_CALL(favicon_service_, UnableToDownloadFavicon(kManifestURL));
+  EXPECT_CALL(favicon_service_,
+              UpdateFaviconMappingsAndFetch(_, kManifestURL, _, _, _, _));
+  EXPECT_CALL(favicon_service_,
+              UpdateFaviconMappingsAndFetch(_, kIconURL12x12, _, _, _, _));
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL12x12, _, _));
+
+  RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  EXPECT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL, kIconURL12x12));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL));
+}
+
+// Test that Delegate::OnFaviconUpdated() is called if a page uses Javascript to
+// modify the page's <link rel="manifest"> tag to point to a different manifest.
+TEST_F(FaviconHandlerManifestsEnabledTest, ManifestUpdateViaJavascript) {
+  const GURL kManifestURL1("http://www.google.com/manifest1.json");
+  const GURL kManifestURL2("http://www.google.com/manifest2.json");
+  const std::vector<favicon::FaviconURL> kManifestIcons1 = {
+      FaviconURL(kIconURL64x64, FAVICON, kEmptySizes),
+  };
+  const std::vector<favicon::FaviconURL> kManifestIcons2 = {
+      FaviconURL(kIconURL10x10, FAVICON, kEmptySizes),
+  };
+
+  delegate_.fake_manifest_downloader().Add(kManifestURL1, kManifestIcons1);
+  delegate_.fake_manifest_downloader().Add(kManifestURL2, kManifestIcons2);
+
+  std::unique_ptr<FaviconHandler> handler =
+      RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL1);
+  ASSERT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL1));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL1, kIconURL64x64));
+  ASSERT_TRUE(VerifyAndClearExpectations());
+
+  // Simulate the page changing it's manifest URL via Javascript.
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL2, _, _));
+  handler->OnUpdateCandidates(kPageURL,
+                              {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)},
+                              kManifestURL2);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kManifestURL2));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL2, kIconURL10x10));
+}
+
+// Test that Delegate::OnFaviconUpdated() is called if a page uses Javascript to
+// remove the page's <link rel="manifest"> tag (i.e. no web manifest) WHILE a
+// lookup to the history database is ongoing for the manifest URL.
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       RemoveManifestViaJavascriptWhileHistoryQuery) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+  const std::vector<favicon::FaviconURL> kManifestIcons = {
+      FaviconURL(kIconURL64x64, FAVICON, kEmptySizes),
+  };
+
+  delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons);
+  // Defer the database lookup completion such that RunUntilIdle() doesn't
+  // complete it.
+  favicon_service_.fake()->SetRunCallbackManuallyForUrl(kManifestURL);
+
+  std::unique_ptr<FaviconHandler> handler =
+      RunHandlerWithSimpleFaviconCandidates({kIconURL12x12}, kManifestURL);
+  ASSERT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL));
+  // Database lookup for |kManifestURL1| is ongoing.
+  ASSERT_TRUE(favicon_service_.fake()->HasPendingManualCallback());
+
+  // Simulate the page changing it's manifest URL to empty via Javascript.
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kIconURL12x12, _, _));
+  handler->OnUpdateCandidates(kPageURL,
+                              {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)},
+                              base::nullopt);
+  // Complete the lookup.
+  EXPECT_TRUE(favicon_service_.fake()->RunCallbackManually());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kManifestURL, kIconURL12x12));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL12x12));
+}
+
+// Test that Delegate::OnFaviconUpdated() is called a page without manifest uses
+// Javascript to add a <link rel="manifest"> tag (i.e. a new web manifest) WHILE
+// a lookup to the history database is ongoing for the icon URL.
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       AddManifestViaJavascriptWhileHistoryQuery) {
+  const GURL kManifestURL("http://www.google.com/manifest.json");
+  const std::vector<favicon::FaviconURL> kManifestIcons = {
+      FaviconURL(kIconURL64x64, FAVICON, kEmptySizes),
+  };
+
+  delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons);
+  // Defer the database lookup completion such that RunUntilIdle() doesn't
+  // complete it.
+  favicon_service_.fake()->SetRunCallbackManuallyForUrl(kIconURL12x12);
+
+  std::unique_ptr<FaviconHandler> handler =
+      RunHandlerWithSimpleFaviconCandidates({kIconURL12x12});
+  ASSERT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kIconURL12x12));
+  // Database lookup for |kIconURL12x12| is ongoing.
+  ASSERT_TRUE(favicon_service_.fake()->HasPendingManualCallback());
+
+  // Simulate the page changing it's manifest URL to |kManifestURL| via
+  // Javascript.
+  EXPECT_CALL(delegate_, OnFaviconUpdated(_, _, kManifestURL, _, _));
+  handler->OnUpdateCandidates(kPageURL,
+                              {FaviconURL(kIconURL12x12, FAVICON, kEmptySizes)},
+                              kManifestURL);
+  // Complete the lookup.
+  EXPECT_TRUE(favicon_service_.fake()->RunCallbackManually());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_THAT(favicon_service_.fake()->db_requests(),
+              ElementsAre(kPageURL, kIconURL12x12, kManifestURL));
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL64x64));
+}
+
 }  // namespace
 }  // namespace favicon