Add ability to store hidpi theme images in data pack

Chrome does not support hidpi themes other than the default.
However, Chrome generates images which are a combination of the toolbar icon from the default theme + a tint.
Added the ability for themes to store hidpi assets in the data pack such that the few hidpi assets which are generated are preserved when a user closes Chrome and reopens it.

Repro steps/steps for testing:
1) Run chrome with --force-device-scale-factor=2
2) Change the theme.
3) Close Chrome and relaunch with --force-device-scale-factor=2
4) Ensure that the toolbar buttons are still hidpi

Test=Manual, see instructions above
Bug=136888
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150054

[pkotwicz, 2012-07-15 19:28:17]

As a quick summary of my change. I store the 1x and the 2x theme images in the
same data pack. This is a bit different to what is done elsewhere in Chrome. The
reason I chose to do this is that a theme has  a lot of information which is the
same for 1x and 2x. (Tints for instance).
The CL introduces a raw id which is the resource id used to store bitmaps to
disk. Wherever the data of an image is extracted as RefCountedMemory, that data
is stored with a RawID. Images on the other hand are cached with a persistent
ID.

[Elliot Glaysher, 2012-07-16 17:04:06]

On 2012/07/15 19:28:17, pkotwicz wrote:
> As a quick summary of my change. I store the 1x and the 2x theme images in the
> same data pack. This is a bit different to what is done elsewhere in Chrome.
The
> reason I chose to do this is that a theme has  a lot of information which is
the
> same for 1x and 2x. (Tints for instance).
> The CL introduces a raw id which is the resource id used to store bitmaps to
> disk. Wherever the data of an image is extracted as RefCountedMemory, that
data
> is stored with a RawID. Images on the other hand are cached with a persistent
> ID.

So how does this affect theme install time? Does this mean that we're doing more
image tinting operations since we're now doing {number of theme images} *
{number of scale factors}?

[pkotwicz, 2012-07-20 01:45:55]

Sorry for taking so long. I have updated the patch such that it is thread safe.
Only the scale factors in use are loaded on the UI thread and the other scale
factors are loaded on the file thread.

[pkotwicz, 2012-07-20 18:09:04]

I think I have addressed your concerns off line. However, to answer in the code
review, this CL should not affect theme install time.
Only images for scale factors currently in use are generated on the UI thread.
Images for the other scale factors are generated on the file thread as part of
BrowserThemePack::WriteToDisk.

[Elliot Glaysher, 2012-07-20 18:17:14]

http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
File chrome/browser/themes/browser_theme_pack.cc (right):

http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
chrome/browser/themes/browser_theme_pack.cc:41: const int kThemePackVersion =
24;
Do you need to rev this? You're adding members to the data pack.

http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
chrome/browser/themes/browser_theme_pack.cc:1144: const
std::vector<ui::ScaleFactor>& scale_factors) {
Could you add a thread dcheck here? I see you modifying both the ui and file
thread caches, and I'm currently unsure where you expect this to run.

[pkotwicz, 2012-07-20 20:00:45]

http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
File chrome/browser/themes/browser_theme_pack.cc (right):

http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
chrome/browser/themes/browser_theme_pack.cc:41: const int kThemePackVersion =
24;
You're right I don't need to. Line 503 will force the theme to be rebuilt.

http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
chrome/browser/themes/browser_theme_pack.cc:1144: const
std::vector<ui::ScaleFactor>& scale_factors) {
Will do! It is supposed to run on the UI thread

[Elliot Glaysher, 2012-07-23 21:37:15]

On 2012/07/20 20:00:45, pkotwicz wrote:
>
http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
> File chrome/browser/themes/browser_theme_pack.cc (right):
> 
>
http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
> chrome/browser/themes/browser_theme_pack.cc:41: const int kThemePackVersion =
> 24;
> You're right I don't need to. Line 503 will force the theme to be rebuilt.
> 
>
http://codereview.chromium.org/10783015/diff/21001/chrome/browser/themes/brow...
> chrome/browser/themes/browser_theme_pack.cc:1144: const
> std::vector<ui::ScaleFactor>& scale_factors) {
> Will do! It is supposed to run on the UI thread

(Did you upload this?)

[Elliot Glaysher, 2012-07-25 22:37:19]

looks like you uploaded the change. lgtm.

[pkotwicz, 2012-07-26 00:36:42]

Changes as requested. erg@, thanks for reviewing!
Sky for OWNERS

[sky, 2012-08-02 20:34:16]

LGTM

[commit-bot: I haz the power, 2012-08-05 19:08:14]

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

[commit-bot: I haz the power, 2012-08-05 20:07:46]

Change committed as 150054