Ensure that favicons always have 1x representation regardless of whether the platform supports it

This CL ensures that favicons always
1) Stores a 1x representation in history
2) Returns a 1x representation upon calling FaviconService::GetFaviconImageForURL() such that the 1x favicon can be pushed to sync.

BUG=160503
Test=Manual, see instructions below
1) Go to http://www.corp.google.com/~pkotwicz/favicon_small_big/test.html on an iOS device and bookmark it. On the iOS device, the favicon should be red.
2) On a windows device, make sure that the favicon which is received via sync is blue

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168079

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168233

[pkotwicz, 2012-11-13 21:14:07]

noyau@, can you please take a look in terms of the general approach before I
send it off to sky@ to look at?

[noyau (Ping after 24h), 2012-11-14 10:02:44]

On 2012/11/13 21:14:07, pkotwicz wrote:
> noyau@, can you please take a look in terms of the general approach before I
> send it off to sky@ to look at?

This is fine, a good approach to avoid 100p as a supported scale factor on iOS
outside the scope of favicons (actually, you should remove 100p from
ui::GetSupportedScaleFactors() on iOS in that CL, there is no reason to keep it
there anymore).

However the returned image is still a gfx::Image backed up by an multi scale
factor ImageSkia, so this doesn't solve the performance issue of the conversion
back to png done by sync.

[pkotwicz, 2012-11-14 14:49:05]

I will write a different CL for the perf problem.

[pkotwicz, 2012-11-15 17:10:07]

sky@, can you please take a look? This CL removes 1x from the list of supported
scale factors on iOS (added in https://chromiumcodereview.appspot.com/11293147).
It maintains 1x a scale factor used for favicons though as the 1x favicon needs
to be sent to sync. (That's the change in FaviconUtil)

Cleanup in this CL:
- Removing passing a vector of scale factors where not needed
- Replacing ui::GetSupportedScaleFactors() with ui::GetMaxScaleFactor() where
appropriate

[sky, 2012-11-15 18:14:54]

https://codereview.chromium.org/11360233/diff/5001/chrome/browser/favicon/fav...
File chrome/browser/favicon/favicon_util.cc (right):

https://codereview.chromium.org/11360233/diff/5001/chrome/browser/favicon/fav...
chrome/browser/favicon/favicon_util.cc:17: static std::vector<ui::ScaleFactor>*
favicon_scale_factors =
Is it really worth caching this? If we do, it means any tests that reset the
scale factors and then call this would have problems.

[pkotwicz, 2012-11-15 18:47:57]

Good point. Removed the caching.

[sky, 2012-11-15 19:20:59]

LGTM

[commit-bot: I haz the power, 2012-11-15 20:23:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/11360233/12001

[commit-bot: I haz the power, 2012-11-15 23:41:11]

Change committed as 168079

[noyau (Ping after 24h), 2012-11-15 23:59:46]

https://chromiumcodereview.appspot.com/11360233/diff/12001/ui/base/layout.cc
File ui/base/layout.cc (right):

https://chromiumcodereview.appspot.com/11360233/diff/12001/ui/base/layout.cc#...
ui/base/layout.cc:89: supported_scale_factors->push_back(SCALE_FACTOR_200P);
Wow, this is now broken. On non retina iphone this will return an empty
scale_factor list...

This test need an else clause pushing SCALE_FACTOR_100P for non retina devices.

[pkotwicz, 2012-11-16 15:26:31]

Relanding in order to properly add SCALE_FACTOR_100P for non retina iOS devices.

[commit-bot: I haz the power, 2012-11-16 15:27:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/11360233/12004

[commit-bot: I haz the power, 2012-11-16 17:25:44]

Change committed as 168233