Avoid cyclic dependency FaviconHandler<-->FaviconDriverImpl

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>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/ref_counted_memory.h"
 #include "build/build_config.h"
-#include "components/favicon/core/favicon_driver.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon_base/favicon_util.h"
 #include "components/favicon_base/select_favicon_frames.h"
 #include "skia/ext/image_operations.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_util.h"
 
 namespace favicon {
 namespace {
@@ skipping 179 matching lines @@
     favicon_base::IconType icon_type)
     : image_url(image_url),
       image(image),
       score(score),
       icon_type(icon_type) {}
 
 ////////////////////////////////////////////////////////////////////////////////
 
 FaviconHandler::FaviconHandler(
     FaviconService* service,
-    FaviconDriver* driver,
+    Delegate* delegate,
     FaviconDriverObserver::NotificationIconType handler_type)
     : handler_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 == FaviconDriverObserver::NON_TOUCH_LARGEST ||
           handler_type == FaviconDriverObserver::TOUCH_LARGEST),
       notification_icon_type_(favicon_base::INVALID_ICON),
       service_(service),
-      driver_(driver),
-      current_candidate_index_(0u) {
-  DCHECK(driver_);
+      delegate_(delegate),
+      current_candidate_index_(0u),
+      weak_ptr_factory_(this) {
+  // DCHECK(service_);
+  DCHECK(delegate_);
 }
 
 FaviconHandler::~FaviconHandler() {
 }
 
 // static
 int FaviconHandler::GetIconTypesFromHandlerType(
     FaviconDriverObserver::NotificationIconType handler_type) {
   switch (handler_type) {
     case FaviconDriverObserver::NON_TOUCH_16_DIP:
@@ skipping 106 matching lines @@
 
 void FaviconHandler::NotifyFaviconUpdated(const GURL& icon_url,
                                           favicon_base::IconType icon_type,
                                           const gfx::Image& image) {
   if (image.IsEmpty())
     return;
 
   gfx::Image image_with_adjusted_colorspace = image;
   favicon_base::SetFaviconColorSpace(&image_with_adjusted_colorspace);
 
-  driver_->OnFaviconUpdated(url_, handler_type_, icon_url,
-                            icon_url != notification_icon_url_,
-                            image_with_adjusted_colorspace);
+  delegate_->OnFaviconUpdated(url_, handler_type_, icon_url,
+                              icon_url != notification_icon_url_,
+                              image_with_adjusted_colorspace);
 
   notification_icon_url_ = icon_url;
   notification_icon_type_ = icon_type;
 }
 
 void FaviconHandler::OnUpdateFaviconURL(
     const GURL& page_url,
     const std::vector<FaviconURL>& candidates) {
   if (page_url != url_)
     return;
@@ skipping 37 matching lines @@
     // We are done. No additional downloads or history requests are needed.
     // TODO: Store all of the icon URLs associated with a page in history so
     // that we can check whether the page's icon URLs match the page's icon URLs
     // at the time that the favicon data was stored to the history database.
     return;
   }
 
   DownloadCurrentCandidateOrAskFaviconService();
 }
 
-void FaviconHandler::OnDidDownloadFavicon(
+void FaviconHandler::DidDownloadFavicon(
     int id,
+    int http_status_code,
     const GURL& image_url,
     const std::vector<SkBitmap>& bitmaps,
     const std::vector<gfx::Size>& original_bitmap_sizes) {
+  if (bitmaps.empty() && http_status_code == 404) {
+    DVLOG(1) << "Failed to Download Favicon:" << image_url;
+    if (service_)
+      service_->UnableToDownloadFavicon(image_url);
+  }
+
   DownloadRequests::iterator i = download_requests_.find(id);
   if (i == download_requests_.end()) {
     // Currently WebContents notifies us of ANY downloads so that it is
     // possible to get here.
     return;
   }
 
   DownloadRequest download_request = i->second;
   download_requests_.erase(i);
 
@@ skipping 54 matching lines @@
     current_candidate_index_ = image_urls_.size();
     best_favicon_candidate_ = FaviconCandidate();
   }
 }
 
 bool FaviconHandler::HasPendingTasksForTest() {
   return !download_requests_.empty() ||
          cancelable_task_tracker_.HasTrackedTasks();
 }
 
-int FaviconHandler::DownloadFavicon(const GURL& image_url,
-                                    int max_bitmap_size) {
-  if (!image_url.is_valid()) {
-    NOTREACHED();
-    return 0;
-  }
-  return driver_->StartDownload(image_url, max_bitmap_size);
-}
-
 void FaviconHandler::UpdateFaviconMappingAndFetch(
     const GURL& page_url,
     const GURL& icon_url,
     favicon_base::IconType icon_type,
     const favicon_base::FaviconResultsCallback& callback,
     base::CancelableTaskTracker* tracker) {
   // TODO(pkotwicz): pass in all of |image_urls_| to
   // UpdateFaviconMappingsAndFetch().
   if (service_) {
     std::vector<GURL> icon_urls;
@@ skipping 29 matching lines @@
 void FaviconHandler::SetHistoryFavicons(const GURL& page_url,
                                         const GURL& icon_url,
                                         favicon_base::IconType icon_type,
                                         const gfx::Image& image) {
   if (service_) {
     service_->SetFavicons(page_url, icon_url, icon_type, image);
   }
 }
 
 bool FaviconHandler::ShouldSaveFavicon() {
-  if (!driver_->IsOffTheRecord())
+  if (!delegate_->IsOffTheRecord())
     return true;
 
   // Always save favicon if the page is bookmarked.
-  return driver_->IsBookmarked(url_);
+  // TODO(mastiz): Fix.

[pkotwicz, 2017/02/13 23:35:48]

It is possible to bookmark a page while in incognito. This code is there so that
the new bookmark has an icon on the bookmarks bar and the bookmarks page.

[mastiz, 2017/02/23 21:55:53]

I believe this is a bug that needs to be fixed but is orthogonal to this patch.
We're leaking incognito-mode activity and in fact it's something that users
could exploit by artificially creating a bookmark and clearing favicon cache, to
later know if another person visited that site.

+  // return driver_->IsBookmarked(url_);
+  DCHECK(false);
+  return false;
 }
 
 int FaviconHandler::GetMaximalIconSize(favicon_base::IconType icon_type) {
   switch (icon_type) {
     case favicon_base::FAVICON:
 #if defined(OS_ANDROID)
       return 192;
 #else
       return gfx::ImageSkia::GetMaxSupportedScale() * gfx::kFaviconSize;
 #endif
@@ skipping 37 matching lines @@
   GURL icon_url = current_candidate()->icon_url;
   favicon_base::IconType icon_type = current_candidate()->icon_type;
 
   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()) {
+    if (delegate_->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.
@@ skipping 29 matching lines @@
   }
 
   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) {
+  if (!image_url.is_valid()) {
+    NOTREACHED();
+    return;
+  }
   // 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));
+  const int download_id =
+      delegate_->DownloadImage(image_url, GetMaximalIconSize(icon_type),
+                               base::Bind(&FaviconHandler::DidDownloadFavicon,
+                                          weak_ptr_factory_.GetWeakPtr()));
 
   // 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>());
-
+    DidDownloadFavicon(download_id, 0, image_url, std::vector<SkBitmap>(),
+                       std::vector<gfx::Size>());
   }
 }
 
 }  // namespace favicon