Add Media Router WebUI icons.

Add Media Router WebUI icons.

BUG=464222
Committed: https://crrev.com/541203fdae95da5116a1a14acd8aeb39994de264
Cr-Commit-Position: refs/heads/master@{#321850}

[apacible, 2015-03-21 01:40:29]

apacible@chromium.org changed reviewers:
+ mfoltz@chromium.org

[apacible, 2015-03-21 01:43:49]

apacible@chromium.org changed reviewers:
+ jhawkins@chromium.org

[apacible, 2015-03-21 01:45:30]

mfoltz, PTAL.

+jhawkins@ as OWNER: chrome/browser/browser_resources.grd

Thanks!

[James Hawkins, 2015-03-23 14:56:03]

On 2015/03/21 01:45:30, apacible wrote:
> mfoltz, PTAL.
> 
> +jhawkins@ as OWNER: chrome/browser/browser_resources.grd
> 
> Thanks!

Can you provide a link to documentation somewhere about what Media Router is?

[apacible, 2015-03-23 15:26:07]

On 2015/03/23 14:56:03, James Hawkins wrote:
> On 2015/03/21 01:45:30, apacible wrote:
> > mfoltz, PTAL.
> > 
> > +jhawkins@ as OWNER: chrome/browser/browser_resources.grd
> > 
> > Thanks!
> 
> Can you provide a link to documentation somewhere about what Media Router is?

Our design doc is here:
http://www.chromium.org/developers/design-documents/media-router

We have README files in c/b/media/router, c/b/ui/views/media_router, and I have
a patch out for c/b/ui/webui/media_router. Should I add one to
c/b/resources/media_router and/or add a comment in the browser_resources.grd
file as well?

[James Hawkins, 2015-03-23 15:59:53]

On 2015/03/23 15:26:07, apacible wrote:
> On 2015/03/23 14:56:03, James Hawkins wrote:
> > On 2015/03/21 01:45:30, apacible wrote:
> > > mfoltz, PTAL.
> > > 
> > > +jhawkins@ as OWNER: chrome/browser/browser_resources.grd
> > > 
> > > Thanks!
> > 
> > Can you provide a link to documentation somewhere about what Media Router
is?
> 
> Our design doc is here:
> http://www.chromium.org/developers/design-documents/media-router
> 
> We have README files in c/b/media/router, c/b/ui/views/media_router, and I
have
> a patch out for c/b/ui/webui/media_router. Should I add one to
> c/b/resources/media_router and/or add a comment in the browser_resources.grd
> file as well?

No, I just want to make sure I know what I'm reviewing :-)  Can you send me a
link to mocks for the WebUI?

[James Hawkins, 2015-03-23 16:09:37]

I feel like we almost certainly have some of these images checked in already. 
Have you checked?

[apacible, 2015-03-23 16:18:57]

On 2015/03/23 16:09:37, James Hawkins wrote:
> I feel like we almost certainly have some of these images checked in already. 
> Have you checked?

Yup, I searched through the browser_resources.grd and did code searches for the
icons. There are some similarly named ones (e.g. play, down arrow) but they're
not the same icon.

[James Hawkins, 2015-03-23 16:20:47]

OK, LGTM

[mark a. foltz, 2015-03-23 17:49:57]

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
File chrome/browser/browser_resources.grd (right):

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:452: <include
name="IDR_MEDIA_ROUTER_CHROMECAST_32_ICON"
file="resources\media_router\elements\icon\chromecast-icon-32.png"
type="BINDATA" />
How does the 32.png look on a Retina display?

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:454: <include name="IDR_CLOSE_GRAY_2X_ICON"
file="resources\media_router\elements\icon\close-gray2x.png" type="BINDATA" />
I would like to be consistent with file naming, using 2x for the high res
versions, since the pixel sizes may vary.

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:457: <include
name="IDR_DOWN_ARROW_SHOWING_ICON"
file="resources\media_router\elements\icon\down-arrow-showing.png"
type="BINDATA" />
This PNG looked corrupted in Reitveld?  Or maybe just looks odd out of context.

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:462: <include name="IDR_MUTE_ICON"
file="resources\media_router\elements\icon\mute.png" type="BINDATA" />
Per offline convo, since we are not planning on supporting generic media
controls in the first iteration, we can hold out the supporting resources
(mute,pause,play,unmute)

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:468: <include name="IDR_SAD_FACE_ICON"
file="resources\media_router\elements\icon\sad-face.png" type="BINDATA" />
Does this have a 2X version?

[apacible, 2015-03-23 18:22:48]

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
File chrome/browser/browser_resources.grd (right):

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:452: <include
name="IDR_MEDIA_ROUTER_CHROMECAST_32_ICON"
file="resources\media_router\elements\icon\chromecast-icon-32.png"
type="BINDATA" />
On 2015/03/23 17:49:57, mark a. foltz wrote:
> How does the 32.png look on a Retina display?  

It's not as sharp as it could be. This goes for most icons, actually. I'll talk
to Jonny about making a set that looks nice on a Retina display.

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:454: <include name="IDR_CLOSE_GRAY_2X_ICON"
file="resources\media_router\elements\icon\close-gray2x.png" type="BINDATA" />
On 2015/03/23 17:49:57, mark a. foltz wrote:
> I would like to be consistent with file naming, using 2x for the high res
> versions, since the pixel sizes may vary.

Done.

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:457: <include
name="IDR_DOWN_ARROW_SHOWING_ICON"
file="resources\media_router\elements\icon\down-arrow-showing.png"
type="BINDATA" />
On 2015/03/23 17:49:57, mark a. foltz wrote:
> This PNG looked corrupted in Reitveld?  Or maybe just looks odd out of
context.

It looks a lot better on a file explorer and image preview than on Reitveld.
It's a downward white arrow with blackened lower left and lower right corners.
(Inverse of down-arrow-hover.)

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:462: <include name="IDR_MUTE_ICON"
file="resources\media_router\elements\icon\mute.png" type="BINDATA" />
On 2015/03/23 17:49:57, mark a. foltz wrote:
> Per offline convo, since we are not planning on supporting generic media
> controls in the first iteration, we can hold out the supporting resources
> (mute,pause,play,unmute)

Removed mute, pause, play, unmute.

https://codereview.chromium.org/1028503006/diff/1/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:468: <include name="IDR_SAD_FACE_ICON"
file="resources\media_router\elements\icon\sad-face.png" type="BINDATA" />
On 2015/03/23 17:49:57, mark a. foltz wrote:
> Does this have a 2X version?

Not at the moment. I'll talk to Jonny about that.

[mark a. foltz, 2015-03-23 19:52:21]

LGTM

Yeah, the down arrow looked weird on a white background.

Feel free to add the hi-res sad face icon (if there is one0 in later patch.

[apacible, 2015-03-23 20:01:20]

The CQ bit was checked by apacible@chromium.org

[apacible, 2015-03-23 20:01:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
jhawkins@chromium.org
Link to the patchset: https://codereview.chromium.org/1028503006/#ps20001
(title: " ")

[apacible, 2015-03-23 20:01:44]

On 2015/03/23 19:52:21, mark a. foltz wrote:
> LGTM
> 
> Yeah, the down arrow looked weird on a white background.
> 
> Feel free to add the hi-res sad face icon (if there is one0 in later patch.

Thanks! Jonny's looking into the Retina-friendly icons now.

[commit-bot: I haz the power, 2015-03-23 20:02:14]

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

[commit-bot: I haz the power, 2015-03-23 21:11:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-23 21:12:11]

Patchset 2 (id:??) landed as
https://crrev.com/541203fdae95da5116a1a14acd8aeb39994de264
Cr-Commit-Position: refs/heads/master@{#321850}