Make logic in OnFaviconDataForInitialURLFromFaviconService() the same for touch & non-touch favicons

components/favicon/core/favicon_handler.cc

 // Copyright (c) 2012 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/core/favicon_handler.h"
 
 #include <algorithm>
 #include <cmath>
 #include <vector>
 
@@ skipping 197 matching lines @@
       score(score),
       icon_type(icon_type) {}
 
 ////////////////////////////////////////////////////////////////////////////////
 
 FaviconHandler::FaviconHandler(FaviconService* service,
                                FaviconDriver* driver,
                                Type handler_type)
     : got_favicon_from_history_(false),
       initial_history_result_expired_or_incomplete_(false),
+      redownload_icons_(false),
       icon_types_(FaviconHandler::GetIconTypesFromHandlerType(handler_type)),
       download_largest_icon_(handler_type == LARGEST_FAVICON ||
                              handler_type == LARGEST_TOUCH),
       notification_icon_type_(favicon_base::INVALID_ICON),
       service_(service),
       driver_(driver),
       current_candidate_index_(0u) {
   DCHECK(driver_);
 }
 
@@ skipping 12 matching lines @@
   }
   return 0;
 }
 
 void FaviconHandler::FetchFavicon(const GURL& url) {
   cancelable_task_tracker_.TryCancelAll();
 
   url_ = url;
 
   initial_history_result_expired_or_incomplete_ = false;
+  redownload_icons_ = false;
   got_favicon_from_history_ = false;
   download_requests_.clear();
   image_urls_.clear();
   notification_icon_url_ = GURL();
   notification_icon_type_ = favicon_base::INVALID_ICON;
   current_candidate_index_ = 0u;
   best_favicon_candidate_ = FaviconCandidate();
 
   // Request the favicon from the history service. In parallel to this the
   // renderer is going to notify us (well WebContents) when the favicon url is
@@ skipping 305 matching lines @@
       return 0;
   }
   NOTREACHED();
   return 0;
 }
 
 void FaviconHandler::OnFaviconDataForInitialURLFromFaviconService(
     const std::vector<favicon_base::FaviconRawBitmapResult>&
         favicon_bitmap_results) {
   got_favicon_from_history_ = true;
-  bool has_results = !favicon_bitmap_results.empty();
-  initial_history_result_expired_or_incomplete_ = HasExpiredOrIncompleteResult(
-      preferred_icon_size(), favicon_bitmap_results);
   bool has_valid_result = HasValidResult(favicon_bitmap_results);
+  initial_history_result_expired_or_incomplete_ =
+      !has_valid_result ||
+      HasExpiredOrIncompleteResult(preferred_icon_size(),
+                                   favicon_bitmap_results);
+  redownload_icons_ = initial_history_result_expired_or_incomplete_ &&
+                      !favicon_bitmap_results.empty();
 
-  if (has_results && !download_largest_icon_ &&
+  if (has_valid_result &&
       (!current_candidate() ||
        DoUrlsAndIconsMatch(*current_candidate(), favicon_bitmap_results))) {
-    if (has_valid_result) {
-      // The db knows the favicon (although it may be out of date) and the entry
-      // doesn't have an icon. Set the favicon now, and if the favicon turns out
-      // to be expired (or the wrong url) we'll fetch later on. This way the
-      // user doesn't see a flash of the default favicon.
-      NotifyFaviconAvailable(favicon_bitmap_results);
-    } else {
-      // If |favicon_bitmap_results| does not have any valid results, treat the
-      // favicon as if it's expired.
-      // TODO(pkotwicz): Do something better.
-      initial_history_result_expired_or_incomplete_ = true;
-    }
+    // The db knows the favicon (although it may be out of date) and the entry
+    // doesn't have an icon. Set the favicon now, and if the favicon turns out
+    // to be expired (or the wrong url) we'll fetch later on. This way the
+    // user doesn't see a flash of the default favicon.
+
+    NotifyFaviconAvailable(favicon_bitmap_results);
   }
 
-  if (has_valid_result && download_largest_icon_)
-    NotifyFaviconAvailable(favicon_bitmap_results);
-
   if (current_candidate())
     OnGotInitialHistoryDataAndIconURLCandidates();
 }
 
 void FaviconHandler::DownloadCurrentCandidateOrAskFaviconService() {
   GURL icon_url = current_candidate()->icon_url;
   favicon_base::IconType icon_type = current_candidate()->icon_type;
 
-  if (initial_history_result_expired_or_incomplete_) {
+  if (redownload_icons_) {
     // We have the mapping, but the favicon is out of date. Download it now.
     ScheduleDownload(icon_url, icon_type);
   } else {
     // We don't know the favicon, but we may have previously downloaded the
     // favicon for another page that shares the same favicon. Ask for the
     // favicon given the favicon URL.
     if (driver_->IsOffTheRecord()) {
       GetFaviconFromFaviconService(
           icon_url, icon_type,
           base::Bind(&FaviconHandler::OnFaviconData, base::Unretained(this)),
           &cancelable_task_tracker_);
     } else {
       // Ask the history service for the icon. This does two things:
       // 1. Attempts to fetch the favicon data from the database.
       // 2. If the favicon exists in the database, this updates the database to
       //    include the mapping between the page url and the favicon url.
       // This is asynchronous. The history service will call back when done.
       UpdateFaviconMappingAndFetch(
           url_, icon_url, icon_type,
           base::Bind(&FaviconHandler::OnFaviconData, base::Unretained(this)),
           &cancelable_task_tracker_);
     }
   }
 }
 
 void FaviconHandler::OnFaviconData(const std::vector<
     favicon_base::FaviconRawBitmapResult>& favicon_bitmap_results) {
   bool has_results = !favicon_bitmap_results.empty();
-  bool has_expired_or_incomplete_result = HasExpiredOrIncompleteResult(
-      preferred_icon_size(), favicon_bitmap_results);
   bool has_valid_result = HasValidResult(favicon_bitmap_results);
+  bool has_expired_or_incomplete_result =
+      !has_valid_result || HasExpiredOrIncompleteResult(preferred_icon_size(),
+                                                        favicon_bitmap_results);
 
   if (has_valid_result) {
     // There is a valid favicon. Notify any observers. It is useful to notify
     // the observers even if the favicon is expired or incomplete (incorrect
     // size) because temporarily showing the user an expired favicon or
     // streched favicon is preferable to showing the user the default favicon.
     NotifyFaviconAvailable(favicon_bitmap_results);
   }
 
   if (!current_candidate() ||
       (has_results &&
        !DoUrlsAndIconsMatch(*current_candidate(), favicon_bitmap_results))) {
     // The icon URLs have been updated since the favicon data was requested.
     return;
   }
 
-  if (!has_results || has_expired_or_incomplete_result) {
+  if (has_expired_or_incomplete_result) {
     ScheduleDownload(current_candidate()->icon_url,
                      current_candidate()->icon_type);
   }
 }
 
 void FaviconHandler::ScheduleDownload(const GURL& image_url,
                                       favicon_base::IconType icon_type) {
   // A max bitmap size is specified to avoid receiving huge bitmaps in
   // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
   // for more details about the max bitmap size.
   const int download_id = DownloadFavicon(image_url,
                                           GetMaximalIconSize(icon_type));
 
   // Download ids should be unique.
   DCHECK(download_requests_.find(download_id) == download_requests_.end());
   download_requests_[download_id] = DownloadRequest(image_url, icon_type);
 
   if (download_id == 0) {
     // If DownloadFavicon() did not start a download, it returns a download id
     // of 0. We still need to call OnDidDownloadFavicon() because the method is
     // responsible for initiating the data request for the next candidate.
     OnDidDownloadFavicon(download_id, image_url, std::vector<SkBitmap>(),
                          std::vector<gfx::Size>());
 
   }
 }
 
 }  // namespace favicon