[ios clean] Use dispatcher for showing TabStrip

[ios clean] Use dispatcher for showing TabStrip

Small change to stop depending on the responder chain for showing the TabStrip

BUG=683793
Review-Url: https://codereview.chromium.org/2940853003
Cr-Commit-Position: refs/heads/master@{#480485}
Committed: https://chromium.googlesource.com/chromium/src/+/402898975cee69b841e0b02e19cd46774acb0531

[sczs, 2017-06-15 00:57:45]

Description was changed from

==========
[ios clean] Use dispatcher for showing TabStrip

BUG=683793
==========

to

==========
[ios clean] Use dispatcher for showing TabStrip

Small change to stop depending on the responder chain for showing the TabStrip

BUG=683793
==========

[sczs, 2017-06-15 00:57:45]

sczs@chromium.org changed reviewers:
+ lpromero@chromium.org, marq@chromium.org

[sczs, 2017-06-15 01:09:41]

sczs@chromium.org changed reviewers:
+ edchin@chromium.org
- lpromero@chromium.org

[sczs, 2017-06-15 01:15:45]

PTAL

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
File ios/clean/chrome/browser/ui/tab_strip/tab_strip_toolbar.mm (right):

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
ios/clean/chrome/browser/ui/tab_strip/tab_strip_toolbar.mm:52: //
TODO(crbug.com/733453): Use dispatcher instead of responder chain.
Adding this TODO so Ed doesn't miss it. I know he was planning on refactoring
this at some point, so I didn't think adding all the plumbing was worth it right
now.

[marq (ping after 24h), 2017-06-15 07:35:35]

Essentially LGTM with some broader comments.

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
File ios/clean/chrome/browser/ui/commands/tab_strip_commands.h (right):

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
ios/clean/chrome/browser/ui/commands/tab_strip_commands.h:14: @optional
So far either all of the commands in a command protocol are optional, or none of
them are; I thin I'd like to avoid mixing the two.

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
File ios/clean/chrome/browser/ui/toolbar/toolbar_view_controller.mm (right):

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
ios/clean/chrome/browser/ui/toolbar/toolbar_view_controller.mm:408: -
(void)showTabStrip:(id)sender {
No change, but this block of boilerplate makes me wonder if there's any reason
to route the dispatcher calls through local methods at all. Should we just set
the dispatcher as an action target when we can?

[sczs, 2017-06-15 19:56:18]

Thanks for taking a look! 
I've made some changes to get rid of the boilerplate, let me know if still lgty.

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
File ios/clean/chrome/browser/ui/commands/tab_strip_commands.h (right):

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
ios/clean/chrome/browser/ui/commands/tab_strip_commands.h:14: @optional
On 2017/06/15 07:35:35, marq (ping after 24h) wrote:
> So far either all of the commands in a command protocol are optional, or none
of
> them are; I thin I'd like to avoid mixing the two.

Done.

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
File ios/clean/chrome/browser/ui/toolbar/toolbar_view_controller.mm (right):

https://codereview.chromium.org/2940853003/diff/1/ios/clean/chrome/browser/ui...
ios/clean/chrome/browser/ui/toolbar/toolbar_view_controller.mm:408: -
(void)showTabStrip:(id)sender {
On 2017/06/15 07:35:35, marq (ping after 24h) wrote:
> No change, but this block of boilerplate makes me wonder if there's any reason
> to route the dispatcher calls through local methods at all. Should we just set
> the dispatcher as an action target when we can?

I agree, might was well change it on this CL. Since the button setup is done on
init (the mediator needs to access the buttons before viewDidLoad) I've added an
extra method to init the VC with a dispatcher.

I'm still exposing the dispatcher for consistency and I guess it could be needed
for swipe to change tabs?

[sczs, 2017-06-19 16:11:56]

Sending to the CQ since there are no new comments.

[sczs, 2017-06-19 16:12:01]

The CQ bit was checked by sczs@chromium.org

[sczs, 2017-06-19 16:12:01]

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/2940853003/#ps20001
(title: "Refactors ToolbarVC init")

[commit-bot: I haz the power, 2017-06-19 16:12:20]

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-19 17:35:05]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497888721331260,
"parent_rev": "2e02d15d69e6235ae201b6d080aec35355fd5152", "commit_rev":
"402898975cee69b841e0b02e19cd46774acb0531"}

[commit-bot: I haz the power, 2017-06-19 17:35:18]

Description was changed from

==========
[ios clean] Use dispatcher for showing TabStrip

Small change to stop depending on the responder chain for showing the TabStrip

BUG=683793
==========

to

==========
[ios clean] Use dispatcher for showing TabStrip

Small change to stop depending on the responder chain for showing the TabStrip

BUG=683793

Review-Url: https://codereview.chromium.org/2940853003
Cr-Commit-Position: refs/heads/master@{#480485}
Committed:
https://chromium.googlesource.com/chromium/src/+/402898975cee69b841e0b02e19cd...
==========

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

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/402898975cee69b841e0b02e19cd...