Modified resources for M6 theme. This doesn't actually modify layout yet, ju...

Modified resources for M6 theme.  This doesn't actually modify layout yet, just images and identifier names.

BUG=50107
TEST=none

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

[Peter Kasting, 2010-07-28 01:31:10]

Note: I'll add the Cached Theme.pak and all the image modifications when I
really submit, but they're omitted from this version so the trybots can apply
the patch, and so it's obvious what actually needs review.

[Elliot Glaysher, 2010-07-28 17:20:31]

http://codereview.chromium.org/3035034/diff/6003/14001
File chrome/app/theme/theme_resources.grd (right):

http://codereview.chromium.org/3035034/diff/6003/14001#newcode52
chrome/app/theme/theme_resources.grd:52: <include
name="IDR_BROWSER_ACTIONS_OVERFLOW_P" file="browser_actions_overflow_p.png"
type="BINDATA" />
Unless other platforms use the win resources and vice versa, shouldn't you have
these resolve to an <if expr="os == 'win'"></if> and <if expr="os !=
'win'"></if> blocks, setting different images to the same resource names? That
would let you pull out the ifdefs in the other files and shrink the resources
file.

[Peter Kasting, 2010-07-28 18:09:40]

http://codereview.chromium.org/3035034/diff/6003/14001
File chrome/app/theme/theme_resources.grd (right):

http://codereview.chromium.org/3035034/diff/6003/14001#newcode52
chrome/app/theme/theme_resources.grd:52: <include
name="IDR_BROWSER_ACTIONS_OVERFLOW_P" file="browser_actions_overflow_p.png"
type="BINDATA" />
On 2010/07/28 17:20:31, Elliot Glaysher wrote:
> Unless other platforms use the win resources and vice versa, shouldn't you
have
> these resolve to an <if expr="os == 'win'"></if> and <if expr="os !=
> 'win'"></if> blocks, setting different images to the same resource names? That
> would let you pull out the ifdefs in the other files and shrink the resources
> file.

Good idea.  Changed to use this method.  Instead of comparing os to "win32", I
check for "win" in the os string.  This makes things work right whether the
python used in building is cygwin or not.  I also modified the only similar
checks, in chromium_strings.grd and google_chrome_strings.grd, to do the same
thing.

[Elliot Glaysher, 2010-07-28 18:32:04]

On 2010/07/28 18:09:40, Peter Kasting wrote:
> http://codereview.chromium.org/3035034/diff/6003/14001
> File chrome/app/theme/theme_resources.grd (right):
> 
> http://codereview.chromium.org/3035034/diff/6003/14001#newcode52
> chrome/app/theme/theme_resources.grd:52: <include
> name="IDR_BROWSER_ACTIONS_OVERFLOW_P" file="browser_actions_overflow_p.png"
> type="BINDATA" />
> On 2010/07/28 17:20:31, Elliot Glaysher wrote:
> > Unless other platforms use the win resources and vice versa, shouldn't you
> have
> > these resolve to an <if expr="os == 'win'"></if> and <if expr="os !=
> > 'win'"></if> blocks, setting different images to the same resource names?
That
> > would let you pull out the ifdefs in the other files and shrink the
resources
> > file.
> 
> Good idea.  Changed to use this method.  Instead of comparing os to "win32", I
> check for "win" in the os string.  This makes things work right whether the
> python used in building is cygwin or not.  I also modified the only similar
> checks, in chromium_strings.grd and google_chrome_strings.grd, to do the same
> thing.

LGTM