Find Favicon in priority of icon_type.

chrome/browser/favicon/favicon_service.cc

Index: chrome/browser/favicon/favicon_service.cc
diff --git a/chrome/browser/favicon/favicon_service.cc b/chrome/browser/favicon/favicon_service.cc
index fd0f9eb557c4cd9931b4a70dee6df9e80020add7..9fe51a223d5780db26f033594f9acb71787d983e 100644
--- a/chrome/browser/favicon/favicon_service.cc
+++ b/chrome/browser/favicon/favicon_service.cc
@@ -45,6 +45,23 @@ CancelableTaskTracker::TaskId RunWithEmptyResultAsync(
       Bind(callback, std::vector<chrome::FaviconBitmapResult>()));
 }
 
+// Return the TaskId of the one to retreive the favicon from chrome specific

[pkotwicz, 2013/10/15 21:39:41]

How about: "Returns the TaskId to retrieve the favicon from a chrome-specific
URL."

[michaelbai, 2013/10/15 22:52:34]

Done.

+// URL.
+CancelableTaskTracker::TaskId GetFaviconForChromeURL(
+    Profile* profile,
+    const GURL& page_url,
+    const std::vector<ui::ScaleFactor>& desired_scale_factors,
+    const FaviconService::FaviconResultsCallback& callback,
+    CancelableTaskTracker* tracker) {
+  CancelableTaskTracker::IsCanceledCallback is_canceled_cb;
+  CancelableTaskTracker::TaskId id = tracker->NewTrackedTaskId(&is_canceled_cb);
+  FaviconService::FaviconResultsCallback cancelable_cb =
+      Bind(&CancelOrRunFaviconResultsCallback, is_canceled_cb, callback);
+  ChromeWebUIControllerFactory::GetInstance()->GetFaviconForURL(profile,
+      page_url, desired_scale_factors, cancelable_cb);
+  return id;
+}
+
 }  // namespace
 
 FaviconService::FaviconService(HistoryService* history_service)
@@ -169,6 +186,23 @@ CancelableTaskTracker::TaskId FaviconService::GetRawFaviconForURL(
       tracker);
 }
 
+CancelableTaskTracker::TaskId FaviconService::GetLargestRawFaviconForURL(
+    Profile* profile,
+    const GURL& page_url,
+    const std::vector<int>& icon_types,
+    int minimal_size_in_pixels,
+    const FaviconRawCallback& callback,
+    CancelableTaskTracker* tracker) {
+  return GetLargestRawFaviconForURLImpl(
+      profile, page_url, icon_types, minimal_size_in_pixels,
+      Bind(&FaviconService::RunFaviconRawCallbackWithBitmapResults,
+           base::Unretained(this),
+           callback,
+           0,
+           ui::ScaleFactor()),
+      tracker);
+}
+
 CancelableTaskTracker::TaskId FaviconService::GetFaviconForURL(
     const FaviconForURLParams& params,
     const FaviconResultsCallback& callback,
@@ -284,15 +318,8 @@ CancelableTaskTracker::TaskId FaviconService::GetFaviconForURLImpl(
     CancelableTaskTracker* tracker) {
   if (params.page_url.SchemeIs(chrome::kChromeUIScheme) ||
       params.page_url.SchemeIs(extensions::kExtensionScheme)) {
-    CancelableTaskTracker::IsCanceledCallback is_canceled_cb;
-    CancelableTaskTracker::TaskId id =
-        tracker->NewTrackedTaskId(&is_canceled_cb);
-
-    FaviconResultsCallback cancelable_cb =
-        Bind(&CancelOrRunFaviconResultsCallback, is_canceled_cb, callback);
-    ChromeWebUIControllerFactory::GetInstance()->GetFaviconForURL(
-        params.profile, params.page_url, desired_scale_factors, cancelable_cb);
-    return id;
+    return GetFaviconForChromeURL(params.profile, params.page_url,
+                                  desired_scale_factors, callback, tracker);
   } else if (history_service_) {
     return history_service_->GetFaviconsForURL(params.page_url,
                                                params.icon_types,
@@ -300,9 +327,26 @@ CancelableTaskTracker::TaskId FaviconService::GetFaviconForURLImpl(
                                                desired_scale_factors,
                                                callback,
                                                tracker);
-  } else {
-    return RunWithEmptyResultAsync(callback, tracker);
   }
+  return RunWithEmptyResultAsync(callback, tracker);
+}
+
+CancelableTaskTracker::TaskId FaviconService::GetLargestRawFaviconForURLImpl(

[pkotwicz, 2013/10/15 21:39:41]

Can you explain why you need both GetLargestRawFaviconForURL() and
GetLargestRawFaviconForURLImpl()

[michaelbai, 2013/10/15 22:52:34]

Removed

On 2013/10/15 21:39:41, pkotwicz wrote:

+    Profile* profile,
+    const GURL& page_url,
+    const std::vector<int>& icon_types,
+    int minimal_size_in_pixels,
+    const FaviconResultsCallback& callback,
+    CancelableTaskTracker* tracker) {
+  if (page_url.SchemeIs(chrome::kChromeUIScheme) ||
+      page_url.SchemeIs(extensions::kExtensionScheme)) {
+    return GetFaviconForChromeURL(profile, page_url,
+        std::vector<ui::ScaleFactor>(), callback, tracker);

[pkotwicz, 2013/10/15 21:39:41]

You should pass in a vector with ui::SCALE_FACTOR_100P. Passing in an empty
vector is wrong given ChromeWebUIControllerFactory::GetFaviconForURL().

[michaelbai, 2013/10/15 22:52:34]

Thanks

+  } else if (history_service_) {
+    return history_service_->GetLargestFaviconForURL(page_url, icon_types,
+        minimal_size_in_pixels, callback, tracker);
+  }
+  return RunWithEmptyResultAsync(callback, tracker);
 }
 
 void FaviconService::RunFaviconImageCallbackWithBitmapResults(