Download and store large favicon for Android.

Download and store large favicon for Android.

BUG=298446
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=231074

[michaelbai, 2013-10-23 21:46:24]

PTAL, side by side diff didn't work, though sync many times.

https://codereview.chromium.org/28943004/diff/1/chrome/browser/favicon/favico...
File chrome/browser/favicon/favicon_handler.cc (right):

https://codereview.chromium.org/28943004/diff/1/chrome/browser/favicon/favico...
chrome/browser/favicon/favicon_handler.cc:38: 
Peter pointed out this could have performance issue when encoding large favicon.
This size is a little larger than kTouchIconSize, I will collect data to see if
it is really a issue.

I was thinking to use feature flag like kDownloadLargestFavicon, but
kFaviconMaximumSize also needs defined, it is not like a good design.

[pkotwicz, 2013-10-24 02:15:04]

LGTM is you have thought of all the corner cases

NOTE: This change affects which favicon gets synced to other clients for any
bookmarks (eg for http://www.reddit.com). Now the full alien will get synced to
other clients

[sky, 2013-10-24 15:30:16]

LGTM

https://codereview.chromium.org/28943004/diff/1/chrome/browser/favicon/favico...
File chrome/browser/favicon/favicon_handler.cc (right):

https://codereview.chromium.org/28943004/diff/1/chrome/browser/favicon/favico...
chrome/browser/favicon/favicon_handler.cc:38: 
I say nuke this field since you end up with platform specific code anyway.

https://codereview.chromium.org/28943004/diff/1/chrome/browser/favicon/favico...
chrome/browser/favicon/favicon_handler.cc:77: 
nit: remove comment. not helpful since you have notreached

[michaelbai, 2013-10-24 21:44:21]

https://codereview.chromium.org/28943004/diff/1/chrome/browser/favicon/favico...
File chrome/browser/favicon/favicon_handler.cc (right):

https://codereview.chromium.org/28943004/diff/1/chrome/browser/favicon/favico...
chrome/browser/favicon/favicon_handler.cc:38: 
On 2013/10/24 15:30:17, sky wrote:
> I say nuke this field since you end up with platform specific code anyway.

Done.

https://codereview.chromium.org/28943004/diff/1/chrome/browser/favicon/favico...
chrome/browser/favicon/favicon_handler.cc:77: 
On 2013/10/24 15:30:17, sky wrote:
> nit: remove comment. not helpful since you have notreached

Done.

[commit-bot: I haz the power, 2013-10-24 21:47:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/28943004/140001

[commit-bot: I haz the power, 2013-10-24 22:42:12]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-25 03:50:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/28943004/420001

[commit-bot: I haz the power, 2013-10-25 10:18:44]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2013-10-25 16:55:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/28943004/730001

[commit-bot: I haz the power, 2013-10-25 19:16:43]

Change committed as 231074