[iOS] Factor out the KeyboardAccessoryView.

[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate
class.

Notable changes:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and then
pressing the voice icon in the keyboard accessory view).

BUG=728588
Review-Url: https://codereview.chromium.org/2914193002
Cr-Commit-Position: refs/heads/master@{#476651}
Committed: https://chromium.googlesource.com/chromium/src/+/45a1d81d83b120a07933838d25989a2ef80b471f

[jif, 2017-06-01 13:01:26]

Description was changed from

==========
[iOS] Factor out the KeyboardAccessoryView.

BUG=None.
==========

to

==========
[iOS] Factor out the KeyboardAccessoryView.

BUG=728588
==========

[jif, 2017-06-01 13:14:26]

Description was changed from

==========
[iOS] Factor out the KeyboardAccessoryView.

BUG=728588
==========

to

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate class:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*

Notable change:
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then pressing the voice icon in the keyboard accessory view).

BUG=728588
==========

[jif, 2017-06-01 13:16:16]

Description was changed from

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate class:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*

Notable change:
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then pressing the voice icon in the keyboard accessory view).

BUG=728588
==========

to

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate class.
Notable change:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then pressing the voice icon in the keyboard accessory view).

BUG=728588
==========

[jif, 2017-06-01 13:17:24]

Description was changed from

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate class.
Notable change:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then pressing the voice icon in the keyboard accessory view).

BUG=728588
==========

to

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate
class.
Notable change:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then
pressing the voice icon in the keyboard accessory view).

BUG=728588
==========

[jif, 2017-06-01 13:18:56]

jif@chromium.org changed reviewers:
+ marq@chromium.org

[jif, 2017-06-01 13:19:21]

Description was changed from

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate
class.
Notable change:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then
pressing the voice icon in the keyboard accessory view).

BUG=728588
==========

to

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate
class.

Notable changes:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then
pressing the voice icon in the keyboard accessory view).

BUG=728588
==========

[jif, 2017-06-01 13:19:37]

ptal

[marq (ping after 24h), 2017-06-02 09:15:10]

LGTM, style nits only. Good refactor!

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
File ios/chrome/browser/ui/toolbar/keyboard_accessory_view.h (right):

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
ios/chrome/browser/ui/toolbar/keyboard_accessory_view.h:39: 
Comment for the initializer, documenting behavior if |buttonTitles| is nil or
empty, etc.

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
ios/chrome/browser/ui/toolbar/keyboard_accessory_view.h:40: -
(instancetype)initWithButtons:(NSArray<NSString*>*)buttonTitles
Optional style nit: Put the designated initializer first, then follow with the
unavailable ones.

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
File ios/chrome/browser/ui/toolbar/keyboard_accessory_view.mm (right):

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
ios/chrome/browser/ui/toolbar/keyboard_accessory_view.mm:56: // TODO(pinkerton):
purge this view when low memory.
I realize this is mostly just extracted from the other file, so I'm not going to
worry too much about nitpicking this code, but maybe just delete the 5-year-old
TODOs that don't have bugs.

[jif, 2017-06-02 13:01:18]

Patchset #3 (id:40001) has been deleted

[jif, 2017-06-02 13:03:36]

Patchset #3 (id:60001) has been deleted

[jif, 2017-06-02 13:06:18]

Note that I had accidentally introduced a retain cycle!

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
File ios/chrome/browser/ui/toolbar/keyboard_accessory_view.h (right):

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
ios/chrome/browser/ui/toolbar/keyboard_accessory_view.h:40: -
(instancetype)initWithButtons:(NSArray<NSString*>*)buttonTitles
On 2017/06/02 09:15:10, marq (ping after 24h) wrote:
> Optional style nit: Put the designated initializer first, then follow with the
> unavailable ones.
> 

Done.

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
File ios/chrome/browser/ui/toolbar/keyboard_accessory_view.mm (right):

https://codereview.chromium.org/2914193002/diff/20001/ios/chrome/browser/ui/t...
ios/chrome/browser/ui/toolbar/keyboard_accessory_view.mm:56: // TODO(pinkerton):
purge this view when low memory.
On 2017/06/02 09:15:10, marq (ping after 24h) wrote:
> I realize this is mostly just extracted from the other file, so I'm not going
to
> worry too much about nitpicking this code, but maybe just delete the
5-year-old
> TODOs that don't have bugs.

Done.

[jif, 2017-06-02 13:07:28]

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

[commit-bot: I haz the power, 2017-06-02 13:07:45]

Dry run: CQ is trying da patch.

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

[jif, 2017-06-02 14:50:40]

The CQ bit was unchecked by jif@chromium.org

[jif, 2017-06-02 14:50:44]

The CQ bit was checked by jif@chromium.org

[jif, 2017-06-02 14:50:45]

The patchset sent to the CQ was uploaded after l-g-t-m from marq@chromium.org
Link to the patchset: https://codereview.chromium.org/2914193002/#ps80001
(title: "Addressed comments and broke retain cycle.")

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

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-02 15:02:59]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1496415044423470,
"parent_rev": "b08320999af0d7e05789d81e4d8ded9c506aab3f", "commit_rev":
"3744e933d94b216f4c5b842e19324e54b4009685"}

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

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1496415044423470,
"parent_rev": "b331ebbfd338d17f2b640f3828399cb8f3b3937d", "commit_rev":
"45a1d81d83b120a07933838d25989a2ef80b471f"}

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

Description was changed from

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate
class.

Notable changes:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then
pressing the voice icon in the keyboard accessory view).

BUG=728588
==========

to

==========
[iOS] Factor out the KeyboardAccessoryView.

This CL moves the code describing the KeyboardAccessoryView to a separate
class.

Notable changes:
*Communication with the WebToolbarController now occurs via the
KeyboardAccessoryViewDelegate.
*The preloading of Voice Search now occurs on all touch downs, as opposed to
only the first touch down. This shouldn't be a problem, because preloading could
already happen multiple time (by pressing the voice icon in the toolbar, and
then
pressing the voice icon in the keyboard accessory view).

BUG=728588

Review-Url: https://codereview.chromium.org/2914193002
Cr-Commit-Position: refs/heads/master@{#476651}
Committed:
https://chromium.googlesource.com/chromium/src/+/45a1d81d83b120a07933838d2598...
==========

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

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/45a1d81d83b120a07933838d2598...