[Mac] Add Home button to Default Touch Bar

[Mac] Add Home button to Default Touch Bar

BUG=700057
Review-Url: https://codereview.chromium.org/2743633005
Cr-Commit-Position: refs/heads/master@{#456209}
Committed: https://chromium.googlesource.com/chromium/src/+/0f0ed150a7c3aa62413e8d9cbe4d27594cc74af7

[spqchan, 2017-03-10 00:04:10]

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

[commit-bot: I haz the power, 2017-03-10 00:04:47]

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

[spqchan, 2017-03-10 00:10:28]

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

[commit-bot: I haz the power, 2017-03-10 00:10:37]

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, 2017-03-10 00:33:28]

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

[commit-bot: I haz the power, 2017-03-10 00:33:29]

Dry run: This issue passed the CQ dry run.

[spqchan, 2017-03-10 01:09:34]

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

[spqchan, 2017-03-10 01:09:34]

PTAL

[Robert Sesek, 2017-03-10 17:39:56]

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right):

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_touch_bar.mm:96: void UpdateTouchBar() {
[bwc_ refreshTouchBar]; }
I don't understand why refreshTouchBar needs to exist? Can't
BrowserWindowTouchBar just update itself?

[spqchan, 2017-03-10 18:04:27]

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right):

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_touch_bar.mm:96: void UpdateTouchBar() {
[bwc_ refreshTouchBar]; }
On 2017/03/10 17:39:56, Robert Sesek wrote:
> I don't understand why refreshTouchBar needs to exist? Can't
> BrowserWindowTouchBar just update itself?

In order to update the touch bar, the first responder containing the touchBar
property needs to be set nil.
It could be done in BrowserWindowTouchBar, but we probably want to update it
more often in the future so rather than having a lot of [GetWindow()
setTouchBar:nil] all over the code in the future, I thought it might be better
to just have one method that would refresh it.

[Robert Sesek, 2017-03-10 21:23:58]

LGTM w/ a naming nit

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_controller.h:394: -
(void)refreshTouchBar;
naming: I'd suggest invalidateTouchBar, because this is the terminology that
Apple use in the docs.

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right):

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_touch_bar.mm:96: void UpdateTouchBar() {
[bwc_ refreshTouchBar]; }
On 2017/03/10 18:04:27, spqchan wrote:
> On 2017/03/10 17:39:56, Robert Sesek wrote:
> > I don't understand why refreshTouchBar needs to exist? Can't
> > BrowserWindowTouchBar just update itself?
> 
> In order to update the touch bar, the first responder containing the touchBar
> property needs to be set nil.
> It could be done in BrowserWindowTouchBar, but we probably want to update it
> more often in the future so rather than having a lot of [GetWindow()
> setTouchBar:nil] all over the code in the future, I thought it might be better
> to just have one method that would refresh it.

You're right, that's what the docs say to do. That's a bit surprising to me, but
OK.

[Robert Sesek, 2017-03-10 21:23:58]

LGTM w/ a naming nit

[spqchan, 2017-03-10 22:50:12]

Thanks!

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_controller.h:394: -
(void)refreshTouchBar;
On 2017/03/10 21:23:58, Robert Sesek wrote:
> naming: I'd suggest invalidateTouchBar, because this is the terminology that
> Apple use in the docs.

Done.

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right):

https://codereview.chromium.org/2743633005/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_touch_bar.mm:96: void UpdateTouchBar() {
[bwc_ refreshTouchBar]; }
On 2017/03/10 21:23:58, Robert Sesek wrote:
> On 2017/03/10 18:04:27, spqchan wrote:
> > On 2017/03/10 17:39:56, Robert Sesek wrote:
> > > I don't understand why refreshTouchBar needs to exist? Can't
> > > BrowserWindowTouchBar just update itself?
> > 
> > In order to update the touch bar, the first responder containing the
touchBar
> > property needs to be set nil.
> > It could be done in BrowserWindowTouchBar, but we probably want to update it
> > more often in the future so rather than having a lot of [GetWindow()
> > setTouchBar:nil] all over the code in the future, I thought it might be
better
> > to just have one method that would refresh it.
> 
> You're right, that's what the docs say to do. That's a bit surprising to me,
but
> OK.

I was surprised too when I found out. It's weird

[spqchan, 2017-03-10 22:50:16]

The CQ bit was checked by spqchan@chromium.org

[spqchan, 2017-03-10 22:50:16]

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/2743633005/#ps40001
(title: "Fix for rsesek")

[commit-bot: I haz the power, 2017-03-10 22:50:47]

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, 2017-03-10 23:11:22]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489186216197980,
"parent_rev": "b3adecaccaaa4c19f3a94be273e813d81b357a99", "commit_rev":
"0f0ed150a7c3aa62413e8d9cbe4d27594cc74af7"}

[commit-bot: I haz the power, 2017-03-10 23:12:02]

Description was changed from

==========
[Mac] Add Home button to Default Touch Bar

BUG=700057
==========

to

==========
[Mac] Add Home button to Default Touch Bar

BUG=700057

Review-Url: https://codereview.chromium.org/2743633005
Cr-Commit-Position: refs/heads/master@{#456209}
Committed:
https://chromium.googlesource.com/chromium/src/+/0f0ed150a7c3aa62413e8d9cbe4d...
==========

[commit-bot: I haz the power, 2017-03-10 23:12:03]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/0f0ed150a7c3aa62413e8d9cbe4d...