Web MIDI: Implement icon image and bubble

Web MIDI: Implement icon image and bubble

The last change to implement content settings for Web MIDI.
With this change, the page will have an icon which indicate
the page has a full control of MIDI devices or not.

XIB changes:
* add ContentBlockedMIDISysEx.xib. This is almost the same thing with
ContentBlockedGeolocation.xib except for NSButtonCell's content IDS id.

BUG=257618
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216650

[Takashi Toyoshima, 2013-08-07 12:32:41]

Hi Bernhard,

Can you take a look?
This change completes Web MIDI content settings implementation.


Miranda:

I added you as a nibs/OWNERS since this change will add one xib file there.


Thanks in advance.

[Miranda Callahan, 2013-08-07 12:51:24]

On 2013/08/07 12:32:41, Takashi Toyoshima (chromium) wrote:
> Hi Bernhard,
> 
> Can you take a look?
> This change completes Web MIDI content settings implementation.
> 
> 
> Miranda:
> 
> I added you as a nibs/OWNERS since this change will add one xib file there.
> 
> 
> Thanks in advance.

Hi Takashi -- passing over to rsesek to review this nibs change; I should not
still be an OWNER here.

[Bernhard Bauer, 2013-08-07 13:16:30]

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/genera...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/genera...
chrome/app/generated_resources.grd:14665: +      This page has a full control of
MIDI devices.
"Control" is an uncountable noun, therefore no "a".

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/genera...
chrome/app/generated_resources.grd:14677: +      Settings will be cleared on
next reload
Add a period at the end of the sentence?

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/nibs/C...
File chrome/app/nibs/ContentBlockedMIDISysEx.xib (right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/nibs/C...
chrome/app/nibs/ContentBlockedMIDISysEx.xib:113: <string
key="NSContents">^IDS_MIDI_SYSEX_BUBBLE_MANAGE_LINK</string>
Can you describe the changes you made to the XIB file in plain text in the CL
description?

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/browser/ui...
File chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
(right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/browser/ui...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:644:
GroupIterator;
Does the typedef here really simplify anything if it's only used once?

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/chrome_nib...
File chrome/chrome_nibs.gypi (right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/chrome_nib...
chrome/chrome_nibs.gypi:22: 'app/nibs/ContentBlockedMedia.xib',
Alphabetize please.

[Robert Sesek, 2013-08-07 14:11:56]

Please enumerate the changes in the new XIB file (it's branched/copied from an
existing one it looks like -- mention that), as documented here:
http://www.chromium.org/developers/design-documents/mac-xib-files

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/browser/ui...
File chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
(right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/browser/ui...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:607:
if(content.custom_link_enabled) {
nit: space before (

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/browser/ui...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:644:
GroupIterator;
On 2013/08/07 13:16:31, Bernhard Bauer wrote:
> Does the typedef here really simplify anything if it's only used once?

Also, in Mac code, you can use C++11 auto.

[Takashi Toyoshima, 2013-08-08 06:44:34]

Thank you.
I Uploaded Patch Set 32.

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/genera...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/genera...
chrome/app/generated_resources.grd:14665: +      This page has a full control of
MIDI devices.
Thanks.
Fixed this and following three same nits.

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/genera...
chrome/app/generated_resources.grd:14677: +      Settings will be cleared on
next reload
Add a period on these two sentence, and similar two sentence for geolocation to
be consistent.

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/nibs/C...
File chrome/app/nibs/ContentBlockedMIDISysEx.xib (right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/app/nibs/C...
chrome/app/nibs/ContentBlockedMIDISysEx.xib:113: <string
key="NSContents">^IDS_MIDI_SYSEX_BUBBLE_MANAGE_LINK</string>
On 2013/08/07 13:16:31, Bernhard Bauer wrote:
> Can you describe the changes you made to the XIB file in plain text in the CL
> description?

Done.

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/browser/ui...
File chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
(right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/browser/ui...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:607:
if(content.custom_link_enabled) {
On 2013/08/07 14:11:56, rsesek wrote:
> nit: space before (

Done.

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/browser/ui...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:644:
GroupIterator;
Oh, I'm happy to use C++11 auto here :)

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/chrome_nib...
File chrome/chrome_nibs.gypi (right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/chrome_nib...
chrome/chrome_nibs.gypi:22: 'app/nibs/ContentBlockedMedia.xib',
Oh, these are MI, Me, and Mi. It looks a little complicated, but can I trust
unix sort command?

$ (echo MIDI;echo Media;echo Mixed)|sort
Media
MIDI
Mixed

[Bernhard Bauer, 2013-08-08 07:29:49]

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/chrome_nib...
File chrome/chrome_nibs.gypi (right):

https://chromiumcodereview.appspot.com/18004004/diff/121001/chrome/chrome_nib...
chrome/chrome_nibs.gypi:22: 'app/nibs/ContentBlockedMedia.xib',
On 2013/08/08 06:44:34, Takashi Toyoshima (chromium) wrote:
> Oh, these are MI, Me, and Mi. It looks a little complicated, but can I trust
> unix sort command?
> 
> $ (echo MIDI;echo Media;echo Mixed)|sort
> Media
> MIDI
> Mixed

I'm actually a bit surprised that plain `sort` doesn't sort by ASCII value (i.e.
all uppercase characters before all lowercase characters), but this sort order
is definitely fine. There is no explicit rule that says whether to sort
case-sensitive or insensitive, so either should be okay.

https://chromiumcodereview.appspot.com/18004004/diff/135001/chrome/app/genera...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/18004004/diff/135001/chrome/app/genera...
chrome/app/generated_resources.grd:14628: +      Clear these settings for future
visits.
Sorry, this is a command, so it does *not* take a period.

[jochen (gone - plz use gerrit), 2013-08-08 07:35:31]

sort sorts according to the configured locale.

For ascii, use ... | LC_ALL=C sort

[Takashi Toyoshima, 2013-08-08 08:24:01]

Wow, thanks Jochen.
So, I reordered file entries in Patch Set 33.

[Bernhard Bauer, 2013-08-08 08:32:51]

On 2013/08/08 08:24:01, Takashi Toyoshima (chromium) wrote:
> Wow, thanks Jochen.
> So, I reordered file entries in Patch Set 33.

I'm sorry, I didn't mean to imply I *want* ASCII sort order, I was just
wondering why your (plain) sort did not apply it, when mine did. I do slightly
prefer the english sorting, but if you want to keep it this way, I'm fine with
it too. Apologies for the confusion.

[Takashi Toyoshima, 2013-08-08 08:44:50]

For the case of include directives, to follow unix sort command results is
expected by coding style guide, IIRC.
I'm not sure it's also true for gyp entries, but I think to be consistent is
better here.
Actually, it's often the case a file needs to include MIDI... and Media..., and
I applied ASCII sort against them before.

[Bernhard Bauer, 2013-08-08 09:23:03]

On 2013/08/08 08:44:50, Takashi Toyoshima (chromium) wrote:
> For the case of include directives, to follow unix sort command results is
> expected by coding style guide, IIRC.
> I'm not sure it's also true for gyp entries, but I think to be consistent is
> better here.
> Actually, it's often the case a file needs to include MIDI... and Media...,
and
> I applied ASCII sort against them before.

Hm, the style guide only says "alphabetically". The PRESUBMIT check for includes
uses Python string comparison though, which is ASCII.

So, LGTM once you fix the strings in generated_resources.grd.

[Takashi Toyoshima, 2013-08-08 09:43:35]

Thanks.

https://chromiumcodereview.appspot.com/18004004/diff/135001/chrome/app/genera...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/18004004/diff/135001/chrome/app/genera...
chrome/app/generated_resources.grd:14628: +      Clear these settings for future
visits.
On 2013/08/08 07:29:50, Bernhard Bauer wrote:
> Sorry, this is a command, so it does *not* take a period.

Done.

[Robert Sesek, 2013-08-08 14:37:49]

https://chromiumcodereview.appspot.com/18004004/diff/161002/chrome/browser/ui...
File chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
(right):

https://chromiumcodereview.appspot.com/18004004/diff/161002/chrome/browser/ui...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:467:
for (auto i = content.domain_lists.rbegin();
Do you want const auto& for any of these?

[Takashi Toyoshima, 2013-08-08 14:52:31]

https://chromiumcodereview.appspot.com/18004004/diff/161002/chrome/browser/ui...
File chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
(right):

https://chromiumcodereview.appspot.com/18004004/diff/161002/chrome/browser/ui...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:467:
for (auto i = content.domain_lists.rbegin();
If I didn't miss something, const auto& doesn't allow following ++i.

[Robert Sesek, 2013-08-08 14:53:51]

LGTM

https://chromiumcodereview.appspot.com/18004004/diff/161002/chrome/browser/ui...
File chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
(right):

https://chromiumcodereview.appspot.com/18004004/diff/161002/chrome/browser/ui...
chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm:467:
for (auto i = content.domain_lists.rbegin();
On 2013/08/08 14:52:32, Takashi Toyoshima (chromium) wrote:
> If I didn't miss something, const auto& doesn't allow following ++i.

Ah, you're right. You can't with the reverse iterators, but you can if you use
the for (:) syntax.

[commit-bot: I haz the power, 2013-08-09 05:15:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/18004004/161002

[commit-bot: I haz the power, 2013-08-09 10:24:54]

Change committed as 216650