Fix in-document navigations breaking icons from Web Manifests

chrome/browser/favicon/content_favicon_driver_browsertest.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 "components/favicon/content/content_favicon_driver.h"
 
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
@@ skipping 161 matching lines @@
   content::WebContents* web_contents() {
     return browser()->tab_strip_model()->GetActiveWebContents();
   }
 
   // FaviconDriverPendingTaskChecker:
   bool HasPendingTasks() override {
     return favicon::ContentFaviconDriver::FromWebContents(web_contents())
         ->HasPendingTasksForTest();
   }
 
-  favicon_base::FaviconRawBitmapResult GetFaviconForPageURL(const GURL& url) {
+  favicon_base::FaviconRawBitmapResult GetFaviconForPageURL(
+      const GURL& url,
+      favicon_base::IconType icon_type) {
     favicon::FaviconService* favicon_service =
         FaviconServiceFactory::GetForProfile(
             browser()->profile(), ServiceAccessType::EXPLICIT_ACCESS);
 
     std::vector<favicon_base::FaviconRawBitmapResult> results;
     base::CancelableTaskTracker tracker;
     base::RunLoop loop;
     favicon_service->GetFaviconForPageURL(
-        url, favicon_base::FAVICON, /*desired_size_in_dip=*/0,
+        url, icon_type, /*desired_size_in_dip=*/0,
         base::Bind(
             [](std::vector<favicon_base::FaviconRawBitmapResult>* save_results,
                base::RunLoop* loop,
                const std::vector<favicon_base::FaviconRawBitmapResult>&
                    results) {
               *save_results = results;
               loop->Quit();
             },
             &results, &loop),
         &tracker);
@@ skipping 72 matching lines @@
   content::ResourceDispatcherHost::Get()->SetDelegate(delegate.get());
 
   PendingTaskWaiter waiter(web_contents(), this);
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), url, WindowOpenDisposition::CURRENT_TAB,
       ui_test_utils::BROWSER_TEST_NONE);
   waiter.Wait();
 
 #if defined(OS_ANDROID) || defined(OS_IOS)
   EXPECT_TRUE(delegate->was_requested());
+  EXPECT_NE(
+      nullptr,
+      GetFaviconForPageURL(url, favicon_base::WEB_MANIFEST_ICON).bitmap_data);
 #else
   EXPECT_FALSE(delegate->was_requested());
 #endif
 }
 
 // Test that loading a page that contains a Web Manifest without icons and a
 // regular favicon in the HTML reports the icon. The regular icon is initially
 // cached in the Favicon database.
 IN_PROC_BROWSER_TEST_F(ContentFaviconDriverTest,
                        LoadRegularIconDespiteWebManifestWithoutIcons) {
   ASSERT_TRUE(embedded_test_server()->Start());
   GURL url = embedded_test_server()->GetURL(
       "/favicon/page_with_manifest_without_icons.html");
   GURL icon_url = embedded_test_server()->GetURL("/favicon/icon.png");
 
   // Initial visit with the feature still disabled, to populate the cache.
   {
     PendingTaskWaiter waiter(web_contents(), this);
     ui_test_utils::NavigateToURLWithDisposition(
         browser(), url, WindowOpenDisposition::CURRENT_TAB,
         ui_test_utils::BROWSER_TEST_NONE);
     waiter.Wait();
   }
-  ASSERT_NE(nullptr, GetFaviconForPageURL(url).bitmap_data);
+  ASSERT_NE(nullptr,
+            GetFaviconForPageURL(url, favicon_base::FAVICON).bitmap_data);
 
   ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
 
   // Enable the feature and visit the page again.
   base::test::ScopedFeatureList override_features;
   override_features.InitAndEnableFeature(favicon::kFaviconsFromWebManifest);
 
   {
     PendingTaskWaiter waiter(web_contents(), this);
     ui_test_utils::NavigateToURLWithDisposition(
         browser(), url, WindowOpenDisposition::CURRENT_TAB,
         ui_test_utils::BROWSER_TEST_NONE);
     waiter.Wait();
   }
 
-  EXPECT_NE(nullptr, GetFaviconForPageURL(url).bitmap_data);
+  EXPECT_NE(nullptr,
+            GetFaviconForPageURL(url, favicon_base::FAVICON).bitmap_data);
 }
+
+#if defined(OS_ANDROID) || defined(OS_IOS)
+IN_PROC_BROWSER_TEST_F(ContentFaviconDriverTest,
+                       LoadIconFromWebManifestDespitePushState) {
+  base::test::ScopedFeatureList override_features;
+  override_features.InitAndEnableFeature(favicon::kFaviconsFromWebManifest);
+
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url =
+      embedded_test_server()->GetURL("/favicon/pushstate_with_manifest.html");
+  GURL pushstate_url = embedded_test_server()->GetURL(
+      "/favicon/pushstate_with_manifest.html#pushState");
+
+  PendingTaskWaiter waiter(web_contents(), this);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), url, WindowOpenDisposition::CURRENT_TAB,
+      ui_test_utils::BROWSER_TEST_NONE);

[pkotwicz, 2017/06/21 18:48:34]

You should probably wait till the JavaScript has executed. I know of two ways of
doing this (there are probably others)

Option #1: Make the JavaScript change the title of the page when it is done and
wait till the page title was changed. You could use content::TitleWatcher to
wait till the title is set

Option #2: Execute JavaScript via C++ via
RenderFrameHost::ExecuteJavaScriptForTests() and use TestNavigationObserver

+  waiter.Wait();
+
+  EXPECT_NE(nullptr,
+            GetFaviconForPageURL(pushstate_url, favicon_base::WEB_MANIFEST_ICON)
+                .bitmap_data);
+}
+#endif