Commit ab48fc09 authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Don't clear the page's cached favicon URL when iframe navigations start

https://chromium-review.googlesource.com/c/chromium/src/+/1959334
upstreamed the favicon caching functionality of ContentFaviconDriver
into WebContentsImpl. One thing that it missed was making clearing
the favicon cache on navigation start conditional that the main frame
was being navigated.

This CL adds that conditional and fixes a bug where delayed iframe
loads would prevent "Create shortcut..." from working as it would wait
indefinitely for a favicon URL event when it saw there was none.

Bug: 1046883
Change-Id: I520531558dec2d2daa8817884a33701b75f017bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2068266Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743907}
parent ef383ff4
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "chrome/browser/web_applications/components/web_app_id.h" #include "chrome/browser/web_applications/components/web_app_id.h"
#include "chrome/browser/web_applications/components/web_app_provider_base.h" #include "chrome/browser/web_applications/components/web_app_provider_base.h"
#include "chrome/browser/web_applications/test/web_app_install_observer.h" #include "chrome/browser/web_applications/test/web_app_install_observer.h"
#include "content/public/test/browser_test_utils.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace web_app { namespace web_app {
...@@ -126,6 +127,28 @@ IN_PROC_BROWSER_TEST_P(CreateShortcutBrowserTest, ...@@ -126,6 +127,28 @@ IN_PROC_BROWSER_TEST_P(CreateShortcutBrowserTest,
NavigateAndCheckForToolbar(app_browser, popup_url, false); NavigateAndCheckForToolbar(app_browser, popup_url, false);
} }
// Tests that Create Shortcut doesn't timeout on a page that has a delayed
// iframe load. Context: crbug.com/1046883
IN_PROC_BROWSER_TEST_P(CreateShortcutBrowserTest, WorksAfterDelayedIFrameLoad) {
ASSERT_TRUE(embedded_test_server()->Start());
NavigateToURLAndWait(browser(), embedded_test_server()->GetURL(
"/favicon/page_with_favicon.html"));
// Append an iframe and wait for it to finish loading.
const char script[] = R"(
const iframe = document.createElement('iframe');
iframe.onload = _ => domAutomationController.send('success');
iframe.srcdoc = 'inner page';
document.body.appendChild(iframe);
)";
EXPECT_EQ(content::EvalJsWithManualReply(
browser()->tab_strip_model()->GetActiveWebContents(), script)
.ExtractString(),
"success");
InstallShortcutAppForCurrentUrl();
}
INSTANTIATE_TEST_SUITE_P( INSTANTIATE_TEST_SUITE_P(
All, All,
CreateShortcutBrowserTest, CreateShortcutBrowserTest,
......
...@@ -4412,7 +4412,8 @@ void WebContentsImpl::SetFocusToLocationBar() { ...@@ -4412,7 +4412,8 @@ void WebContentsImpl::SetFocusToLocationBar() {
void WebContentsImpl::DidStartNavigation(NavigationHandle* navigation_handle) { void WebContentsImpl::DidStartNavigation(NavigationHandle* navigation_handle) {
TRACE_EVENT1("navigation", "WebContentsImpl::DidStartNavigation", TRACE_EVENT1("navigation", "WebContentsImpl::DidStartNavigation",
"navigation_handle", navigation_handle); "navigation_handle", navigation_handle);
favicon_urls_.clear(); if (navigation_handle->IsInMainFrame())
favicon_urls_.clear();
for (auto& observer : observers_) for (auto& observer : observers_)
observer.DidStartNavigation(navigation_handle); observer.DidStartNavigation(navigation_handle);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment