Do not attempt to download favicons with 404 status in WebView

android_webview/browser/icon_helper.cc

 // Copyright (c) 2013 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 "android_webview/browser/icon_helper.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/hash.h"
 #include "base/logging.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/favicon_url.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/size.h"
 
 using content::BrowserThread;
 using content::WebContents;
 
 namespace android_webview {
 
 IconHelper::IconHelper(WebContents* web_contents)
     : WebContentsObserver(web_contents),
-      listener_(NULL) {
+      listener_(NULL),
+      missing_favicon_urls_() {
 }
 
 IconHelper::~IconHelper() {
 }
 
 void IconHelper::SetListener(Listener* listener) {
   listener_ = listener;
 }
 
 void IconHelper::DownloadFaviconCallback(
     int id,
     int http_status_code,
     const GURL& image_url,
     const std::vector<SkBitmap>& bitmaps,
     const std::vector<gfx::Size>& original_bitmap_sizes) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (http_status_code == 404) {
+    MarkUnableToDownloadFavicon(image_url);
+    return;
+  }
+
   if (bitmaps.size() == 0) {
     return;
   }
 
   // We can protentially have multiple frames of the icon
   // in different sizes. We need more fine grain API spec
   // to let clients pick out the frame they want.
 
   // TODO(acleung): Pick the best icon to return based on size.
   if (listener_)
     listener_->OnReceivedIcon(image_url, bitmaps[0]);
 }
 
 void IconHelper::DidUpdateFaviconURL(int32 page_id,
     const std::vector<content::FaviconURL>& candidates) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   for (std::vector<content::FaviconURL>::const_iterator i = candidates.begin();
        i != candidates.end(); ++i) {
     if (!i->icon_url.is_valid())
       continue;
 
     switch(i->icon_type) {
       case content::FaviconURL::FAVICON:
-        if (listener_ && !listener_->ShouldDownloadFavicon(i->icon_url)) break;
+        if ((listener_ && !listener_->ShouldDownloadFavicon(i->icon_url)) ||
+            WasUnableToDownloadFavicon(i->icon_url)) {
+          break;
+        }
         web_contents()->DownloadImage(i->icon_url,
             true,  // Is a favicon
             0,  // No maximum size
             base::Bind(
                 &IconHelper::DownloadFaviconCallback, base::Unretained(this)));
         break;
       case content::FaviconURL::TOUCH_ICON:
         if (listener_)
           listener_->OnReceivedTouchIconUrl(i->icon_url.spec(), false);
         break;
       case content::FaviconURL::TOUCH_PRECOMPOSED_ICON:
         if (listener_)
           listener_->OnReceivedTouchIconUrl(i->icon_url.spec(), true);
         break;
       case content::FaviconURL::INVALID_ICON:
         // Silently ignore it. Only trigger a callback on valid icons.
         break;
       default:
         NOTREACHED();
         break;
     }
   }
 }
 
+void IconHelper::DidStartNavigationToPendingEntry(
+    const GURL& url,
+    content::NavigationController::ReloadType reload_type) {
+  if (reload_type == content::NavigationController::RELOAD_IGNORING_CACHE)
+    ClearUnableToDownloadFavicons();

[mkosiba (inactive), 2014/04/24 15:42:44]

just curious - why don't we want to clear when we navigate to different urls
too?

[cimamoglu (inactive), 2014/04/28 10:41:04]

The original downstream bug description specifically defines the problem as
"WebView tries to fetch favicon.ico when WebView.goBack()" We would like to keep
track of past favicon 404s, so navigating to different URLs should not cause us
to clear. Also, this is essentially how it's done in the original CL.

+}
+
+void IconHelper::MarkUnableToDownloadFavicon(const GURL& icon_url) {
+  MissingFaviconURLHash url_hash = base::Hash(icon_url.spec());
+  missing_favicon_urls_.insert(url_hash);
+}
+
+bool IconHelper::WasUnableToDownloadFavicon(const GURL& icon_url) const {
+  MissingFaviconURLHash url_hash = base::Hash(icon_url.spec());
+  return missing_favicon_urls_.find(url_hash) != missing_favicon_urls_.end();
+}
+
+void IconHelper::ClearUnableToDownloadFavicons() {
+  missing_favicon_urls_.clear();
+}
+
+

[benm (inactive), 2014/04/24 19:03:21]

nit: double new line

[cimamoglu (inactive), 2014/04/28 10:41:04]

Done.

 }  // namespace android_webview