Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2734863002:
[ios] Adds a CommandDispatcher to proxy method calls to UI handlers. (Closed)
|
Created:
3 years, 9 months ago by rohitrao (ping after 24h) Modified:
3 years, 9 months ago CC:
chromium-reviews, marq+scrutinize_chromium.org, lpromero+watch_chromium.org, ios-reviews+clean_chromium.org, ios-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[ios] Adds a CommandDispatcher to proxy method calls to UI handlers.
This class will be used by the command architecture in the clean skeleton app.
View controllers will receive a pointer to a CommandDispatcher and can use it to
send command methods to the appropriate coordinators.
BUG=698748
TEST=None
Review-Url: https://codereview.chromium.org/2734863002
Cr-Commit-Position: refs/heads/master@{#456075}
Committed: https://chromium.googlesource.com/chromium/src/+/5dc19b0874e342a145a3a648a2c82a05406065c0
Patch Set 1 #Patch Set 2 : Fix GN. #
Total comments: 17
Patch Set 3 : Review. #
Total comments: 8
Messages
Total messages: 36 (18 generated)
The CQ bit was checked by rohitrao@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
rohitrao@chromium.org changed reviewers: + sdefresne@chromium.org
Rough draft. Is this all I need, or can you think of anything that's missing? Any other unittests that I should add? Right now the dispatcher doesn't conform to any protocols, so I need to use it as an "id" in order to avoid compiler errors. Can we do anything smarter?
The CQ bit was unchecked by commit-bot@chromium.org
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...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by rohitrao@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Yes, I think that this is all that is needed to implement the CommandDispatcher as @selector are guaranteed to be unique by the compiler (https://developer.apple.com/library/content/documentation/General/Conceptual/...). To make the code a bit safer, I would recommend defining @protocol for the different sets of methods where are interested in going through the dispatcher (not idea how to name them), have the CommandDispatcher implements them in the @implementation (to avoid bringing all of them in the header file), and then have client add conformsToProtocol: call in their setter when they receive the dispatcher. Let say you have this: command_dispatcher.h @interface CommandDispatcher : NSObject @end command_dispatcher.mm @interface CommandDispatcher () <HideCommand> @end hide_command.h @protocol HideCommand - (void)hide; @end some_controller.h @interface SomeController - (void)setDispatcher:(id)dispatcher; - (void)doSomething; @end hide_client.mm @implementation SomeController { __weak id<HideCommand> _dispatcher; } - (void)setDispatcher:(id)dispatcher { DCHECK([dispatcher conformsToProtocol:@protocol(HideCommand)]); _dispatcher = dispatcher; } - (void)doSomething { [_dispatcher hide]; } @end https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:23: std::vector<SEL> selectorsToErase; I'm wondering whether using C++ algorithms is simpler or not: std::vector<std::pair<const SEL, id>> elementsToErase; std::copy_if( _forwardingTargets.begin(), _forwardingTargets.end(), std::back_inserter(elementsToErase), [target](std::pair<const SEL, id> element) { return element->second == target; }); https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:41: return nil; I think we should NOTREACHED() here as this will give us a nice callstack (Objective-C exception terminate the app without any callstack). Maybe CHECK(false).
lpromero@chromium.org changed reviewers: + lpromero@chromium.org
To Sylvain's point about private conformance to a protocol: I would put it publicly. For example if the dispatcher was coming from a prebuilt third-party library, how would client know when they can use the dispatcher? The proposed DCHECK in - (void)setDispatcher:(id)dispatcher would be a game of trials and errors. https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:11: Missing ARC header. https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:35: #pragma mark - NSObject to signify that this is a Foundation method. Also in the class comment, I think we should mention that we rely on message forwarding and clients can just call a registered selector on an instance of this. https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher_unittest.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher_unittest.mm:11: Missing ARC header.
On 2017/03/07 10:04:17, lpromero wrote: > To Sylvain's point about private conformance to a protocol: I would put it > publicly. For example if the dispatcher was coming from a prebuilt third-party > library, how would client know when they can use the dispatcher? The proposed > DCHECK in - (void)setDispatcher:(id)dispatcher would be a game of trials and > errors. The problem is that we expect to have ~40 or so protocols (one for each command and we have around that number of commands). We do not want clients to know about all the commands. Putting the inheritance in the public API will defeat that. If you think that adding the DCHECK is not useful, then I would prefer not having them, and not having the CommandDispatcher implements any of the protocols either and just use untyped "id". > https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... > File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): > > https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... > ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:11: > Missing ARC header. > > https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... > ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:35: > #pragma mark - NSObject > to signify that this is a Foundation method. Also in the class comment, I think > we should mention that we rely on message forwarding and clients can just call a > registered selector on an instance of this. > > https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... > File ios/clean/chrome/browser/ui/commands/command_dispatcher_unittest.mm > (right): > > https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... > ios/clean/chrome/browser/ui/commands/command_dispatcher_unittest.mm:11: > Missing ARC header.
On 2017/03/07 17:04:16, sdefresne wrote: > The problem is that we expect to have ~40 or so protocols (one for each command > and we have around that number of commands). We do not want clients to know > about all the commands. Putting the inheritance in the public API will defeat > that. If you think that adding the DCHECK is not useful, then I would prefer not > having them, and not having the CommandDispatcher implements any of the > protocols either and just use untyped "id". Won't clients only know the dispatcher as an id<SomeCommands>?
On 2017/03/07 17:58:43, lpromero wrote: > On 2017/03/07 17:04:16, sdefresne wrote: > > The problem is that we expect to have ~40 or so protocols (one for each > command > > and we have around that number of commands). We do not want clients to know > > about all the commands. Putting the inheritance in the public API will defeat > > that. If you think that adding the DCHECK is not useful, then I would prefer > not > > having them, and not having the CommandDispatcher implements any of the > > protocols either and just use untyped "id". > > Won't clients only know the dispatcher as an id<SomeCommands>? The problem is how to safely converting from "id" to "id<SomeCommands>".
I'm not convinced that conforming to protocols will actually help. Even if the dispatcher conforms to every possible protocol (publicly or privately), there is no guarantee that some object will register to handle the messages for a given protocol. In this case, conformance does not mean an implementation will actually exist. Overall, the dispatcher doesn't really offer compile-time guarantees for anything. The best I can offer is a runtime DCHECK, in case we call a method that isn't handled. Is this acceptable behavior, or should it be a deal-killer? https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:41: return nil; On 2017/03/06 22:32:59, sdefresne wrote: > I think we should NOTREACHED() here as this will give us a nice callstack > (Objective-C exception terminate the app without any callstack). Maybe > CHECK(false). I can't put a check here, because this object will handle some methods directly (init/dealloc/retain/release), so there won't be a forwarding target. I could instead override doesNotRecognizeSelector and put a DCHECK() in there, but it would make testing slightly more annoying.
https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.h (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:10: @interface CommandDispatcher : NSObject Can you document the class and public methods? https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:41: return nil; On 2017/03/07 18:53:58, rohitrao wrote: > On 2017/03/06 22:32:59, sdefresne wrote: > > I think we should NOTREACHED() here as this will give us a nice callstack > > (Objective-C exception terminate the app without any callstack). Maybe > > CHECK(false). > > I can't put a check here, because this object will handle some methods directly > (init/dealloc/retain/release), so there won't be a forwarding target. I could > instead override doesNotRecognizeSelector and put a DCHECK() in there, but it > would make testing slightly more annoying. Ack. We'll have to watch for those uncaught Objective-C exceptions then. Do you know if they are easy to spot in crash/?
On 2017/03/07 18:53:59, rohitrao wrote: > I'm not convinced that conforming to protocols will actually help. Even if the > dispatcher conforms to every possible protocol (publicly or privately), there is > no guarantee that some object will register to handle the messages for a given > protocol. In this case, conformance does not mean an implementation will > actually exist. Acknowledged. > Overall, the dispatcher doesn't really offer compile-time guarantees for > anything. The best I can offer is a runtime DCHECK, in case we call a method > that isn't handled. > > Is this acceptable behavior, or should it be a deal-killer? I don't think this is a deal breaker. The alternative (NSNotification) is worse because it uses strings instead of selector, so it is even harder to look for. So, we should probably have the CommandHandler implements no protocols (client can double cast it to "id" then "id<SomeCommand>" to have code completion of the list of selector that can be invoked without any real safety they are supported). > https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... > File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): > > https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... > ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:41: return nil; > On 2017/03/06 22:32:59, sdefresne wrote: > > I think we should NOTREACHED() here as this will give us a nice callstack > > (Objective-C exception terminate the app without any callstack). Maybe > > CHECK(false). > > I can't put a check here, because this object will handle some methods directly > (init/dealloc/retain/release), so there won't be a forwarding target. I could > instead override doesNotRecognizeSelector and put a DCHECK() in there, but it > would make testing slightly more annoying. Should we add a method "-(BOOL) isSupportedSelector:(SEL)selector"? That would allow us to test the method, and we can add DCHECK on the calling site. WDYT?
The CQ bit was checked by rohitrao@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.h (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:10: @interface CommandDispatcher : NSObject On 2017/03/07 19:12:35, sdefresne wrote: > Can you document the class and public methods? Done. https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:11: On 2017/03/07 10:04:17, lpromero wrote: > Missing ARC header. We should add this to boilerplate.py =) https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:23: std::vector<SEL> selectorsToErase; On 2017/03/06 22:32:59, sdefresne wrote: > I'm wondering whether using C++ algorithms is simpler or not: > > std::vector<std::pair<const SEL, id>> elementsToErase; > std::copy_if( > _forwardingTargets.begin(), > _forwardingTargets.end(), > std::back_inserter(elementsToErase), > [target](std::pair<const SEL, id> element) { > return element->second == target; > }); Kept my original code but changed it to a range-based for loop. https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:35: On 2017/03/07 10:04:17, lpromero wrote: > #pragma mark - NSObject > to signify that this is a Foundation method. Also in the class comment, I think > we should mention that we rely on message forwarding and clients can just call a > registered selector on an instance of this. Done. https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:41: return nil; On 2017/03/07 19:12:35, sdefresne wrote: > On 2017/03/07 18:53:58, rohitrao wrote: > > On 2017/03/06 22:32:59, sdefresne wrote: > > > I think we should NOTREACHED() here as this will give us a nice callstack > > > (Objective-C exception terminate the app without any callstack). Maybe > > > CHECK(false). > > > > I can't put a check here, because this object will handle some methods > directly > > (init/dealloc/retain/release), so there won't be a forwarding target. I could > > instead override doesNotRecognizeSelector and put a DCHECK() in there, but it > > would make testing slightly more annoying. > > Ack. We'll have to watch for those uncaught Objective-C exceptions then. Do you > know if they are easy to spot in crash/? Let's revisit this in a few days. I can try to add a CHECK in a followup CL. https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher_unittest.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher_unittest.mm:11: On 2017/03/07 10:04:17, lpromero wrote: > Missing ARC header. Done.
lgtm https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:11: On 2017/03/09 14:41:11, rohitrao wrote: > On 2017/03/07 10:04:17, lpromero wrote: > > Missing ARC header. > > We should add this to boilerplate.py =) This has already been done (https://codereview.chromium.org/2727903004/). You were just unlucky and created this file before you got the fix :-) https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:23: std::vector<SEL> selectorsToErase; On 2017/03/09 14:41:11, rohitrao wrote: > On 2017/03/06 22:32:59, sdefresne wrote: > > I'm wondering whether using C++ algorithms is simpler or not: > > > > std::vector<std::pair<const SEL, id>> elementsToErase; > > std::copy_if( > > _forwardingTargets.begin(), > > _forwardingTargets.end(), > > std::back_inserter(elementsToErase), > > [target](std::pair<const SEL, id> element) { > > return element->second == target; > > }); > > Kept my original code but changed it to a range-based for loop. Looks better, thank you.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.mm (right): https://codereview.chromium.org/2734863002/diff/20001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.mm:11: On 2017/03/09 14:41:11, rohitrao wrote: > On 2017/03/07 10:04:17, lpromero wrote: > > Missing ARC header. > > We should add this to boilerplate.py =) Well, Sylvain has struck again! https://codereview.chromium.org/2727903004 https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.h (right): https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:20: - (void)stopDispatchingForTarget:(id)target; s/…/unregisterTarget: ?
The CQ bit was checked by rohitrao@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.h (right): https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:20: - (void)stopDispatchingForTarget:(id)target; On 2017/03/09 16:16:30, lpromero wrote: > s/…/unregisterTarget: ? I went through a number of iterations: unregisterTarget deregisterTarget stopForwardingForTarget Eventually I settled on stopDispatchingForTarget because it sounded the least odd. I can change the name if no one else likes it =) I've also been a bit inconsistent about "target" vs "handler". Do you have a preference for one vs the other?
https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.h (right): https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:17: - (void)registerTarget:(id)target forSelector:(SEL)selector; The comment doesn't specify that you can't have several targets for the same selector. Actually, what happens when that case arises? https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:20: - (void)stopDispatchingForTarget:(id)target; On 2017/03/10 14:44:31, rohitrao wrote: > On 2017/03/09 16:16:30, lpromero wrote: > > s/…/unregisterTarget: ? > > I went through a number of iterations: > unregisterTarget > deregisterTarget > stopForwardingForTarget > > Eventually I settled on stopDispatchingForTarget because it sounded the least > odd. I can change the name if no one else likes it =) > > I've also been a bit inconsistent about "target" vs "handler". Do you have a > preference for one vs the other? Handler might convey more that fact that we can only have one, no?
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489156950209940,
"parent_rev": "679eb85f60078ada8e9c57cc56db76b6c4cee54f", "commit_rev":
"5dc19b0874e342a145a3a648a2c82a05406065c0"}
Message was sent while issue was closed.
Description was changed from ========== [ios] Adds a CommandDispatcher to proxy method calls to UI handlers. This class will be used by the command architecture in the clean skeleton app. View controllers will receive a pointer to a CommandDispatcher and can use it to send command methods to the appropriate coordinators. BUG=698748 TEST=None ========== to ========== [ios] Adds a CommandDispatcher to proxy method calls to UI handlers. This class will be used by the command architecture in the clean skeleton app. View controllers will receive a pointer to a CommandDispatcher and can use it to send command methods to the appropriate coordinators. BUG=698748 TEST=None Review-Url: https://codereview.chromium.org/2734863002 Cr-Commit-Position: refs/heads/master@{#456075} Committed: https://chromium.googlesource.com/chromium/src/+/5dc19b0874e342a145a3a648a2c8... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/5dc19b0874e342a145a3a648a2c8...
Message was sent while issue was closed.
marq@chromium.org changed reviewers: + marq@chromium.org
Message was sent while issue was closed.
Post-hoc comments. https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.h (right): https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:13: @interface CommandDispatcher : NSObject Doesn't CommandDispatcher need to publicly conform to all of the command protocols? https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:20: - (void)stopDispatchingForTarget:(id)target; On 2017/03/10 16:17:36, lpromero wrote: > On 2017/03/10 14:44:31, rohitrao wrote: > > On 2017/03/09 16:16:30, lpromero wrote: > > > s/…/unregisterTarget: ? > > > > I went through a number of iterations: > > unregisterTarget > > deregisterTarget > > stopForwardingForTarget > > > > Eventually I settled on stopDispatchingForTarget because it sounded the least > > odd. I can change the name if no one else likes it =) > > > > I've also been a bit inconsistent about "target" vs "handler". Do you have a > > preference for one vs the other? > > Handler might convey more that fact that we can only have one, no? I like -stopDispatching:, so much that I'd like to see the other method become dispatchToTarget: forSelector:
Message was sent while issue was closed.
https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... File ios/clean/chrome/browser/ui/commands/command_dispatcher.h (right): https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:13: @interface CommandDispatcher : NSObject On 2017/03/16 13:54:46, marq wrote: > Doesn't CommandDispatcher need to publicly conform to all of the command > protocols? This is discussed upthread. https://codereview.chromium.org/2734863002/diff/40001/ios/clean/chrome/browse... ios/clean/chrome/browser/ui/commands/command_dispatcher.h:20: - (void)stopDispatchingForTarget:(id)target; On 2017/03/16 13:54:46, marq wrote: > On 2017/03/10 16:17:36, lpromero wrote: > > On 2017/03/10 14:44:31, rohitrao wrote: > > > On 2017/03/09 16:16:30, lpromero wrote: > > > > s/…/unregisterTarget: ? > > > > > > I went through a number of iterations: > > > unregisterTarget > > > deregisterTarget > > > stopForwardingForTarget > > > > > > Eventually I settled on stopDispatchingForTarget because it sounded the > least > > > odd. I can change the name if no one else likes it =) > > > > > > I've also been a bit inconsistent about "target" vs "handler". Do you have > a > > > preference for one vs the other? > > > > Handler might convey more that fact that we can only have one, no? > > I like -stopDispatching:, so much that I'd like to see the other method become > dispatchToTarget: forSelector: https://codereview.chromium.org/2766663002/ |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
