MacViews: Fix missing browser assets

MacViews: Fix missing browser assets

The MacViews browser were missing infobar and background tab assets.
mac_views_browser grit define is added so that we can include infobar assets.
Don't remap theme image ids so that Mac's background tab assets can be
found.
Map IDR_THEME_TAB_BACKGROUND_INCOGNITO to the same asset as
IDR_THEME_TAB_BACKGROUND.

BUG=425229
Committed: https://crrev.com/c306fb9a8d5179f34589d6ab330a71ebd40eb073
Cr-Commit-Position: refs/heads/master@{#321602}

[Andre, 2015-03-12 17:36:19]

andresantoso@chromium.org changed reviewers:
+ tapted@chromium.org

[Andre, 2015-03-12 17:36:20]

Trent PTAL.

[tapted, 2015-03-12 18:16:46]

lgtm % nits

https://codereview.chromium.org/983823004/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/983823004/diff/20001/build/common.gypi#newcod...
build/common.gypi:653: # Whether the browser is non-native (using Views Toolkit)
on Mac.
nit: I wouldn't say "non-native", I would just say 

# Whether the entire browser uses toolkit-views on Mac instead of Cocoa.

(same in gn)

https://codereview.chromium.org/983823004/diff/20001/chrome/app/theme/theme_r...
File chrome/app/theme/theme_resources.grd (right):

https://codereview.chromium.org/983823004/diff/20001/chrome/app/theme/theme_r...
chrome/app/theme/theme_resources.grd:1104: <structure type="chrome_scaled_image"
name="IDR_THEME_TAB_BACKGROUND_INCOGNITO" file="mac/theme_default_inactive.png"
/>
hm, is it weird to provide the same resource with two names? I guess not. This
is consistent with the ash stuff above.

https://codereview.chromium.org/983823004/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/theme_image_mapper.cc (right):

https://codereview.chromium.org/983823004/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/theme_image_mapper.cc:12: #if !defined(OS_CHROMEOS) &&
!defined(OS_MACOSX)
Perhaps a comment here, since we were a bit confused.

E.g. ~"On platforms where there is both Ash and a desktop browser, provide
DESKTOP resources for the latter."

[Andre, 2015-03-12 21:15:41]

Patchset #3 (id:40001) has been deleted

[Andre, 2015-03-12 21:15:45]

Patchset #3 (id:40001) has been deleted

[Andre, 2015-03-12 21:16:41]

https://codereview.chromium.org/983823004/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/983823004/diff/20001/build/common.gypi#newcod...
build/common.gypi:653: # Whether the browser is non-native (using Views Toolkit)
on Mac.
On 2015/03/12 18:16:46, tapted wrote:
> nit: I wouldn't say "non-native", I would just say 
> 
> # Whether the entire browser uses toolkit-views on Mac instead of Cocoa.
> 
> (same in gn)

Done.

https://codereview.chromium.org/983823004/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/theme_image_mapper.cc (right):

https://codereview.chromium.org/983823004/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/theme_image_mapper.cc:12: #if !defined(OS_CHROMEOS) &&
!defined(OS_MACOSX)
On 2015/03/12 18:16:46, tapted wrote:
> Perhaps a comment here, since we were a bit confused.
> 
> E.g. ~"On platforms where there is both Ash and a desktop browser, provide
> DESKTOP resources for the latter."

Done.

[Andre, 2015-03-12 21:28:33]

andresantoso@chromium.org changed reviewers:
+ thakis@chromium.org

[Andre, 2015-03-12 21:28:33]

+Nico for OWNERS.

[Nico, 2015-03-12 21:30:59]

https://codereview.chromium.org/983823004/diff/60001/chrome/app/theme/theme_r...
File chrome/app/theme/theme_resources.grd (right):

https://codereview.chromium.org/983823004/diff/60001/chrome/app/theme/theme_r...
chrome/app/theme/theme_resources.grd:1104: <structure type="chrome_scaled_image"
name="IDR_THEME_TAB_BACKGROUND_INCOGNITO" file="mac/theme_default_inactive.png"
/>
Is this needed in non-views mode? Doesn't look like it?

[Andre, 2015-03-12 22:38:44]

https://codereview.chromium.org/983823004/diff/60001/chrome/app/theme/theme_r...
File chrome/app/theme/theme_resources.grd (right):

https://codereview.chromium.org/983823004/diff/60001/chrome/app/theme/theme_r...
chrome/app/theme/theme_resources.grd:1104: <structure type="chrome_scaled_image"
name="IDR_THEME_TAB_BACKGROUND_INCOGNITO" file="mac/theme_default_inactive.png"
/>
On 2015/03/12 21:30:59, Nico (traveling) wrote:
> Is this needed in non-views mode? Doesn't look like it?

It is not used by Cocoa browser.
It is the same asset as IDR_THEME_TAB_BACKGROUND, but I just found out
that they don't get deduped so the pak file did grow.
ChromeOS has the same problem above, it would include the same image
twice.
We can't just map them to the same resource on Mac and ChromeOS,
because a custom theme might provide different resources for them.
I've added a mac_views_browser conditional around this.
Ideally, the packer should dedupe and avoid multiple copies of the same
image. Do you know if there's a bug/plan for that?

[Andre, 2015-03-17 15:55:02]

Nico, ping?

[Andre, 2015-03-17 21:53:56]

IDR_THEME_FRAME* is actually not used when using default theme on Mac.
We let NSThemeFrame draw the background when not incognito, and we use
GRADIENT_FRAME_INCOGNITO when incognito (see FramedBrowserWindow).
In patchset 5, I excluded these assets on Mac to reduce the .pak size by
about 8KB.

[Nico, 2015-03-19 20:57:02]

On 2015/03/12 22:38:44, Andre wrote:
>
https://codereview.chromium.org/983823004/diff/60001/chrome/app/theme/theme_r...
> File chrome/app/theme/theme_resources.grd (right):
> 
>
https://codereview.chromium.org/983823004/diff/60001/chrome/app/theme/theme_r...
> chrome/app/theme/theme_resources.grd:1104: <structure
type="chrome_scaled_image"
> name="IDR_THEME_TAB_BACKGROUND_INCOGNITO"
file="mac/theme_default_inactive.png"
> />
> On 2015/03/12 21:30:59, Nico (traveling) wrote:
> > Is this needed in non-views mode? Doesn't look like it?
> 
> It is not used by Cocoa browser.
> It is the same asset as IDR_THEME_TAB_BACKGROUND, but I just found out
> that they don't get deduped so the pak file did grow.
> ChromeOS has the same problem above, it would include the same image
> twice.
> We can't just map them to the same resource on Mac and ChromeOS,
> because a custom theme might provide different resources for them.
> I've added a mac_views_browser conditional around this.
> Ideally, the packer should dedupe and avoid multiple copies of the same
> image. Do you know if there's a bug/plan for that?

I'm not aware of one. http://crbug.com/338759 is kind of in the same area
though.

[Nico, 2015-03-19 20:59:31]

lgtm

[Andre, 2015-03-19 23:00:15]

The CQ bit was checked by andresantoso@chromium.org

[Andre, 2015-03-19 23:00:15]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/983823004/#ps100001
(title: "IDR_THEME_FRAME* not used on Mac")

[commit-bot: I haz the power, 2015-03-19 23:00:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/983823004/100001

[commit-bot: I haz the power, 2015-03-20 01:43:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-20 01:43:14]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[Andre, 2015-03-20 16:51:47]

Uploaded patchset 6 to undo patchset 5 due to test failures.
When a theme does not provide frame images, the theme service
generates them from the default ones. It would be weird if a theme
does not provide them since the default ones are Windows blue, but
I can't know for sure.
See
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/the...

[Nico, 2015-03-20 17:02:56]

lgtm

[Andre, 2015-03-20 17:43:32]

The CQ bit was checked by andresantoso@chromium.org

[Andre, 2015-03-20 17:43:33]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/983823004/#ps120001
(title: "Undo patchset 5")

[commit-bot: I haz the power, 2015-03-20 17:43:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/983823004/120001

[commit-bot: I haz the power, 2015-03-20 18:16:47]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2015-03-20 18:17:17]

Patchset 6 (id:??) landed as
https://crrev.com/c306fb9a8d5179f34589d6ab330a71ebd40eb073
Cr-Commit-Position: refs/heads/master@{#321602}