[clean] Use CommandDispatcher to show/close Settings.

[clean] Use CommandDispatcher to show/close Settings.

BUG=none
R=marq@chromium.org

Review-Url: https://codereview.chromium.org/2779213003
Cr-Commit-Position: refs/heads/master@{#460743}
Committed: https://chromium.googlesource.com/chromium/src/+/39d403eb61f5598b7308a36decfbfe8aaee3e97d

[lpromero, 2017-03-29 17:47:14]

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

[commit-bot: I haz the power, 2017-03-29 17:47:48]

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

[lpromero, 2017-03-29 17:48:47]

https://codereview.chromium.org/2779213003/diff/1/ios/clean/chrome/browser/ui...
File ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm (right):

https://codereview.chromium.org/2779213003/diff/1/ios/clean/chrome/browser/ui...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm:138: [dispatcher
stopDispatchingToTarget:self];
This will be problematic if this controller starts listening for other
selectors. This is a stopAll method. Should I add a fine-grained
stopDispatchingToTarget:forSelector:?

[commit-bot: I haz the power, 2017-03-29 17:56:06]

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

[commit-bot: I haz the power, 2017-03-29 17:56:07]

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

[marq (ping after 24h), 2017-03-30 07:53:34]

https://codereview.chromium.org/2779213003/diff/1/ios/clean/chrome/browser/ui...
File ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm (right):

https://codereview.chromium.org/2779213003/diff/1/ios/clean/chrome/browser/ui...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm:138: [dispatcher
stopDispatchingToTarget:self];
On 2017/03/29 17:48:46, lpromero wrote:
> This will be problematic if this controller starts listening for other
> selectors. This is a stopAll method. Should I add a fine-grained
> stopDispatchingToTarget:forSelector:?

Yes, or maybe (also?) -stopDispatchingToTarget:forProtocol:

[marq (ping after 24h), 2017-03-30 07:53:46]

But also: LGTM

[lpromero, 2017-03-30 11:45:22]

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

[lpromero, 2017-03-30 11:45:26]

https://codereview.chromium.org/2779213003/diff/1/ios/clean/chrome/browser/ui...
File ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm (right):

https://codereview.chromium.org/2779213003/diff/1/ios/clean/chrome/browser/ui...
ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm:138: [dispatcher
stopDispatchingToTarget:self];
On 2017/03/30 07:53:33, marq wrote:
> On 2017/03/29 17:48:46, lpromero wrote:
> > This will be problematic if this controller starts listening for other
> > selectors. This is a stopAll method. Should I add a fine-grained
> > stopDispatchingToTarget:forSelector:?
> 
> Yes, or maybe (also?) -stopDispatchingToTarget:forProtocol:

Done for the individual method. I don't pass the target as it's not used (or
maybe we could DCHECK?).
I'd rather add this in a CL adding startDispatchingToTarget:forProtocol:.

[commit-bot: I haz the power, 2017-03-30 11:45:36]

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

[lpromero, 2017-03-30 11:51:41]

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

[commit-bot: I haz the power, 2017-03-30 11:51:48]

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

[lpromero, 2017-03-30 12:05:00]

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

[commit-bot: I haz the power, 2017-03-30 12:05:18]

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

[lpromero, 2017-03-30 12:50:11]

The CQ bit was checked by lpromero@chromium.org

[lpromero, 2017-03-30 12:50:12]

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/2779213003/#ps60001
(title: "Fix tests")

[commit-bot: I haz the power, 2017-03-30 12:50:18]

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-30 13:41:04]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490878211948390,
"parent_rev": "9093358a6c479fdf41bb40ec5203850608240600", "commit_rev":
"39d403eb61f5598b7308a36decfbfe8aaee3e97d"}

[commit-bot: I haz the power, 2017-03-30 13:41:45]

Description was changed from

==========
[clean] Use CommandDispatcher to show/close Settings.

BUG=none
R=marq@chromium.org
==========

to

==========
[clean] Use CommandDispatcher to show/close Settings.

BUG=none
R=marq@chromium.org

Review-Url: https://codereview.chromium.org/2779213003
Cr-Commit-Position: refs/heads/master@{#460743}
Committed:
https://chromium.googlesource.com/chromium/src/+/39d403eb61f5598b7308a36decfb...
==========

[commit-bot: I haz the power, 2017-03-30 13:41:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/39d403eb61f5598b7308a36decfb...