[Resources] Use the new product logo in several places

[Resources] Use the new product logo in several places

* Move the newer-style product_logo_name_22.png to components/resources
* Change the resource ID to better reflect file name
* Remove outdated product_logo.png in components/resources
* Remove product_logo_name_white.png and supporting codepath, because the code path was dead and unneeded(even with a dark theme).
* Remove product_logo_name_48.png because it's unused.

As a result, the size of the Chrome logo in chrome://apps, chrome://version and the old ChromeOS first run experience will change from 32px to 22px height. See the bug for more information.

BUG=697607
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

[Mathieu, 2017-03-01 21:43:35]

Description was changed from

==========
[Resources] Use the new product logo in several places

BUG=697607
TEST=manual
==========

to

==========
[Resources] Use the new product logo in several places

See bug for description.

BUG=697607
TEST=manual
==========

[Mathieu, 2017-03-01 21:43:35]

mathp@chromium.org changed reviewers:
+ estade@chromium.org, grt@chromium.org

[Mathieu, 2017-03-01 21:44:11]

Description was changed from

==========
[Resources] Use the new product logo in several places

See bug for description.

BUG=697607
TEST=manual
==========

to

==========
[Resources] Use the new product logo in several places

Doesn't need to be in components, and so we can reuse product_logo_name_22.png
in chrome resources.

See bug for description.

BUG=697607
TEST=manual
==========

[Mathieu, 2017-03-01 21:44:26]

The CQ bit was checked by mathp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-01 21:45:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-01 21:55:02]

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

[commit-bot: I haz the power, 2017-03-01 21:55:03]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mathieu, 2017-03-01 21:56:25]

Hi:

Greg: theme_resources
Evan: overall

I've sent the internal changes removing
default_{100,200}_percent/google_chrome/product_logo.png to Evan

Thanks

[Mathieu, 2017-03-01 22:03:03]

On 2017/03/01 21:56:25, Mathieu Perreault wrote:
> Hi:
> 
> Greg: theme_resources
> Evan: overall
> 
> I've sent the internal changes removing
> default_{100,200}_percent/google_chrome/product_logo.png to Evan
> 
> Thanks

Ah, not sure this is possible because of 

https://cs.chromium.org/chromium/src/components/version_ui/resources/about_ve...

apparently iOS cannot use chrome://theme directly so we may need to keep the
logo in components/... Oh well. Will rework this change.

[Mathieu, 2017-03-01 22:03:09]

mathp@chromium.org changed reviewers:
- estade@chromium.org, grt@chromium.org

[Evan Stade, 2017-03-01 22:14:06]

estade@chromium.org changed reviewers:
+ estade@chromium.org

[Evan Stade, 2017-03-01 22:14:06]

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

https://codereview.chromium.org/2726793004/diff/20001/chrome/app/theme/theme_...
chrome/app/theme/theme_resources.grd:352: <structure type="chrome_scaled_image"
name="IDR_PRODUCT_LOGO" file="chromium/product_logo_name_22.png" />
this doesn't seem like the best name for the resource (it's not just the logo,
it's the logo and the name and a particular size), can we make the IDR
identifier match the file name?

https://codereview.chromium.org/2726793004/diff/20001/components/resources/co...
File components/resources/components_scaled_resources.grd (left):

https://codereview.chromium.org/2726793004/diff/20001/components/resources/co...
components/resources/components_scaled_resources.grd:20: <part
file="version_ui_scaled_resources.grdp" />
why are these here in the first place? Is it not because they're used in ios or
something?

[Mathieu, 2017-03-02 14:27:36]

Description was changed from

==========
[Resources] Use the new product logo in several places

Doesn't need to be in components, and so we can reuse product_logo_name_22.png
in chrome resources.

See bug for description.

BUG=697607
TEST=manual
==========

to

==========
[Resources] Use the new product logo in several places

Doesn't need to be in components, and so we can reuse product_logo_name_22.png
in chrome resources.

See bug for description.

BUG=697607
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Mathieu, 2017-03-02 14:31:56]

Patchset #3 (id:40001) has been deleted

[Mathieu, 2017-03-02 14:33:40]

Description was changed from

==========
[Resources] Use the new product logo in several places

Doesn't need to be in components, and so we can reuse product_logo_name_22.png
in chrome resources.

See bug for description.

BUG=697607
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Resources] Use the new product logo in several places

* Move the newer-style product_logo_name_22.png to components/resources
* Change the resource ID to better reflect file name
* Remove outdated product_logo.png in components/resources
* Remove product_logo_name_white.png and supporting codepath, because the code
path was dead and unneeded(even with a dark theme).
* Remove product_logo_name_48.png because it's unused.

See bug even more information.

BUG=697607
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Mathieu, 2017-03-02 14:36:08]

Hi Evan, 

So it had to stay in components (iOS). I found several other issues that I
detailed in the bug description. 

Once we converge to a good change I will prepare the 4 google_chrome internal
changes.

PTAL

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

https://codereview.chromium.org/2726793004/diff/20001/chrome/app/theme/theme_...
chrome/app/theme/theme_resources.grd:352: <structure type="chrome_scaled_image"
name="IDR_PRODUCT_LOGO" file="chromium/product_logo_name_22.png" />
On 2017/03/01 22:14:06, Evan Stade wrote:
> this doesn't seem like the best name for the resource (it's not just the logo,
> it's the logo and the name and a particular size), can we make the IDR
> identifier match the file name? 

Done

https://codereview.chromium.org/2726793004/diff/20001/components/resources/co...
File components/resources/components_scaled_resources.grd (left):

https://codereview.chromium.org/2726793004/diff/20001/components/resources/co...
components/resources/components_scaled_resources.grd:20: <part
file="version_ui_scaled_resources.grdp" />
On 2017/03/01 22:14:06, Evan Stade wrote:
> why are these here in the first place? Is it not because they're used in ios
or
> something?

Yes, our messages crossed. I've uploaded a new patchset which solves this

[Mathieu, 2017-03-02 14:39:56]

On 2017/03/02 14:36:08, Mathieu Perreault wrote:
> Hi Evan, 
> 
> So it had to stay in components (iOS). I found several other issues that I
> detailed in the bug description. 
> 
> Once we converge to a good change I will prepare the 4 google_chrome internal
> changes.
> 
> PTAL
> 
>
https://codereview.chromium.org/2726793004/diff/20001/chrome/app/theme/theme_...
> File chrome/app/theme/theme_resources.grd (right):
> 
>
https://codereview.chromium.org/2726793004/diff/20001/chrome/app/theme/theme_...
> chrome/app/theme/theme_resources.grd:352: <structure
type="chrome_scaled_image"
> name="IDR_PRODUCT_LOGO" file="chromium/product_logo_name_22.png" />
> On 2017/03/01 22:14:06, Evan Stade wrote:
> > this doesn't seem like the best name for the resource (it's not just the
logo,
> > it's the logo and the name and a particular size), can we make the IDR
> > identifier match the file name? 
> 
> Done
> 
>
https://codereview.chromium.org/2726793004/diff/20001/components/resources/co...
> File components/resources/components_scaled_resources.grd (left):
> 
>
https://codereview.chromium.org/2726793004/diff/20001/components/resources/co...
> components/resources/components_scaled_resources.grd:20: <part
> file="version_ui_scaled_resources.grdp" />
> On 2017/03/01 22:14:06, Evan Stade wrote:
> > why are these here in the first place? Is it not because they're used in ios
> or
> > something?
> 
> Yes, our messages crossed. I've uploaded a new patchset which solves this

BTW will land this after branch

[Mathieu, 2017-03-08 14:07:18]

On 2017/03/02 14:39:56, Mathieu Perreault wrote:
> On 2017/03/02 14:36:08, Mathieu Perreault wrote:
> > Hi Evan, 
> > 
> > So it had to stay in components (iOS). I found several other issues that I
> > detailed in the bug description. 
> > 
> > Once we converge to a good change I will prepare the 4 google_chrome
internal
> > changes.
> > 
> > PTAL
> > 
> >
>
https://codereview.chromium.org/2726793004/diff/20001/chrome/app/theme/theme_...
> > File chrome/app/theme/theme_resources.grd (right):
> > 
> >
>
https://codereview.chromium.org/2726793004/diff/20001/chrome/app/theme/theme_...
> > chrome/app/theme/theme_resources.grd:352: <structure
> type="chrome_scaled_image"
> > name="IDR_PRODUCT_LOGO" file="chromium/product_logo_name_22.png" />
> > On 2017/03/01 22:14:06, Evan Stade wrote:
> > > this doesn't seem like the best name for the resource (it's not just the
> logo,
> > > it's the logo and the name and a particular size), can we make the IDR
> > > identifier match the file name? 
> > 
> > Done
> > 
> >
>
https://codereview.chromium.org/2726793004/diff/20001/components/resources/co...
> > File components/resources/components_scaled_resources.grd (left):
> > 
> >
>
https://codereview.chromium.org/2726793004/diff/20001/components/resources/co...
> > components/resources/components_scaled_resources.grd:20: <part
> > file="version_ui_scaled_resources.grdp" />
> > On 2017/03/01 22:14:06, Evan Stade wrote:
> > > why are these here in the first place? Is it not because they're used in
ios
> > or
> > > something?
> > 
> > Yes, our messages crossed. I've uploaded a new patchset which solves this
> 
> BTW will land this after branch

Comments from UX addressed, PTAL.

[Evan Stade, 2017-03-08 17:56:30]

can you update the CL description to reflect the fact you're changing the size
of the logo in several places?

https://codereview.chromium.org/2726793004/diff/100001/chrome/browser/themes/...
File chrome/browser/themes/theme_service.cc (left):

https://codereview.chromium.org/2726793004/diff/100001/chrome/browser/themes/...
chrome/browser/themes/theme_service.cc:749: if (id == IDR_PRODUCT_LOGO &&
ntp_alternate != 0)
are you sure this code is dead? It seems to be hit when I open the app page with
a custom theme.

If this code is dead, we also don't need NTP_LOGO_ALTERNATE any more do we?

[Mathieu, 2017-03-08 18:06:35]

Description was changed from

==========
[Resources] Use the new product logo in several places

* Move the newer-style product_logo_name_22.png to components/resources
* Change the resource ID to better reflect file name
* Remove outdated product_logo.png in components/resources
* Remove product_logo_name_white.png and supporting codepath, because the code
path was dead and unneeded(even with a dark theme).
* Remove product_logo_name_48.png because it's unused.

See bug even more information.

BUG=697607
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Resources] Use the new product logo in several places

* Move the newer-style product_logo_name_22.png to components/resources
* Change the resource ID to better reflect file name
* Remove outdated product_logo.png in components/resources
* Remove product_logo_name_white.png and supporting codepath, because the code
path was dead and unneeded(even with a dark theme).
* Remove product_logo_name_48.png because it's unused.

As a result, the size of the Chrome logo in chrome://apps, chrome://version and
the old ChromeOS first run experience will change from 32px to 22px height. See
the bug for more information.

BUG=697607
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Mathieu, 2017-03-08 18:20:22]

Thanks, updated the description.

https://codereview.chromium.org/2726793004/diff/100001/chrome/browser/themes/...
File chrome/browser/themes/theme_service.cc (left):

https://codereview.chromium.org/2726793004/diff/100001/chrome/browser/themes/...
chrome/browser/themes/theme_service.cc:749: if (id == IDR_PRODUCT_LOGO &&
ntp_alternate != 0)
On 2017/03/08 17:56:30, Evan Stade wrote:
> are you sure this code is dead? It seems to be hit when I open the app page
with
> a custom theme.
> 
> If this code is dead, we also don't need NTP_LOGO_ALTERNATE any more do we?
> 

I am certain. I instrumented the codepaths in this file and theme_source.cc and
the image fetch goes through ThemeSource::SendThemeBitmap (which just gets it
from the resource bundle). 

NTP_LOGO_ALTERNATE as a property is still used by the current NTP [1].

[1]
https://cs.chromium.org/chromium/src/chrome/browser/search/instant_service.cc...

[Evan Stade, 2017-03-09 03:10:46]

On 2017/03/08 18:20:22, Mathieu Perreault wrote:
> Thanks, updated the description.
> 
>
https://codereview.chromium.org/2726793004/diff/100001/chrome/browser/themes/...
> File chrome/browser/themes/theme_service.cc (left):
> 
>
https://codereview.chromium.org/2726793004/diff/100001/chrome/browser/themes/...
> chrome/browser/themes/theme_service.cc:749: if (id == IDR_PRODUCT_LOGO &&
> ntp_alternate != 0)
> On 2017/03/08 17:56:30, Evan Stade wrote:
> > are you sure this code is dead? It seems to be hit when I open the app page
> with
> > a custom theme.
> > 
> > If this code is dead, we also don't need NTP_LOGO_ALTERNATE any more do we?
> > 
> 
> I am certain. I instrumented the codepaths in this file and theme_source.cc
and
> the image fetch goes through ThemeSource::SendThemeBitmap (which just gets it
> from the resource bundle). 

When I add logging to the conditional you deleted, I find that it is hit for
certain themes.

> 
> NTP_LOGO_ALTERNATE as a property is still used by the current NTP [1].
> 
> [1]
>
https://cs.chromium.org/chromium/src/chrome/browser/search/instant_service.cc...

I don't see where the chrome logo appears in the ntp.