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

Unified Diff: chrome/browser/android/shortcut_helper.cc

Issue 38523002: Using largest favicon for shortcut when possible. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 7 years, 2 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
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/android/shortcut_helper.cc
diff --git a/chrome/browser/android/shortcut_helper.cc b/chrome/browser/android/shortcut_helper.cc
index b4b5931ca4275dd7c3c84da3de99f6401b0b6189..673379ee896547767598144811a42d5bb4f395cb 100644
--- a/chrome/browser/android/shortcut_helper.cc
+++ b/chrome/browser/android/shortcut_helper.cc
@@ -25,6 +25,7 @@
#include "ui/gfx/android/java_bitmap.h"
#include "ui/gfx/codec/png_codec.h"
#include "ui/gfx/color_analysis.h"
+#include "ui/gfx/favicon_size.h"
#include "url/gurl.h"
ShortcutBuilder::ShortcutBuilder(content::WebContents* web_contents,
@@ -71,18 +72,32 @@ void ShortcutBuilder::OnDidRetrieveWebappInformation(
// Grab the best, largest icon we can find to represent this bookmark.
// TODO(dfalcantara): Try combining with the new BookmarksHandler once its
// rewrite is further along.
- FaviconService::FaviconForURLParams favicon_params(
- profile,
- url_,
- chrome::TOUCH_PRECOMPOSED_ICON | chrome::TOUCH_ICON | chrome::FAVICON,
- 0);
-
+ std::vector<int> icon_types;
+ icon_types.push_back(chrome::FAVICON);
+ icon_types.push_back(chrome::TOUCH_PRECOMPOSED_ICON | chrome::TOUCH_ICON);
FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
profile, Profile::EXPLICIT_ACCESS);
-
- favicon_service->GetRawFaviconForURL(
- favicon_params,
- ui::SCALE_FACTOR_100P,
+ // The favicon was resized by ScaleFactor and stored in HistoryBackend.
+ // We need to get largest possible size of the resized normal favicon and
+ // pass it into GetLargestFawFaviconForURL(), otherwise, the normal favicon is
+ // returned even there is larger touch icon avaliable.
+ // In theory, the side effect is that the touch icon or precomposed touch
+ // icon whose size is smaller than kFaviconSize*ScaleFactor and larger than
+ // kFaviconSize can't be returned, but it actually not a issue as the normal
+ // favicon is 16x16, the maximum scale factor we used is 2.0f, so the maximum
+ // resized favicon is 32x32 and much smaller than touch icon whose size is
+ // from 57x57 to 144x144.
michaelbai 2013/10/23 23:20:47 I have a CL to fix this issue, the icon will not b
+ std::vector<ui::ScaleFactor> favicon_scale_factors =
+ ui::GetSupportedScaleFactors();
+ ui::ScaleFactor scale_factor = ui::SCALE_FACTOR_100P;
+ for (std::vector<ui::ScaleFactor>::const_iterator i =
+ favicon_scale_factors.begin(); i != favicon_scale_factors.end();
+ ++i) {
+ if (scale_factor < *i)
+ scale_factor = *i;
+ }
+ favicon_service->GetLargestRawFaviconForURL(profile, url_, icon_types,
+ gfx::kFaviconSize * ui::GetImageScale(scale_factor),
base::Bind(&ShortcutBuilder::FinishAddingShortcut,
base::Unretained(this)),
&cancelable_task_tracker_);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698