Reposition the settings button as part of the notification update method

Reposition the settings button as part of the notification update method

BUG=564028
Committed: https://crrev.com/5964baec90ea4aec2683bc00c732a891b8411179
Cr-Commit-Position: refs/heads/master@{#362749}

[Miguel Garcia, 2015-12-02 12:59:26]

miguelg@chromium.org changed reviewers:
+ dewittj@chromium.org, rsesek@chromium.org

[Miguel Garcia, 2015-12-02 12:59:27]

I would like to merge this in M48 if possible so I'd appreciate a quick review
:)

Adding Robert as the main reviewer and dewittj for OWNERS rubberstamp since the
cocoa directory does not have OWNERS of their own.

Thanks!

[Robert Sesek, 2015-12-02 15:59:39]

Thanks for the quick fix!

https://codereview.chromium.org/1487373003/diff/20001/ui/message_center/cocoa...
File ui/message_center/cocoa/notification_controller.mm (right):

https://codereview.chromium.org/1487373003/diff/20001/ui/message_center/cocoa...
ui/message_center/cocoa/notification_controller.mm:478: if (settingsButton_ !=
nil)
You can skip the nil check here and on 669, since in ObjC, messaging to nil
always returns a zero value.

[Miguel Garcia, 2015-12-02 16:37:00]

Thanks for the quick review!

Nil checks removed. PTAL

https://codereview.chromium.org/1487373003/diff/20001/ui/message_center/cocoa...
File ui/message_center/cocoa/notification_controller.mm (right):

https://codereview.chromium.org/1487373003/diff/20001/ui/message_center/cocoa...
ui/message_center/cocoa/notification_controller.mm:478: if (settingsButton_ !=
nil)
On 2015/12/02 15:59:38, Robert Sesek wrote:
> You can skip the nil check here and on 669, since in ObjC, messaging to nil
> always returns a zero value.

Wow I am starting to like this ObjC business :)

[dewittj, 2015-12-02 17:08:25]

lgtm, please wait for rsesek's lgtm before submitting

https://codereview.chromium.org/1487373003/diff/40001/ui/message_center/cocoa...
File ui/message_center/cocoa/notification_controller.mm (right):

https://codereview.chromium.org/1487373003/diff/40001/ui/message_center/cocoa...
ui/message_center/cocoa/notification_controller.mm:460: 
nit: any reason to add this line?

[Robert Sesek, 2015-12-02 17:19:55]

lgtm

[Miguel Garcia, 2015-12-02 17:29:37]

Thanks both!

https://codereview.chromium.org/1487373003/diff/40001/ui/message_center/cocoa...
File ui/message_center/cocoa/notification_controller.mm (right):

https://codereview.chromium.org/1487373003/diff/40001/ui/message_center/cocoa...
ui/message_center/cocoa/notification_controller.mm:460: 
On 2015/12/02 17:08:24, dewittj wrote:
> nit: any reason to add this line?

no reason, removed now

[Miguel Garcia, 2015-12-02 17:29:40]

The CQ bit was checked by miguelg@chromium.org

[Miguel Garcia, 2015-12-02 17:29:41]

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

[commit-bot: I haz the power, 2015-12-02 17:30:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1487373003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1487373003/60001

[commit-bot: I haz the power, 2015-12-02 18:10:10]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-02 18:11:14]

Description was changed from

==========
Reposition the settings button as part of the notification update method

BUG=564028
==========

to

==========
Reposition the settings button as part of the notification update method

BUG=564028

Committed: https://crrev.com/5964baec90ea4aec2683bc00c732a891b8411179
Cr-Commit-Position: refs/heads/master@{#362749}
==========

[commit-bot: I haz the power, 2015-12-02 18:11:14]

Patchset 4 (id:??) landed as
https://crrev.com/5964baec90ea4aec2683bc00c732a891b8411179
Cr-Commit-Position: refs/heads/master@{#362749}