[ios] Adds a CommandDispatcher to proxy method calls to UI handlers.

ios/clean/chrome/browser/ui/commands/command_dispatcher.mm

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/clean/chrome/browser/ui/commands/command_dispatcher.h"
+
+#include <unordered_map>
+#include <vector>
+
+#include "base/logging.h"
+

[lpromero, 2017/03/07 10:04:17]

Missing ARC header.

[rohitrao (ping after 24h), 2017/03/09 14:41:11]

We should add this to boilerplate.py =)

[sdefresne, 2017/03/09 15:38:07]

This has already been done (https://codereview.chromium.org/2727903004/). You
were just unlucky and created this file before you got the fix :-)

[lpromero, 2017/03/09 16:16:30]

Well, Sylvain has struck again! https://codereview.chromium.org/2727903004

+@implementation CommandDispatcher {
+  std::unordered_map<SEL, __weak id> _forwardingTargets;
+}
+
+- (void)registerTarget:(id)target forSelector:(SEL)selector {
+  DCHECK(_forwardingTargets.find(selector) == _forwardingTargets.end());
+
+  _forwardingTargets[selector] = target;
+}
+
+- (void)stopDispatchingForTarget:(id)target {
+  std::vector<SEL> selectorsToErase;

[sdefresne, 2017/03/06 22:32:59]

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;
      });

[rohitrao (ping after 24h), 2017/03/09 14:41:11]

Kept my original code but changed it to a range-based for loop.

[sdefresne, 2017/03/09 15:38:07]

Looks better, thank you.

+  for (auto it = _forwardingTargets.begin(); it != _forwardingTargets.end();
+       ++it) {
+    if (it->second == target) {
+      selectorsToErase.push_back(it->first);
+    }
+  }
+
+  for (auto* selector : selectorsToErase) {
+    _forwardingTargets.erase(selector);
+  }
+}
+

[lpromero, 2017/03/07 10:04:17]

#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.

[rohitrao (ping after 24h), 2017/03/09 14:41:11]

Done.

+- (id)forwardingTargetForSelector:(SEL)selector {
+  auto target = _forwardingTargets.find(selector);
+  if (target != _forwardingTargets.end()) {
+    return target->second;
+  }
+  return nil;

[sdefresne, 2017/03/06 22:32:59]

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).

[rohitrao (ping after 24h), 2017/03/07 18:53:58]

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.

[sdefresne, 2017/03/07 19:12:35]

Ack. We'll have to watch for those uncaught Objective-C exceptions then. Do you
know if they are easy to spot in crash/?

[rohitrao (ping after 24h), 2017/03/09 14:41:11]

Let's revisit this in a few days.  I can try to add a CHECK in a followup CL.

+}
+
+@end