Reload favicon from HTTP cache on Ctrl+Refresh

chrome/browser/favicon/favicon_tab_helper.cc

Index: chrome/browser/favicon/favicon_tab_helper.cc
diff --git a/chrome/browser/favicon/favicon_tab_helper.cc b/chrome/browser/favicon/favicon_tab_helper.cc
index 053cdfc20a923e9563844f47edd786e9be1e7513..047825fa75c7c5c69a28f9b284d647f9862715a1 100644
--- a/chrome/browser/favicon/favicon_tab_helper.cc
+++ b/chrome/browser/favicon/favicon_tab_helper.cc
@@ -153,11 +153,16 @@ int FaviconTabHelper::StartDownload(const GURL& url, int max_image_size) {
     return 0;
   }
 
+  blink::WebURLRequest::CachePolicy cache_policy =
+      (bypass_cache_page_url_ == GetActiveURL())
+          ? blink::WebURLRequest::ReloadBypassingCache
+          : blink::WebURLRequest::UseProtocolCachePolicy;
+  bypass_cache_page_url_ = GURL();

[pkotwicz, 2015/02/19 16:47:02]

I used a separate variable to track whether the favicon should be fetched
bypassing the cache. It unlikely but possible for the favicon not to be cached
in the history database but to be cached in the HTTP cache (e.g. open a new
profile, navigate to http://www.google.com/favicon.ico, then navigate to
www.google.com and press Ctrl+F5)

+
   return web_contents()->DownloadImage(
-      url,
-      true,
-      max_image_size,
-      base::Bind(&FaviconTabHelper::DidDownloadFavicon,base::Unretained(this)));
+      url, true, max_image_size, cache_policy,
+      base::Bind(&FaviconTabHelper::DidDownloadFavicon,
+                 base::Unretained(this)));
 }
 
 bool FaviconTabHelper::IsOffTheRecord() {
@@ -230,6 +235,8 @@ void FaviconTabHelper::DidStartNavigationToPendingEntry(
     NavigationController::ReloadType reload_type) {
   if (reload_type != NavigationController::NO_RELOAD &&
       !profile_->IsOffTheRecord()) {
+    bypass_cache_page_url_ = url;

[pkotwicz, 2015/02/19 16:47:02]

Not sure if I should do this only if |reload_type| is
NavigationController::RELOAD_IGNORING_CACHE

+
     FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
         profile_, ServiceAccessType::IMPLICIT_ACCESS);
     if (favicon_service) {
@@ -244,8 +251,20 @@ void FaviconTabHelper::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
   favicon_urls_.clear();
+
+  // Wait till the user navigates to a new URL to start checking the cache
+  // again. The cache may be ignored for non-reload navigations (e.g.
+  // history.replace() in-page navigation). This is allowed to increase the
+  // likelihood that "reloading a page ignoring the cache" redownloads the
+  // favicon. In particular, a page may do an in-page navigation before
+  // FaviconHandler has the time to determine that the favicon needs to be
+  // redownloaded.

[pkotwicz, 2015/02/19 16:47:02]

www.youtube.com calls history.replace() right after being navigated to

+  GURL url = details.entry->GetURL();
+  if (url != bypass_cache_page_url_)
+    bypass_cache_page_url_ = GURL();
+
   // Get the favicon, either from history or request it from the net.
-  FetchFavicon(details.entry->GetURL());
+  FetchFavicon(url);
 }
 
 // Returns favicon_base::IconType the given icon_type corresponds to.