mac: Allows extensions to hide the bookmark page menu item.

mac: Allows extensions to hide the bookmark page menu item.

The hidden property of the NSMenuItem is dynamically updated in
validateUserInterfaceItem:. That's not the intended purpose of the method, but
that's already how menu_controller/menu_model update the visibility of menu
items. validateUserInterfaceItem: is also called when a user attempts to invoke
the menu item with an accelerator, so this method will also allow/prevent users
from using an accelerator for the menu item.

This CL also hooks up the Cocoa logic for showing browser/page action popups
when a user selects the "bookmark this page" menu item.

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

[erikchen, 2014-02-12 21:29:32]

This change implements (step 3), as suggested by rsesek in crbug.com/342484

https://codereview.chromium.org/161293002/diff/30001/chrome/browser/ui/bookma...
File chrome/browser/ui/bookmarks/bookmark_utils.h (right):

https://codereview.chromium.org/161293002/diff/30001/chrome/browser/ui/bookma...
chrome/browser/ui/bookmarks/bookmark_utils.h:104: // Returns whether the
bookmark page menu item should be shown.
wittman@ suggested this location for this function. I am happy to move it.

[Nico, 2014-02-20 03:40:41]

So the idea is that when an extension handles bookmarking, the menu won't blink
(and there won't be a menu entry)? Wouldn't it be a better experience if the
menu blinked just like before and clicking the menu item would trigger the
extension instead?

(Sorry about delay and dumb question; this has probably been discussed in detail
already and I just didn't find that discussion.)

[erikchen, 2014-02-20 16:23:03]

I don't know what you mean by "blink".

Here is a link to the bug that encompasses the mac changes I'm making:
https://code.google.com/p/chromium/issues/detail?id=342484
Here is a link to the extension proposal:
https://docs.google.com/a/chromium.org/document/d/1C2Mle92O9uGlji5y5gGDM5tNJ_...

According to the extension proposal, (though it's not very clear),
there are 3 possible states:
no extension: leave bookmark menu as is.
extension hides bookmark shortcut: remove menu entry
extension hides bookmark shortcut, and supplies a hotkey for (cmd-d):
keep menu entry, but clicking on entry/using accelerator invokes the
extension action instead of the standard bookmark action.

Please let me know if you have any other questions.

On Wed, Feb 19, 2014 at 7:40 PM,  <thakis@chromium.org> wrote:
> So the idea is that when an extension handles bookmarking, the menu won't
> blink
> (and there won't be a menu entry)? Wouldn't it be a better experience if the
> menu blinked just like before and clicking the menu item would trigger the
> extension instead?
>
> (Sorry about delay and dumb question; this has probably been discussed in
> detail
> already and I just didn't find that discussion.)
>
> https://codereview.chromium.org/161293002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2014-02-20 16:32:21]

If you hit cmd-d, the Bookmarks menu entry lights up for a short time in
the menu. That's what I mean by "blink".


On Thu, Feb 20, 2014 at 8:22 AM, Erik Chen <erikchen@chromium.org> wrote:

> I don't know what you mean by "blink".
>
> Here is a link to the bug that encompasses the mac changes I'm making:
> https://code.google.com/p/chromium/issues/detail?id=342484
> Here is a link to the extension proposal:
>
>
https://docs.google.com/a/chromium.org/document/d/1C2Mle92O9uGlji5y5gGDM5tNJ_...
>
> According to the extension proposal, (though it's not very clear),
> there are 3 possible states:
> no extension: leave bookmark menu as is.
> extension hides bookmark shortcut: remove menu entry
> extension hides bookmark shortcut, and supplies a hotkey for (cmd-d):
> keep menu entry, but clicking on entry/using accelerator invokes the
> extension action instead of the standard bookmark action.
>
> Please let me know if you have any other questions.
>
> On Wed, Feb 19, 2014 at 7:40 PM,  <thakis@chromium.org> wrote:
> > So the idea is that when an extension handles bookmarking, the menu won't
> > blink
> > (and there won't be a menu entry)? Wouldn't it be a better experience if
> the
> > menu blinked just like before and clicking the menu item would trigger
> the
> > extension instead?
> >
> > (Sorry about delay and dumb question; this has probably been discussed in
> > detail
> > already and I just didn't find that discussion.)
> >
> > https://codereview.chromium.org/161293002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[erikchen, 2014-02-20 17:21:31]

The build that I've uploaded has no effect on "blinking". When the
menu item is shown, it still blinks when the accelerator is pressed.

On Thu, Feb 20, 2014 at 8:32 AM, Nico Weber <thakis@chromium.org> wrote:
> If you hit cmd-d, the Bookmarks menu entry lights up for a short time in the
> menu. That's what I mean by "blink".
>
>
> On Thu, Feb 20, 2014 at 8:22 AM, Erik Chen <erikchen@chromium.org> wrote:
>>
>> I don't know what you mean by "blink".
>>
>> Here is a link to the bug that encompasses the mac changes I'm making:
>> https://code.google.com/p/chromium/issues/detail?id=342484
>> Here is a link to the extension proposal:
>>
>>
https://docs.google.com/a/chromium.org/document/d/1C2Mle92O9uGlji5y5gGDM5tNJ_...
>>
>> According to the extension proposal, (though it's not very clear),
>> there are 3 possible states:
>> no extension: leave bookmark menu as is.
>> extension hides bookmark shortcut: remove menu entry
>> extension hides bookmark shortcut, and supplies a hotkey for (cmd-d):
>> keep menu entry, but clicking on entry/using accelerator invokes the
>> extension action instead of the standard bookmark action.
>>
>> Please let me know if you have any other questions.
>>
>> On Wed, Feb 19, 2014 at 7:40 PM,  <thakis@chromium.org> wrote:
>> > So the idea is that when an extension handles bookmarking, the menu
>> > won't
>> > blink
>> > (and there won't be a menu entry)? Wouldn't it be a better experience if
>> > the
>> > menu blinked just like before and clicking the menu item would trigger
>> > the
>> > extension instead?
>> >
>> > (Sorry about delay and dumb question; this has probably been discussed
>> > in
>> > detail
>> > already and I just didn't find that discussion.)
>> >
>> > https://codereview.chromium.org/161293002/
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Robert Sesek, 2014-02-24 14:06:05]

I think the menubar highlighting/"blinking" is correct in this case, right? The
idea is that an extension can completely replace the Chrome-default bookmarking
system, while still feeling "native" to Chrome.

https://codereview.chromium.org/161293002/diff/30001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

https://codereview.chromium.org/161293002/diff/30001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller.mm:1138: NSMenuItem* menuItem
= static_cast<NSMenuItem*>(item);
Use base::mac::ObjCCast

https://codereview.chromium.org/161293002/diff/30001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller.mm:1139: menuItem.hidden =
shouldHide;
There's a strong preference in this file for calling -setHidden:, so I'd stay
with that local style.

[erikchen, 2014-02-26 01:08:18]

rsesek: Patch set 2 responds to your comments (it includes a small rebase,
oops!). Because wittman@'s mega CL has landed, I've updated this CL to add the
previously missing logic, as well as an implementation for step 4 of
crbug.com/342484.

https://codereview.chromium.org/161293002/diff/30001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/browser_window_controller.mm (right):

https://codereview.chromium.org/161293002/diff/30001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller.mm:1138: NSMenuItem* menuItem
= static_cast<NSMenuItem*>(item);
On 2014/02/24 14:06:07, rsesek wrote:
> Use base::mac::ObjCCast

Done.

https://codereview.chromium.org/161293002/diff/30001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/browser_window_controller.mm:1139: menuItem.hidden =
shouldHide;
On 2014/02/24 14:06:07, rsesek wrote:
> There's a strong preference in this file for calling -setHidden:, so I'd stay
> with that local style.

Done.

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
File chrome/browser/ui/bookmarks/bookmark_utils.cc (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:309: for
(extensions::ExtensionSet::const_iterator i = extension_set->begin();
The extension proposal:
https://docs.google.com/a/chromium.org/document/d/1C2Mle92O9uGlji5y5gGDM5tNJ_...

is unclear about what to do if multiple extensions set the flag to override the
bookmark menu item. I take the simplest approach, which is to use disposition of
the first extension with the given flag. Feel free to suggest a different
approach.

[Robert Sesek, 2014-02-26 18:05:13]

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
File chrome/browser/ui/bookmarks/bookmark_utils.cc (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:307: ->extension_service()
It's more common to put the pointer indirection operator on the preceding line:

https://code.google.com/p/chromium/codesearch#search/&q=%5E%5Cs%2B-%3E%20file...
https://code.google.com/p/chromium/codesearch#search/&q=%5C-%3E$%20file:src/c...

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:308: ->extensions();
This will fit on the previous line.

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:309: for
(extensions::ExtensionSet::const_iterator i = extension_set->begin();
On 2014/02/26 01:08:19, erikchen1 wrote:
> The extension proposal:
>
https://docs.google.com/a/chromium.org/document/d/1C2Mle92O9uGlji5y5gGDM5tNJ_...
> 
> is unclear about what to do if multiple extensions set the flag to override
the
> bookmark menu item. I take the simplest approach, which is to use disposition
of
> the first extension with the given flag. Feel free to suggest a different
> approach.

This approach is probably fine, but you may want to check with the people
designing this feature if the behavior should be better defined.

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_cocoa.h (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.h:184: std::string extension_id);
const&

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_cocoa.mm (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.mm:764: int notification_type;
Initialize this since you do not have a default: case.

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.mm:781:
content::NotificationService::current()->Notify(
There's an active effort to remove notifications from Chrome. Is there a
non-notification way to do this?

[Mike Wittman, 2014-02-26 18:29:05]

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
File chrome/browser/ui/bookmarks/bookmark_utils.cc (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:309: for
(extensions::ExtensionSet::const_iterator i = extension_set->begin();
On 2014/02/26 18:05:13, rsesek wrote:
> On 2014/02/26 01:08:19, erikchen1 wrote:
> > The extension proposal:
> >
>
https://docs.google.com/a/chromium.org/document/d/1C2Mle92O9uGlji5y5gGDM5tNJ_...
> > 
> > is unclear about what to do if multiple extensions set the flag to override
> the
> > bookmark menu item. I take the simplest approach, which is to use
disposition
> of
> > the first extension with the given flag. Feel free to suggest a different
> > approach.
> 
> This approach is probably fine, but you may want to check with the people
> designing this feature if the behavior should be better defined.

The extension keybinding behavior is that the first extension to request the
shortcut gets it. So we don't need to care about which extension asks for the
override as long as one does.

I think the proper logic here should be something like:

if any extension overrides the bookmark shortcut
  return BOOKMARK_SHORTCUT_DISPOSITION_OVERRIDDEN
else if any extension removes the bookmark shortcut
  return BOOKMARK_SHORTCUT_DISPOSITION_REMOVED
else
  return BOOKMARK_SHORTCUT_DISPOSITION_UNCHANGED

[erikchen, 2014-02-26 19:58:23]

rsesek: PTAL

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
File chrome/browser/ui/bookmarks/bookmark_utils.cc (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:307: ->extension_service()
Done. I was using the output of clang-format, fyi.
Also, your links don't load. :(

On 2014/02/26 18:05:13, rsesek wrote:
> It's more common to put the pointer indirection operator on the preceding
line:
> 
>
https://code.google.com/p/chromium/codesearch#search/&q=%255E%255Cs%252B-%253...
>
https://code.google.com/p/chromium/codesearch#search/&q=%255C-%253E%24%2520fi...

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:308: ->extensions();
ditto

On 2014/02/26 18:05:13, rsesek wrote:
> This will fit on the previous line.

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:309: for
(extensions::ExtensionSet::const_iterator i = extension_set->begin();
On 2014/02/26 18:29:05, Mike Wittman wrote:
> On 2014/02/26 18:05:13, rsesek wrote:
> > On 2014/02/26 01:08:19, erikchen1 wrote:
> > > The extension proposal:
> > >
> >
>
https://docs.google.com/a/chromium.org/document/d/1C2Mle92O9uGlji5y5gGDM5tNJ_...
> > > 
> > > is unclear about what to do if multiple extensions set the flag to
override
> > the
> > > bookmark menu item. I take the simplest approach, which is to use
> disposition
> > of
> > > the first extension with the given flag. Feel free to suggest a different
> > > approach.
> > 
> > This approach is probably fine, but you may want to check with the people
> > designing this feature if the behavior should be better defined.
> 
> The extension keybinding behavior is that the first extension to request the
> shortcut gets it. So we don't need to care about which extension asks for the
> override as long as one does.
> 
> I think the proper logic here should be something like:
> 
> if any extension overrides the bookmark shortcut
>   return BOOKMARK_SHORTCUT_DISPOSITION_OVERRIDDEN
> else if any extension removes the bookmark shortcut
>   return BOOKMARK_SHORTCUT_DISPOSITION_REMOVED
> else
>   return BOOKMARK_SHORTCUT_DISPOSITION_UNCHANGED

Done.

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_cocoa.h (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.h:184: std::string extension_id);
ack. good catch.
the method got removed once I stopped using notifications.

On 2014/02/26 18:05:13, rsesek wrote:
> const&

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_cocoa.mm (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.mm:764: int notification_type;
method was removed.

On 2014/02/26 18:05:13, rsesek wrote:
> Initialize this since you do not have a default: case.

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.mm:781:
content::NotificationService::current()->Notify(
certainly. I've plumbed through the necessary logic for showing page/browser
actions directly, rather than through a notification system.

On 2014/02/26 18:05:13, rsesek wrote:
> There's an active effort to remove notifications from Chrome. Is there a
> non-notification way to do this?

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/bookm...
File chrome/browser/ui/bookmarks/bookmark_utils.cc (right):

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:322: if (removed)
I consider this form to be slightly clearer than the ternary operator, let me
know if you prefer the ternary here.

[Mike Wittman, 2014-02-26 20:16:02]

GetBookmarkShortcutDisposition lgtm

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_cocoa.h (right):

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.h:11: #include
"chrome/browser/extensions/api/commands/command_service.h"
Remove this header?

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_cocoa.mm (right):

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.mm:54: #include
"content/public/browser/notification_service.h"
Remove this header?

[Robert Sesek, 2014-02-26 20:30:00]

LGTM

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
File chrome/browser/ui/bookmarks/bookmark_utils.cc (right):

https://codereview.chromium.org/161293002/diff/220007/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:307: ->extension_service()
On 2014/02/26 19:58:23, erikchen1 wrote:
> Done. I was using the output of clang-format, fyi.
> Also, your links don't load. :(

Weird, Rietveld double-percent-escapes them. It's was something like 160
instances of on new line vs 1200 on previous line. 


> On 2014/02/26 18:05:13, rsesek wrote:
> > It's more common to put the pointer indirection operator on the preceding
> line:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=%25255E%25255Cs%25252...
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=%25255C-%25253E%2524%...
>

[erikchen, 2014-02-26 20:53:51]

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_cocoa.h (right):

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.h:11: #include
"chrome/browser/extensions/api/commands/command_service.h"
On 2014/02/26 20:16:03, Mike Wittman wrote:
> Remove this header?

Done.

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_cocoa.mm (right):

https://codereview.chromium.org/161293002/diff/260001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_cocoa.mm:54: #include
"content/public/browser/notification_service.h"
On 2014/02/26 20:16:03, Mike Wittman wrote:
> Remove this header?

Done.

[erikchen, 2014-02-26 21:10:51]

sky: I'm looking for an OWNER to review
chrome/browser/ui/bookmarks/bookmark_utils.h
chrome/browser/ui/bookmarks/bookmark_utils.cc
chrome/browser/ui/toolbar/wrench_menu_model.cc

[sky, 2014-02-26 21:46:31]

Do you need to disable the bookmark command too?

https://codereview.chromium.org/161293002/diff/280001/chrome/browser/ui/bookm...
File chrome/browser/ui/bookmarks/bookmark_utils.cc (right):

https://codereview.chromium.org/161293002/diff/280001/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:305: const
extensions::ExtensionSet* extension_set =
Do you need any null checks for the commandservice or any of these other
services here?

https://codereview.chromium.org/161293002/diff/280001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/wrench_menu_model.cc (right):

https://codereview.chromium.org/161293002/diff/280001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/wrench_menu_model.cc:407: if (command_id ==
IDC_VIEW_INCOMPATIBILITIES) {
I think it's time for a switch statement.

[erikchen, 2014-02-26 22:31:19]

sky: thank you for the fast turn around! 

In response to the question: "Do you need to disable the bookmark command too?".
No. I assume that you are referring to the accelerator (Ctr/Cmd+D). On Mac, the
presence of the NSMenuItem (which this CL controls) is also the way that an
application registers an accelerator with OSX.

See crbug.com/342484 for the discussion behind the validity of this CL.
If you're interested in command dispatch on mac, I rewrote the wiki
documentation here:
https://sites.google.com/a/chromium.org/dev/developers/design-documents/comma...

PTAL

https://codereview.chromium.org/161293002/diff/280001/chrome/browser/ui/bookm...
File chrome/browser/ui/bookmarks/bookmark_utils.cc (right):

https://codereview.chromium.org/161293002/diff/280001/chrome/browser/ui/bookm...
chrome/browser/ui/bookmarks/bookmark_utils.cc:305: const
extensions::ExtensionSet* extension_set =
A search through the codebase shows that we generally do not null check
CommandService.

I've replaced the call to ExtensionSystem/Service with a call to
ExtensionRegistry, since the documentation for ExtensionService indicates that
the "extensions()" method is deprecated.

On 2014/02/26 21:46:31, sky wrote:
> Do you need any null checks for the commandservice or any of these other
> services here?

https://codereview.chromium.org/161293002/diff/280001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/wrench_menu_model.cc (right):

https://codereview.chromium.org/161293002/diff/280001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/wrench_menu_model.cc:407: if (command_id ==
IDC_VIEW_INCOMPATIBILITIES) {
On 2014/02/26 21:46:31, sky wrote:
> I think it's time for a switch statement.

Done.

[sky, 2014-02-27 00:25:40]

Does the disabling work on windows? I don't believe it's keyed off the menu
items.

[erikchen, 2014-02-27 00:32:45]

It's not intended to work on Windows. This is the Cocoa implementation of the
desired behavior. wittman@ is still working on the linux/windows implementation.

[erikchen, 2014-02-27 00:43:14]

sky: I've updated the code in chrome/browser/ui/toolbar/wrench_menu_model.cc to
only hide the bookmark menu item on OSX, so this CL should not longer affect the
behavior of bookmarks on other OS'es.

[sky, 2014-02-27 01:24:30]

Ok, LGTM

[erikchen, 2014-02-27 01:32:13]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-02-27 01:35:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erikchen@chromium.org/161293002/280002

[commit-bot: I haz the power, 2014-02-27 07:18:42]

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

[commit-bot: I haz the power, 2014-02-27 07:18:43]

Try jobs failed on following builders: mac_chromium_rel

[erikchen, 2014-02-27 16:49:27]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-02-27 16:49:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erikchen@chromium.org/161293002/280002

[commit-bot: I haz the power, 2014-02-27 19:01:51]

Change committed as 253888