Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(246)

Unified Diff: chrome/browser/banners/app_banner_data_fetcher.cc

Issue 1261143004: Implement manifest icon downloader (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix issues raised in comments Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/banners/app_banner_data_fetcher.cc
diff --git a/chrome/browser/banners/app_banner_data_fetcher.cc b/chrome/browser/banners/app_banner_data_fetcher.cc
index 6734f4b9afb90e49893a0768f5aa67512b771511..09ab53e8789b08988f571cf114fe6155ca41060b 100644
--- a/chrome/browser/banners/app_banner_data_fetcher.cc
+++ b/chrome/browser/banners/app_banner_data_fetcher.cc
@@ -79,7 +79,8 @@ AppBannerDataFetcher::AppBannerDataFetcher(
is_active_(false),
was_canceled_by_page_(false),
page_requested_prompt_(false),
- event_request_id_(-1) {
+ event_request_id_(-1),
+ icon_downloader_(new ManifestIconDownloader(web_contents)) {
}
void AppBannerDataFetcher::Start(const GURL& validated_url,
@@ -218,27 +219,6 @@ std::string AppBannerDataFetcher::GetAppIdentifier() {
return web_app_data_.start_url.spec();
}
-bool AppBannerDataFetcher::FetchIcon(const GURL& image_url) {
- content::WebContents* web_contents = GetWebContents();
- DCHECK(web_contents);
-
- // Begin asynchronously fetching the app icon. AddRef() is done before the
- // fetch to ensure that the AppBannerDataFetcher isn't deleted before the
- // BitmapFetcher has called OnFetchComplete() (where the references are
- // decremented).
- AddRef();
- Profile* profile =
- Profile::FromBrowserContext(web_contents->GetBrowserContext());
- bitmap_fetcher_.reset(new chrome::BitmapFetcher(image_url, this));
- bitmap_fetcher_->Init(
- profile->GetRequestContext(),
- std::string(),
- net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE,
- net::LOAD_NORMAL);
- bitmap_fetcher_->Start();
- return true;
-}
-
void AppBannerDataFetcher::RecordDidShowBanner(const std::string& event_name) {
content::WebContents* web_contents = GetWebContents();
DCHECK(web_contents);
@@ -316,51 +296,51 @@ void AppBannerDataFetcher::OnDidCheckHasServiceWorker(
}
if (has_service_worker) {
- // Create an infobar to promote the manifest's app.
- GURL icon_url =
- ManifestIconSelector::FindBestMatchingIcon(
- web_app_data_.icons,
- ideal_icon_size_,
- gfx::Screen::GetScreenFor(web_contents->GetNativeView()));
- if (!icon_url.is_empty()) {
- FetchIcon(icon_url);
- return;
- }
- OutputDeveloperNotShownMessage(web_contents, kCannotDetermineBestIcon);
- } else {
- TrackDisplayEvent(DISPLAY_EVENT_LACKS_SERVICE_WORKER);
- OutputDeveloperNotShownMessage(web_contents, kNoMatchingServiceWorker);
+ OnHasServiceWorker(web_contents);
+ return;
}
+ TrackDisplayEvent(DISPLAY_EVENT_LACKS_SERVICE_WORKER);
+ OutputDeveloperNotShownMessage(web_contents, kNoMatchingServiceWorker);
Cancel();
}
-void AppBannerDataFetcher::OnFetchComplete(const GURL& url,
- const SkBitmap* icon) {
- if (is_active_)
- RequestShowBanner(icon);
+void AppBannerDataFetcher::OnHasServiceWorker(
+ content::WebContents* web_contents) {
+ GURL icon_url =
+ ManifestIconSelector::FindBestMatchingIcon(
+ web_app_data_.icons,
+ ideal_icon_size_,
+ gfx::Screen::GetScreenFor(web_contents->GetNativeView()));
+
+ if (FetchAppIcon(web_contents, icon_url)) return;
- Release();
+ OutputDeveloperNotShownMessage(web_contents, kCannotDetermineBestIcon);
+ Cancel();
}
-bool AppBannerDataFetcher::IsWebAppInstalled(
- content::BrowserContext* browser_context,
- const GURL& start_url) {
- return false;
+bool AppBannerDataFetcher::FetchAppIcon(content::WebContents* web_contents,
+ const GURL& icon_url) {
+ return icon_downloader_->Download(
mlamouri (slow - plz ping) 2015/08/05 08:20:16 nit: add DCHECK(web_contents);
Lalit Maganti 2015/08/05 14:47:13 Removed web contents altogether.
+ icon_url,
+ ideal_icon_size_,
+ base::Bind(&AppBannerDataFetcher::OnAppIconFetched,
+ this));
}
-void AppBannerDataFetcher::RequestShowBanner(const SkBitmap* icon) {
+void AppBannerDataFetcher::OnAppIconFetched(const SkBitmap& bitmap) {
+ if (!is_active_) return;
+
content::WebContents* web_contents = GetWebContents();
if (!CheckFetcherIsStillAlive(web_contents)) {
Cancel();
return;
}
- if (!icon) {
+ if (bitmap.drawsNothing()) {
OutputDeveloperNotShownMessage(web_contents, kNoIconAvailable);
Cancel();
return;
}
-
RecordCouldShowBanner();
if (!CheckIfShouldShowBanner()) {
// At this point, the only possible case is that the banner has been added
@@ -370,13 +350,19 @@ void AppBannerDataFetcher::RequestShowBanner(const SkBitmap* icon) {
return;
}
- app_icon_.reset(new SkBitmap(*icon));
+ app_icon_.reset(new SkBitmap(icon));
event_request_id_ = ++gCurrentRequestID;
web_contents->GetMainFrame()->Send(
new ChromeViewMsg_AppBannerPromptRequest(
- web_contents->GetMainFrame()->GetRoutingID(),
- event_request_id_,
- GetBannerType()));
+ web_contents->GetMainFrame()->GetRoutingID(),
+ event_request_id_,
+ GetBannerType()));
+}
+
+bool AppBannerDataFetcher::IsWebAppInstalled(
+ content::BrowserContext* browser_context,
+ const GURL& start_url) {
+ return false;
}
void AppBannerDataFetcher::RecordCouldShowBanner() {

Powered by Google App Engine
This is Rietveld 408576698