Add CommandDispatcher to BrowserViewController.

Add CommandDispatcher to BrowserViewController.

Basic dispatcher to help with ios/clean migration.  Handlers UrlLoader,
OmniboxFocuser and the WebToolbarDelegate, all used by the
GoogleLandingController.

BUG=694750
Review-Url: https://codereview.chromium.org/2829003002
Cr-Commit-Position: refs/heads/master@{#468023}
Committed: https://chromium.googlesource.com/chromium/src/+/75011c3d5ea9d22e2317d6faa7d9901eb2d2a2ca

[justincohen, 2017-04-20 02:16:58]

Description was changed from

==========
rm# Enter a description of the change.
Add dispatchers concept

BUG=694750
==========

to

==========
Add dispatchers concept

BUG=694750
==========

[justincohen, 2017-04-20 02:21:04]

justincohen@chromium.org changed reviewers:
+ marq@chromium.org, rohitrao@chromium.org

[justincohen, 2017-04-20 02:21:05]

Here's a concept CL for adding a dispatcher to old bling.  I'd like to use it to
help refactor the NTP/GoogleLandingPage.

The problem is the NTP can exist multiple times in old bling, meaning the
dispatcher may be 1:N.  I'm assuming this means it's a dead-end, and I have to
either find a different way around, or make the google landing mediator a
singleton?

[marq (ping after 24h), 2017-04-20 13:17:14]

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/google_landing_commands.h (right):

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/google_landing_commands.h:35: - (BOOL)canGoForward;
Our intent elsewhere has been that dispatched commands can only return void. In
general in the new world we're pushing state into the view layer, not providing
sources it can pull state from.

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/google_landing_mediator.h (right):

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/google_landing_mediator.h:34: @property(nonatomic,
assign) id dispatcher;
Actual type instead of id here, since you're calling dispatcher methods on it.

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/google_landing_mediator.mm (right):

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:339: [self.dispatcher
chromeExecuteCommand:command];
Where does the dispatching and handling for this get set up?

[justincohen, 2017-04-20 21:28:31]

ptal

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/google_landing_commands.h (right):

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/google_landing_commands.h:35: - (BOOL)canGoForward;
On 2017/04/20 13:17:14, marq wrote:
> Our intent elsewhere has been that dispatched commands can only return void.
In
> general in the new world we're pushing state into the view layer, not
providing
> sources it can pull state from.

Removed this file for now, since it will break the 1:M relationship.

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/google_landing_mediator.h (right):

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/google_landing_mediator.h:34: @property(nonatomic,
assign) id dispatcher;
On 2017/04/20 13:17:14, marq wrote:
> Actual type instead of id here, since you're calling dispatcher methods on it.

Done.

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/google_landing_mediator.mm (right):

https://codereview.chromium.org/2829003002/diff/20001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:339: [self.dispatcher
chromeExecuteCommand:command];
On 2017/04/20 13:17:14, marq wrote:
> Where does the dispatching and handling for this get set up?

in BVC.

[justincohen, 2017-04-20 21:29:46]

Description was changed from

==========
Add dispatchers concept

BUG=694750
==========

to

==========
Add CommandDispatcher to BrowserViewController.

Basic dispatcher to help with ios/clean migration.  Handlers UrlLoader, 
OmniboxFocuser and the WebToolbarDelegate, all used by the 
GoogleLandingController.

BUG=694750
==========

[marq (ping after 24h), 2017-04-21 09:09:57]

lgtm

[justincohen, 2017-04-22 06:30:16]

Over to rohitrao for review

[justincohen, 2017-04-25 12:51:24]

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

[commit-bot: I haz the power, 2017-04-25 12:51:40]

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-04-25 14:28:53]

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

[commit-bot: I haz the power, 2017-04-25 14:28:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rohitrao (ping after 24h), 2017-04-28 11:56:22]

lgtm

https://codereview.chromium.org/2829003002/diff/80001/ios/chrome/browser/ui/b...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2829003002/diff/80001/ios/chrome/browser/ui/b...
ios/chrome/browser/ui/browser_view_controller.mm:991: [_dispatcher
stopDispatchingToTarget:self];
The way the dispatcher is currently written, it's too late to call this in
dealloc.  (Dispatcher uses weak pointers, which have already been nilled out, so
nothing will be removed by this call.)

You'll need to move this call somewhere else, before dealloc, or stop
dispatching for each method name individually.

https://codereview.chromium.org/2829003002/diff/80001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/new_tab_page_toolbar_controller.h (right):

https://codereview.chromium.org/2829003002/diff/80001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_toolbar_controller.h:23: -
(instancetype)initWithToolbar;
- (instancetype)init;

https://codereview.chromium.org/2829003002/diff/80001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/ntp/ntp_home_coordinator.mm (right):

https://codereview.chromium.org/2829003002/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/ntp/ntp_home_coordinator.mm:58: [super stop];
You've got two [super stop]s in here.  Bad merge?

[justincohen, 2017-04-28 14:01:34]

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

[commit-bot: I haz the power, 2017-04-28 14:02:03]

Dry run: CQ is trying da patch.

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

[justincohen, 2017-04-28 14:02:21]

https://codereview.chromium.org/2829003002/diff/80001/ios/chrome/browser/ui/b...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2829003002/diff/80001/ios/chrome/browser/ui/b...
ios/chrome/browser/ui/browser_view_controller.mm:991: [_dispatcher
stopDispatchingToTarget:self];
On 2017/04/28 11:56:22, rohitrao (ping after 24h) wrote:
> The way the dispatcher is currently written, it's too late to call this in
> dealloc.  (Dispatcher uses weak pointers, which have already been nilled out,
so
> nothing will be removed by this call.)
> 
> You'll need to move this call somewhere else, before dealloc, or stop
> dispatching for each method name individually.

Done.

https://codereview.chromium.org/2829003002/diff/80001/ios/chrome/browser/ui/n...
File ios/chrome/browser/ui/ntp/new_tab_page_toolbar_controller.h (right):

https://codereview.chromium.org/2829003002/diff/80001/ios/chrome/browser/ui/n...
ios/chrome/browser/ui/ntp/new_tab_page_toolbar_controller.h:23: -
(instancetype)initWithToolbar;
On 2017/04/28 11:56:22, rohitrao (ping after 24h) wrote:
> - (instancetype)init;

Done.

https://codereview.chromium.org/2829003002/diff/80001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/ntp/ntp_home_coordinator.mm (right):

https://codereview.chromium.org/2829003002/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/ntp/ntp_home_coordinator.mm:58: [super stop];
On 2017/04/28 11:56:22, rohitrao (ping after 24h) wrote:
> You've got two [super stop]s in here.  Bad merge?

bad merge!

[justincohen, 2017-04-28 14:16:40]

The CQ bit was unchecked by justincohen@chromium.org

[justincohen, 2017-04-28 15:30:05]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2017-04-28 15:30:06]

The patchset sent to the CQ was uploaded after l-g-t-m from marq@chromium.org,
rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2829003002/#ps100001
(title: "rohit comments")

[commit-bot: I haz the power, 2017-04-28 15:30:40]

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-04-28 16:31:43]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493393405656030,
"parent_rev": "73dde766a935605604b2d7cca45033f8dbd9c6fb", "commit_rev":
"75011c3d5ea9d22e2317d6faa7d9901eb2d2a2ca"}

[commit-bot: I haz the power, 2017-04-28 16:33:29]

Description was changed from

==========
Add CommandDispatcher to BrowserViewController.

Basic dispatcher to help with ios/clean migration.  Handlers UrlLoader, 
OmniboxFocuser and the WebToolbarDelegate, all used by the 
GoogleLandingController.

BUG=694750
==========

to

==========
Add CommandDispatcher to BrowserViewController.

Basic dispatcher to help with ios/clean migration.  Handlers UrlLoader,
OmniboxFocuser and the WebToolbarDelegate, all used by the
GoogleLandingController.

BUG=694750

Review-Url: https://codereview.chromium.org/2829003002
Cr-Commit-Position: refs/heads/master@{#468023}
Committed:
https://chromium.googlesource.com/chromium/src/+/75011c3d5ea9d22e2317d6faa7d9...
==========

[commit-bot: I haz the power, 2017-04-28 16:33:30]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/75011c3d5ea9d22e2317d6faa7d9...