[Mac] Omnibox icons active states

[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377, 667707
Committed: https://crrev.com/90af6b8cabfe002784be9735e4a1b675588b06f2
Cr-Commit-Position: refs/heads/master@{#435370}

[spqchan, 2016-11-17 03:37:45]

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

[commit-bot: I haz the power, 2016-11-17 03:38:36]

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, 2016-11-17 04:03:40]

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

[commit-bot: I haz the power, 2016-11-17 04:03:41]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[shrike, 2016-11-17 23:45:04]

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

[shrike, 2016-11-17 23:45:04]

What is the bug for this change? I'm trying to figure out if
https://crbug.com/591140 should be closed out or should be the tracking bug for
this issue.

[spqchan, 2016-11-17 23:49:04]

On 2016/11/17 23:45:04, shrike wrote:
> What is the bug for this change? I'm trying to figure out if
> https://crbug.com/591140 should be closed out or should be the tracking bug
for
> this issue.

crbug.com/588377

Huh, that 591140 might be a dupe

[spqchan, 2016-11-17 23:49:41]

Description was changed from

==========
[Mac] Omnibox icons active states

BUG=
==========

to

==========
[Mac] Omnibox icons active states

(WIP so please don't review)

BUG=
==========

[spqchan, 2016-11-18 01:51:34]

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

[commit-bot: I haz the power, 2016-11-18 01:52:56]

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, 2016-11-18 02:16:55]

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

[commit-bot: I haz the power, 2016-11-18 02:16:56]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[spqchan, 2016-11-18 02:26:08]

Description was changed from

==========
[Mac] Omnibox icons active states

(WIP so please don't review)

BUG=
==========

to

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

The bubbles affected include:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

(WIP so please don't review)

BUG=
==========

[spqchan, 2016-11-18 02:26:25]

Description was changed from

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

The bubbles affected include:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

(WIP so please don't review)

BUG=
==========

to

==========
[Mac] Omnibox icons active states

(WIP so please don't review)

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

The bubbles affected include:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

BUG=
==========

[spqchan, 2016-11-18 02:28:11]

Description was changed from

==========
[Mac] Omnibox icons active states

(WIP so please don't review)

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

The bubbles affected include:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

BUG=
==========

to

==========
[Mac] Omnibox icons active states

(WIP so please don't review)

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=
==========

[spqchan, 2016-11-18 02:28:31]

Description was changed from

==========
[Mac] Omnibox icons active states

(WIP so please don't review)

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=
==========

to

==========
[Mac] Omnibox icons active states

(WIP so please don't review)

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377
==========

[spqchan, 2016-11-18 18:48:34]

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

[commit-bot: I haz the power, 2016-11-18 18:49:29]

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

[spqchan, 2016-11-18 19:34:17]

Description was changed from

==========
[Mac] Omnibox icons active states

(WIP so please don't review)

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377
==========

to

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377
==========

[spqchan, 2016-11-18 19:34:23]

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

[spqchan, 2016-11-18 19:34:42]

CL is now ready, PTAL :)

[commit-bot: I haz the power, 2016-11-18 19:35:51]

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, 2016-11-18 22:03:16]

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

[commit-bot: I haz the power, 2016-11-18 22:03:17]

Dry run: This issue passed the CQ dry run.

[shrike, 2016-11-18 23:41:58]

Looks good except it doesn't seem like the colors are correct for Incognito? I
think hover and pressed states should be a transparent white instead of
transparent black. I'm not sure if the secure state decoration is affected by
this cl but it also has the problem (clicking Secure, for example, the
background just gets darker).

[spqchan, 2016-11-18 23:47:25]

On 2016/11/18 23:41:58, shrike wrote:
> Looks good except it doesn't seem like the colors are correct for Incognito? I
> think hover and pressed states should be a transparent white instead of
> transparent black. I'm not sure if the secure state decoration is affected by
> this cl but it also has the problem (clicking Secure, for example, the
> background just gets darker).

The fix for the incognito background color CL is here:
https://codereview.chromium.org/2513083002/

[spqchan, 2016-11-19 00:25:39]

spqchan@chromium.org changed reviewers:
+ rsesek@chromium.org

[spqchan, 2016-11-19 00:25:40]

PTAL

[Robert Sesek, 2016-11-21 19:28:23]

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h:115: NSPoint
GetSaveCreditCardBubblePoint() const;
What about this and GetPageInfoBubblePoint() ?

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.h (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.h:17: @interface
OmniboxIconBubbleController : BaseBubbleController
s/Icon/Decoration ?

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.mm (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.mm:31: -
(LocationBarDecoration*)decorationForBubble {
A lot of subclasses are now having to store this in their own decoration_ ivar.
I kind of think this should be an argument to the initializer, but
BaseBubbleController has 3 unique ones. Do the bubbles you are changing only use
one initializer, by any chance?

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/toolbar/toolbar_controller.h (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.h:173: -
(LocationBarDecoration*)bookmarkDecoration;
Per the previous comment, by injecting the LocationBarDecoration into the
OmniboxIconBubbleController I think you may not need to expose these accessors.

[spqchan, 2016-11-21 22:16:22]

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

[commit-bot: I haz the power, 2016-11-21 22:16:47]

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, 2016-11-21 22:20:06]

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

[commit-bot: I haz the power, 2016-11-21 22:20:07]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  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_...)

[spqchan, 2016-11-21 22:36:45]

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

[commit-bot: I haz the power, 2016-11-21 22:37:27]

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, 2016-11-21 23:34:55]

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

[commit-bot: I haz the power, 2016-11-21 23:34:56]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[spqchan, 2016-11-21 23:35:56]

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

[commit-bot: I haz the power, 2016-11-21 23:36:41]

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, 2016-11-22 00:14:44]

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

[commit-bot: I haz the power, 2016-11-22 00:14:45]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[spqchan, 2016-11-22 18:49:18]

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

[commit-bot: I haz the power, 2016-11-22 18:49:47]

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, 2016-11-22 19:18:08]

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

[commit-bot: I haz the power, 2016-11-22 19:18:09]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[spqchan, 2016-11-22 19:20:01]

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

[commit-bot: I haz the power, 2016-11-22 19:20:34]

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, 2016-11-22 19:44:06]

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

[commit-bot: I haz the power, 2016-11-22 19:44:07]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[spqchan, 2016-11-22 20:53:59]

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

[commit-bot: I haz the power, 2016-11-22 20:54:38]

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, 2016-11-22 22:45:03]

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

[commit-bot: I haz the power, 2016-11-22 22:45:04]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[spqchan, 2016-11-22 23:29:16]

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

[commit-bot: I haz the power, 2016-11-22 23:30:07]

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, 2016-11-23 01:19:59]

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

[commit-bot: I haz the power, 2016-11-23 01:20:00]

Dry run: This issue passed the CQ dry run.

[spqchan, 2016-11-23 22:58:54]

PTAL after thanksgiving holidays. Have a good one!

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h:115: NSPoint
GetSaveCreditCardBubblePoint() const;
On 2016/11/21 19:28:23, Robert Sesek wrote:
> What about this and GetPageInfoBubblePoint() ?

I want to avoid touching the credit card bubble in this CL. I'm making changes
to it in a follow up CL.

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.h (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.h:17: @interface
OmniboxIconBubbleController : BaseBubbleController
On 2016/11/21 19:28:23, Robert Sesek wrote:
> s/Icon/Decoration ?

sure

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.mm (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.mm:31: -
(LocationBarDecoration*)decorationForBubble {
On 2016/11/21 19:28:23, Robert Sesek wrote:
> A lot of subclasses are now having to store this in their own decoration_
ivar.
> I kind of think this should be an argument to the initializer, but
> BaseBubbleController has 3 unique ones. Do the bubbles you are changing only
use
> one initializer, by any chance?

I agree, I wanted to put it in the initializer but unfortunately, the bubbles
use more than one which makes things complex =/

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/toolbar/toolbar_controller.h (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.h:173: -
(LocationBarDecoration*)bookmarkDecoration;
On 2016/11/21 19:28:23, Robert Sesek wrote:
> Per the previous comment, by injecting the LocationBarDecoration into the
> OmniboxIconBubbleController I think you may not need to expose these
accessors.

The bubble classes still need to access the decorations. I made a change though
so they are pulling the values from locationBarViewMac directly instead of
toolbarController.

[spqchan, 2016-11-28 19:13:28]

Description was changed from

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377
==========

to

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377, 667707
==========

[Robert Sesek, 2016-11-28 22:46:59]

On 2016/11/23 22:58:54, spqchan wrote:
> PTAL after thanksgiving holidays. Have a good one!

Thanks! And welcome back :)

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.mm (right):

https://codereview.chromium.org/2511043002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/omnibox_icon_bubble_controller.mm:31: -
(LocationBarDecoration*)decorationForBubble {
On 2016/11/23 22:58:54, spqchan wrote:
> On 2016/11/21 19:28:23, Robert Sesek wrote:
> > A lot of subclasses are now having to store this in their own decoration_
> ivar.
> > I kind of think this should be an argument to the initializer, but
> > BaseBubbleController has 3 unique ones. Do the bubbles you are changing only
> use
> > one initializer, by any chance?
> 
> I agree, I wanted to put it in the initializer but unfortunately, the bubbles
> use more than one which makes things complex =/

Acknowledged. I figured that'd be the case. Hopefully views will sweep all this
under the rug!

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h (right):

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h:184: NSView*
tracking_area_owner_;
And if so at line 191, do it here as well and remove the initializer list.

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h:191:
DecorationMouseState state_;
Can you do initialization here, too?

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h (right):

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h:110: // associate
bubble aims at.
nit: "at" at the end of this sentence is weird

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/passwords/passwords_bubble_controller.mm (right):

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/passwords/passwords_bubble_controller.mm:75: return
decoration_;
In all the other DecorationControllers, you don't store decoration_ but just
access it through this method. Can the same be done here?

[spqchan, 2016-11-28 23:22:15]

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

[commit-bot: I haz the power, 2016-11-28 23:23:33]

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, 2016-11-29 01:36:04]

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

[commit-bot: I haz the power, 2016-11-29 01:36:05]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[spqchan, 2016-11-29 17:34:49]

PTAL

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h (right):

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h:184: NSView*
tracking_area_owner_;
On 2016/11/28 22:46:59, Robert Sesek wrote:
> And if so at line 191, do it here as well and remove the initializer list.

Done.

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h:191:
DecorationMouseState state_;
On 2016/11/28 22:46:59, Robert Sesek wrote:
> Can you do initialization here, too?

Done.

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h (right):

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h:110: // associate
bubble aims at.
On 2016/11/28 22:46:59, Robert Sesek wrote:
> nit: "at" at the end of this sentence is weird

Done.

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/passwords/passwords_bubble_controller.mm (right):

https://codereview.chromium.org/2511043002/diff/260001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/passwords/passwords_bubble_controller.mm:75: return
decoration_;
On 2016/11/28 22:46:59, Robert Sesek wrote:
> In all the other DecorationControllers, you don't store decoration_ but just
> access it through this method. Can the same be done here?

Done.

[Robert Sesek, 2016-11-29 21:02:21]

LGTM

[spqchan, 2016-11-29 21:13:27]

On 2016/11/29 21:02:21, Robert Sesek wrote:
> LGTM

thanks!

[spqchan, 2016-11-29 21:13:34]

The CQ bit was checked by spqchan@chromium.org

[commit-bot: I haz the power, 2016-11-29 21:14:14]

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, 2016-11-30 00:04:23]

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

[commit-bot: I haz the power, 2016-11-30 00:04:24]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[spqchan, 2016-11-30 00:08:36]

The CQ bit was checked by spqchan@chromium.org

[commit-bot: I haz the power, 2016-11-30 00:09:54]

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, 2016-11-30 01:58:43]

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

[commit-bot: I haz the power, 2016-11-30 01:58:44]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[spqchan, 2016-11-30 18:23:33]

The CQ bit was checked by spqchan@chromium.org

[spqchan, 2016-11-30 18:23:33]

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

[commit-bot: I haz the power, 2016-11-30 18:24:01]

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, 2016-11-30 19:23:02]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1480530213208550,
"parent_rev": "7f25b4e976ffa786fe0a60e6c284d95148d3d4b4", "commit_rev":
"9f4db1b55a9475ede02f6f82878979a420c84635"}

[commit-bot: I haz the power, 2016-11-30 19:23:26]

Description was changed from

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377, 667707
==========

to

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377, 667707
==========

[commit-bot: I haz the power, 2016-11-30 19:23:28]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-11-30 19:27:53]

Description was changed from

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377, 667707
==========

to

==========
[Mac] Omnibox icons active states

Implemented OmniboxIconBubbleController for bubbles anchored to the omnibox
icons. This controller updates the icon's active state based on the state of the
bubble.

This CL affect the following bubbles:
- Translate
- Permission
- Content settings
- Bookmarks
- Manage passwords

Other bubbles will be update in a follow up CL

BUG=588377, 667707

Committed: https://crrev.com/90af6b8cabfe002784be9735e4a1b675588b06f2
Cr-Commit-Position: refs/heads/master@{#435370}
==========

[commit-bot: I haz the power, 2016-11-30 19:27:55]

Patchset 16 (id:??) landed as
https://crrev.com/90af6b8cabfe002784be9735e4a1b675588b06f2
Cr-Commit-Position: refs/heads/master@{#435370}