[Mac] Touch Bar Support for Dialogs

[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util.

Since the bookmark and bookmark editor dialogs contain textfields,
their touch bar would be overridden by the textfields. To prevent
that, the dialogs will provide a DialogTextFieldEditor object as a
custom text field editor.

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
Review-Url: https://codereview.chromium.org/2921083003
Cr-Commit-Position: refs/heads/master@{#477756}
Committed: https://chromium.googlesource.com/chromium/src/+/69b6752a16b133e7dc53822317202bd6f81401dc

[spqchan, 2017-06-02 19:17:11]

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

[spqchan, 2017-06-02 19:17:23]

Description was changed from

==========
[Mac] Touch Bar Support for Dialogs


Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug:698795
Change-Id: I21780dfb552141819c5e8126efea61115565d83f
==========

to

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
==========

[commit-bot: I haz the power, 2017-06-02 19:17:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[spqchan, 2017-06-02 19:53:10]

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

[commit-bot: I haz the power, 2017-06-02 19:53:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[spqchan, 2017-06-02 21:19:37]

Description was changed from

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
==========

to

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util.

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
==========

[spqchan, 2017-06-02 21:22:09]

Description was changed from

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util.

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
==========

to

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util.

Since the bookmark and bookmark editor dialogs contain textfields,
their touch bar would be overridden by the textfields. This issue is
solved by using DialogTextFieldEditor as  a custom field editor for
the dialogs.

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
==========

[spqchan, 2017-06-02 21:22:10]

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

[spqchan, 2017-06-02 21:23:11]

Description was changed from

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util.

Since the bookmark and bookmark editor dialogs contain textfields,
their touch bar would be overridden by the textfields. This issue is
solved by using DialogTextFieldEditor as  a custom field editor for
the dialogs.

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
==========

to

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util.

Since the bookmark and bookmark editor dialogs contain textfields,
their touch bar would be overridden by the textfields. To prevent
that, the dialogs will provide a DialogTextFieldEditor object as a
custom text field editor.

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
==========

[spqchan, 2017-06-02 21:23:58]

PTAL. Let me know if you have any questions or want me to split up the CL.

[commit-bot: I haz the power, 2017-06-02 21:43:05]

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

[commit-bot: I haz the power, 2017-06-02 21:43:06]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2017-06-06 14:52:36]

It would have been nice to split this up, but it's pretty straightforward so
it's fine.

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

https://codereview.chromium.org/2921083003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/dialog_text_field_editor.h:11: // It intercepts touch
bar methods to prevent the textfield's touch Bar from
nit: capitalization of "touch Bar" is off.

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
File ui/base/cocoa/touch_bar_util.h (right):

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
ui/base/cocoa/touch_bar_util.h:1: // Copyright 2017 The Chromium Authors. All
rights reserved.
Unittests for this file?

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
ui/base/cocoa/touch_bar_util.h:13: 
Optional: It may be handy to add a function like:

Class NSTouchBar() {
  return NSClassFromString(@"NSTouchBar");
}

To replace:

base::scoped_nsobject<NSTouchBar> touchBar(
      [[NSClassFromString(@"NSTouchBar") alloc] init]);

With:

base::scoped_nsobject<NSTouchBar> touchBar(
      [[ui::NSTouchBar() alloc] init]);

And the same with NSCustomTouchBarItem.

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
ui/base/cocoa/touch_bar_util.h:16: UI_BASE_EXPORT NSButton*
CreateBlueTouchBarButton(NSString* title,
These functions shouldn't be named "Create" unless they return strongly
referenced objects/scoped_nsobject (per the "create" rule).

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
File ui/base/cocoa/touch_bar_util.mm (right):

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
ui/base/cocoa/touch_bar_util.mm:5: #import "ui/base/cocoa/touch_bar_util.h"
nit: blank line after

[spqchan, 2017-06-06 19:33:44]

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

[commit-bot: I haz the power, 2017-06-06 19:34:08]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[spqchan, 2017-06-06 19:34:51]

I'm writing unit tests for touch_bar_utils. What should I change the "Create"
method names to?

CreateBlueTouchBarButton -> BlueTouchBarButton or GetBlueTouchBarButton?

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

https://codereview.chromium.org/2921083003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/dialog_text_field_editor.h:11: // It intercepts touch
bar methods to prevent the textfield's touch Bar from
On 2017/06/06 14:52:35, Robert Sesek wrote:
> nit: capitalization of "touch Bar" is off.

Done.

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
File ui/base/cocoa/touch_bar_util.h (right):

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
ui/base/cocoa/touch_bar_util.h:13: 
On 2017/06/06 14:52:36, Robert Sesek wrote:
> Optional: It may be handy to add a function like:
> 
> Class NSTouchBar() {
>   return NSClassFromString(@"NSTouchBar");
> }
> 
> To replace:
> 
> base::scoped_nsobject<NSTouchBar> touchBar(
>       [[NSClassFromString(@"NSTouchBar") alloc] init]);
> 
> With:
> 
> base::scoped_nsobject<NSTouchBar> touchBar(
>       [[ui::NSTouchBar() alloc] init]);
> 
> And the same with NSCustomTouchBarItem.

Done.

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
ui/base/cocoa/touch_bar_util.h:16: UI_BASE_EXPORT NSButton*
CreateBlueTouchBarButton(NSString* title,
On 2017/06/06 14:52:35, Robert Sesek wrote:
> These functions shouldn't be named "Create" unless they return strongly
> referenced objects/scoped_nsobject (per the "create" rule).

What should I use? GetBlueTouchBarButton or just BlueTouchBarButton?

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
File ui/base/cocoa/touch_bar_util.mm (right):

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
ui/base/cocoa/touch_bar_util.mm:5: #import "ui/base/cocoa/touch_bar_util.h"
On 2017/06/06 14:52:36, Robert Sesek wrote:
> nit: blank line after

Done.

[Robert Sesek, 2017-06-06 20:15:00]

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
File ui/base/cocoa/touch_bar_util.h (right):

https://codereview.chromium.org/2921083003/diff/20001/ui/base/cocoa/touch_bar...
ui/base/cocoa/touch_bar_util.h:16: UI_BASE_EXPORT NSButton*
CreateBlueTouchBarButton(NSString* title,
On 2017/06/06 19:34:50, spqchan wrote:
> On 2017/06/06 14:52:35, Robert Sesek wrote:
> > These functions shouldn't be named "Create" unless they return strongly
> > referenced objects/scoped_nsobject (per the "create" rule).
> 
> What should I use? GetBlueTouchBarButton or just BlueTouchBarButton?

Yes, I'd use GetBlueTouchBarButton().

[commit-bot: I haz the power, 2017-06-06 21:02:03]

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

[commit-bot: I haz the power, 2017-06-06 21:02:04]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[spqchan, 2017-06-06 22:30:27]

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

[commit-bot: I haz the power, 2017-06-06 22:30:57]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[spqchan, 2017-06-06 22:34:03]

Patchset #4 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-06-06 22:34:47]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[spqchan, 2017-06-06 22:38:05]

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

[commit-bot: I haz the power, 2017-06-06 22:38:50]

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-06-06 22:57:06]

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

[commit-bot: I haz the power, 2017-06-06 22:57:07]

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

[spqchan, 2017-06-06 23:05:11]

Patchset #4 (id:80001) has been deleted

[spqchan, 2017-06-06 23:05:16]

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

[commit-bot: I haz the power, 2017-06-06 23:05:42]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[spqchan, 2017-06-07 00:07:59]

Great, thanks. I added a unit test and addressed your comments

[commit-bot: I haz the power, 2017-06-07 00:49:15]

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

[commit-bot: I haz the power, 2017-06-07 00:49:16]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2017-06-07 15:18:33]

LGTM

[spqchan, 2017-06-07 20:32:46]

On 2017/06/07 15:18:33, Robert Sesek wrote:
> LGTM

thanks!

[spqchan, 2017-06-07 20:32:52]

The CQ bit was checked by spqchan@chromium.org

[commit-bot: I haz the power, 2017-06-07 20:33:29]

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-06-07 20:56:59]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1496867572394610,
"parent_rev": "1250a508840b955c6e96eae698cf9783ec1f39d7", "commit_rev":
"69b6752a16b133e7dc53822317202bd6f81401dc"}

[commit-bot: I haz the power, 2017-06-07 20:57:12]

Description was changed from

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util.

Since the bookmark and bookmark editor dialogs contain textfields,
their touch bar would be overridden by the textfields. To prevent
that, the dialogs will provide a DialogTextFieldEditor object as a
custom text field editor.

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795
==========

to

==========
[Mac] Touch Bar Support for Dialogs

Implemented touch bar for bookmark bubble, bookmark editor,
and content setting bubble dialogs.

Moved GetTouchBarId() and GetTouchBarItemId() from
BrowserWindowTouchBar to foundation_util.

Since the bookmark and bookmark editor dialogs contain textfields,
their touch bar would be overridden by the textfields. To prevent
that, the dialogs will provide a DialogTextFieldEditor object as a
custom text field editor.

Touch bar tests are added to:
- bookmark_bubble_controller_unittest.mm
- bookmark_editor_base_controller_unittest.mm
- content_setting_bubble_cocoa_browsertest.mm

Bug=698795

Review-Url: https://codereview.chromium.org/2921083003
Cr-Commit-Position: refs/heads/master@{#477756}
Committed:
https://chromium.googlesource.com/chromium/src/+/69b6752a16b133e7dc5382231720...
==========

[commit-bot: I haz the power, 2017-06-07 20:57:13]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/69b6752a16b133e7dc5382231720...