 Chromium Code Reviews
 Chromium Code Reviews Issue 1261143004:
  Implement manifest icon downloader  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 1261143004:
  Implement manifest icon downloader  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: chrome/browser/android/shortcut_data_fetcher.cc | 
| diff --git a/chrome/browser/android/shortcut_data_fetcher.cc b/chrome/browser/android/shortcut_data_fetcher.cc | 
| index e419712d7e50b235b052e697cd38250addb6bfc2..1b32b7b0ce7100f87883787d7f2e54553b7d10bd 100644 | 
| --- a/chrome/browser/android/shortcut_data_fetcher.cc | 
| +++ b/chrome/browser/android/shortcut_data_fetcher.cc | 
| @@ -9,6 +9,7 @@ | 
| #include "base/strings/string16.h" | 
| #include "base/task/cancelable_task_tracker.h" | 
| #include "chrome/browser/favicon/favicon_service_factory.h" | 
| +#include "chrome/browser/manifest/manifest_icon_downloader.h" | 
| #include "chrome/browser/manifest/manifest_icon_selector.h" | 
| #include "chrome/browser/profiles/profile.h" | 
| #include "chrome/common/chrome_constants.h" | 
| @@ -45,6 +46,7 @@ ShortcutDataFetcher::ShortcutDataFetcher( | 
| icon_timeout_timer_(false, false), | 
| shortcut_info_(dom_distiller::url_utils::GetOriginalUrlFromDistillerUrl( | 
| web_contents->GetURL())), | 
| + icon_downloader_(new ManifestIconDownloader(web_contents)), | 
| preferred_icon_size_in_px_(kPreferredIconSizeInDp * | 
| gfx::Screen::GetScreenFor(web_contents->GetNativeView())-> | 
| GetPrimaryDisplay().device_scale_factor()) { | 
| @@ -109,17 +111,14 @@ void ShortcutDataFetcher::OnDidGetManifest(const content::Manifest& manifest) { | 
| manifest.icons, | 
| kPreferredIconSizeInDp, | 
| gfx::Screen::GetScreenFor(web_contents()->GetNativeView())); | 
| - if (icon_src.is_valid()) { | 
| - // Grab the best icon from the manifest. | 
| - web_contents()->DownloadImage( | 
| + | 
| + // If fetching the Manifest icon fails, fallback to the best favicon | 
| + // for the page. | 
| + if (!icon_downloader_->Download( | 
| icon_src, | 
| - false, | 
| - preferred_icon_size_in_px_, | 
| - false, | 
| - base::Bind(&ShortcutDataFetcher::OnManifestIconFetched, | 
| - this)); | 
| - } else { | 
| - // Grab the best favicon for the page. | 
| + kPreferredIconSizeInDp, | 
| + base::Bind(&ShortcutDataFetcher::NotifyObserver, | 
| + this))) { | 
| FetchFavicon(); | 
| } | 
| @@ -182,9 +181,7 @@ void ShortcutDataFetcher::FetchFavicon() { | 
| void ShortcutDataFetcher::OnFaviconFetched( | 
| const favicon_base::FaviconRawBitmapResult& bitmap_result) { | 
| - if (!web_contents() || !weak_observer_ || is_icon_saved_) { | 
| - return; | 
| - } | 
| + if (!web_contents() || !weak_observer_ || is_icon_saved_) return; | 
| 
mlamouri (slow - plz ping)
2015/08/10 15:08:08
nit: in C++, |return;| should be in a new line, co
 
Lalit Maganti
2015/08/10 16:55:45
Done - although there are a lot of cases in this f
 | 
| content::BrowserThread::PostTask( | 
| content::BrowserThread::IO, | 
| @@ -217,34 +214,6 @@ void ShortcutDataFetcher::CreateLauncherIcon( | 
| base::Bind(&ShortcutDataFetcher::NotifyObserver, this, icon_bitmap)); | 
| } | 
| -void ShortcutDataFetcher::OnManifestIconFetched( | 
| - int id, | 
| - int http_status_code, | 
| - const GURL& url, | 
| - const std::vector<SkBitmap>& bitmaps, | 
| - const std::vector<gfx::Size>& sizes) { | 
| - if (!web_contents() || !weak_observer_) return; | 
| - | 
| - // If getting the candidate manifest icon failed, the ShortcutHelper should | 
| - // fallback to the favicon. | 
| - // Otherwise, it sets the state as if there was no manifest icon pending. | 
| - if (bitmaps.empty()) { | 
| - FetchFavicon(); | 
| - return; | 
| - } | 
| - | 
| - // There might be multiple bitmaps returned. The one to pick is bigger or | 
| - // equal to the preferred size. |bitmaps| is ordered from bigger to smaller. | 
| - int preferred_bitmap_index = 0; | 
| - for (size_t i = 0; i < bitmaps.size(); ++i) { | 
| - if (bitmaps[i].height() < preferred_icon_size_in_px_) | 
| - break; | 
| - preferred_bitmap_index = i; | 
| - } | 
| - | 
| - NotifyObserver(bitmaps[preferred_bitmap_index]); | 
| -} | 
| - | 
| void ShortcutDataFetcher::NotifyObserver(const SkBitmap& bitmap) { | 
| DCHECK_CURRENTLY_ON(content::BrowserThread::UI); | 
| if (!web_contents() || !weak_observer_ || is_icon_saved_) |