Index: chrome/browser/favicon/content_favicon_driver_browsertest.cc |
diff --git a/chrome/browser/favicon/content_favicon_driver_browsertest.cc b/chrome/browser/favicon/content_favicon_driver_browsertest.cc |
index d786c92994dde549e9a51eb512326fdf5050b100..b0182bf8ad8b4ad536fd6447bafda618541b83d6 100644 |
--- a/chrome/browser/favicon/content_favicon_driver_browsertest.cc |
+++ b/chrome/browser/favicon/content_favicon_driver_browsertest.cc |
@@ -22,11 +22,9 @@ |
#include "components/favicon/core/favicon_handler.h" |
#include "components/favicon/core/favicon_service.h" |
#include "components/favicon/core/features.h" |
-#include "content/public/browser/notification_observer.h" |
-#include "content/public/browser/notification_registrar.h" |
-#include "content/public/browser/notification_types.h" |
#include "content/public/browser/resource_dispatcher_host.h" |
#include "content/public/browser/resource_dispatcher_host_delegate.h" |
+#include "content/public/browser/web_contents_observer.h" |
#include "net/base/load_flags.h" |
#include "net/test/embedded_test_server/embedded_test_server.h" |
#include "net/url_request/url_request.h" |
@@ -84,48 +82,34 @@ class TestResourceDispatcherHostDelegate |
DISALLOW_COPY_AND_ASSIGN(TestResourceDispatcherHostDelegate); |
}; |
-// Checks whether the FaviconDriver is waiting for a download to complete or |
-// for data from the FaviconService. |
-class FaviconDriverPendingTaskChecker { |
- public: |
- virtual ~FaviconDriverPendingTaskChecker() {} |
- |
- virtual bool HasPendingTasks() = 0; |
-}; |
- |
// Waits for the following the finish: |
// - The pending navigation. |
// - FaviconHandler's pending favicon database requests. |
// - FaviconHandler's pending downloads. |
-class PendingTaskWaiter : public content::NotificationObserver { |
+// - Optionally, for a specific page URL (as a mechanism to wait of Javascript |
+// completion). |
+class PendingTaskWaiter : public content::WebContentsObserver { |
public: |
PendingTaskWaiter(content::WebContents* web_contents, |
- FaviconDriverPendingTaskChecker* checker) |
- : checker_(checker), |
- load_stopped_(false), |
- weak_factory_(this) { |
- registrar_.Add(this, content::NOTIFICATION_LOAD_STOP, |
- content::Source<content::NavigationController>( |
- &web_contents->GetController())); |
- } |
+ const GURL& required_url = GURL()) |
+ : WebContentsObserver(web_contents), |
+ required_url_(required_url), |
+ weak_factory_(this) {} |
~PendingTaskWaiter() override {} |
void Wait() { |
- if (load_stopped_ && !checker_->HasPendingTasks()) |
- return; |
- |
base::RunLoop run_loop; |
quit_closure_ = run_loop.QuitClosure(); |
run_loop.Run(); |
} |
private: |
- // content::NotificationObserver: |
- void Observe(int type, |
- const content::NotificationSource& source, |
- const content::NotificationDetails& details) override { |
- if (type == content::NOTIFICATION_LOAD_STOP) |
- load_stopped_ = true; |
+ // content::WebContentsObserver: |
+ void DidStopLoading() override { |
+ if (!required_url_.is_empty() && |
+ required_url_ != web_contents()->GetLastCommittedURL()) { |
+ return; |
+ } |
// We need to poll periodically because Delegate::OnFaviconUpdated() is not |
// guaranteed to be called upon completion of the last database request / |
@@ -146,16 +130,14 @@ class PendingTaskWaiter : public content::NotificationObserver { |
void EndLoopIfCanStopWaiting() { |
if (!quit_closure_.is_null() && |
- load_stopped_ && |
- !checker_->HasPendingTasks()) { |
+ !favicon::ContentFaviconDriver::FromWebContents(web_contents()) |
+ ->HasPendingTasksForTest()) { |
quit_closure_.Run(); |
} |
} |
- FaviconDriverPendingTaskChecker* checker_; // Not owned. |
- bool load_stopped_; |
base::Closure quit_closure_; |
- content::NotificationRegistrar registrar_; |
+ const GURL required_url_; |
base::WeakPtrFactory<PendingTaskWaiter> weak_factory_; |
DISALLOW_COPY_AND_ASSIGN(PendingTaskWaiter); |
@@ -163,8 +145,7 @@ class PendingTaskWaiter : public content::NotificationObserver { |
} // namespace |
-class ContentFaviconDriverTest : public InProcessBrowserTest, |
- public FaviconDriverPendingTaskChecker { |
+class ContentFaviconDriverTest : public InProcessBrowserTest { |
public: |
ContentFaviconDriverTest() {} |
~ContentFaviconDriverTest() override {} |
@@ -173,13 +154,9 @@ class ContentFaviconDriverTest : public InProcessBrowserTest, |
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); |
@@ -188,7 +165,7 @@ class ContentFaviconDriverTest : public InProcessBrowserTest, |
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, |
@@ -225,7 +202,7 @@ IN_PROC_BROWSER_TEST_F(ContentFaviconDriverTest, ReloadBypassingCache) { |
// Initial visit in order to populate the cache. |
{ |
- PendingTaskWaiter waiter(web_contents(), this); |
+ PendingTaskWaiter waiter(web_contents()); |
ui_test_utils::NavigateToURLWithDisposition( |
browser(), url, WindowOpenDisposition::CURRENT_TAB, |
ui_test_utils::BROWSER_TEST_NONE); |
@@ -240,7 +217,7 @@ IN_PROC_BROWSER_TEST_F(ContentFaviconDriverTest, ReloadBypassingCache) { |
// A normal visit should fetch the favicon from either the favicon database or |
// the HTTP cache. |
{ |
- PendingTaskWaiter waiter(web_contents(), this); |
+ PendingTaskWaiter waiter(web_contents()); |
ui_test_utils::NavigateToURLWithDisposition( |
browser(), url, WindowOpenDisposition::CURRENT_TAB, |
ui_test_utils::BROWSER_TEST_NONE); |
@@ -251,7 +228,7 @@ IN_PROC_BROWSER_TEST_F(ContentFaviconDriverTest, ReloadBypassingCache) { |
// A reload ignoring the cache should refetch the favicon from the website. |
{ |
- PendingTaskWaiter waiter(web_contents(), this); |
+ PendingTaskWaiter waiter(web_contents()); |
chrome::ExecuteCommand(browser(), IDC_RELOAD_BYPASSING_CACHE); |
waiter.Wait(); |
} |
@@ -270,7 +247,7 @@ IN_PROC_BROWSER_TEST_F(ContentFaviconDriverTest, LoadIconFromWebManifest) { |
new TestResourceDispatcherHostDelegate(icon_url)); |
content::ResourceDispatcherHost::Get()->SetDelegate(delegate.get()); |
- PendingTaskWaiter waiter(web_contents(), this); |
+ PendingTaskWaiter waiter(web_contents()); |
ui_test_utils::NavigateToURLWithDisposition( |
browser(), url, WindowOpenDisposition::CURRENT_TAB, |
ui_test_utils::BROWSER_TEST_NONE); |
@@ -278,6 +255,9 @@ IN_PROC_BROWSER_TEST_F(ContentFaviconDriverTest, LoadIconFromWebManifest) { |
#if defined(OS_ANDROID) |
EXPECT_TRUE(delegate->was_requested()); |
+ EXPECT_NE( |
+ nullptr, |
+ GetFaviconForPageURL(url, favicon_base::WEB_MANIFEST_ICON).bitmap_data); |
#else |
EXPECT_FALSE(delegate->was_requested()); |
#endif |
@@ -298,24 +278,50 @@ IN_PROC_BROWSER_TEST_F(ContentFaviconDriverTest, |
base::test::ScopedFeatureList override_features; |
override_features.InitAndDisableFeature(favicon::kFaviconsFromWebManifest); |
- PendingTaskWaiter waiter(web_contents(), this); |
+ PendingTaskWaiter waiter(web_contents()); |
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)); |
// Visit the page again now that the feature is enabled (default). |
{ |
- PendingTaskWaiter waiter(web_contents(), this); |
+ PendingTaskWaiter waiter(web_contents()); |
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) |
+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(), /*required_url=*/pushstate_url); |
+ ui_test_utils::NavigateToURLWithDisposition( |
+ browser(), url, WindowOpenDisposition::CURRENT_TAB, |
+ ui_test_utils::BROWSER_TEST_NONE); |
+ waiter.Wait(); |
+ |
+ EXPECT_NE(nullptr, |
+ GetFaviconForPageURL(pushstate_url, favicon_base::WEB_MANIFEST_ICON) |
+ .bitmap_data); |
} |
+#endif |