[ios] Adds support for Find in Page to the new architecture.

ios/clean/chrome/browser/ui/find_in_page/find_in_page_coordinator.mm

Index: ios/clean/chrome/browser/ui/find_in_page/find_in_page_coordinator.mm
diff --git a/ios/clean/chrome/browser/ui/find_in_page/find_in_page_coordinator.mm b/ios/clean/chrome/browser/ui/find_in_page/find_in_page_coordinator.mm
new file mode 100644
index 0000000000000000000000000000000000000000..651d348647d60584b21e60cf9d51ba9ef50b01b6
--- /dev/null
+++ b/ios/clean/chrome/browser/ui/find_in_page/find_in_page_coordinator.mm
@@ -0,0 +1,127 @@
+// 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/find_in_page/find_in_page_coordinator.h"
+
+#include <memory>
+
+#include "base/memory/ptr_util.h"
+#import "ios/chrome/browser/find_in_page/find_tab_helper.h"
+#import "ios/clean/chrome/browser/browser_coordinator+internal.h"
+#import "ios/clean/chrome/browser/model/browser.h"
+#import "ios/clean/chrome/browser/ui/commands/command_dispatcher.h"
+#import "ios/clean/chrome/browser/ui/commands/find_in_page_commands.h"
+#import "ios/clean/chrome/browser/ui/find_in_page/find_in_page_mediator.h"
+#import "ios/clean/chrome/browser/ui/find_in_page/find_in_page_view_controller.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+@interface FindInPageCoordinator ()<FindInPageCommands,
+                                    FindInPageMediatorDelegate>
+
+@property(nonatomic, readwrite, assign) BOOL started;

[lpromero, 2017/03/14 17:17:56]

Should this be a property of all coordinators? In the coordination context?

[marq (ping after 24h), 2017/03/16 13:37:05]

I don't think it should be in the context (which shouldn't contain mutable
state).

[rohitrao (ping after 24h), 2017/03/16 14:00:17]

I was picturing this as a property on BrowserCoordinator.

[lpromero, 2017/03/16 14:57:04]

Sidenote: One can change "animated" in the context for example. We could start
animated but stop non-animated.
I am fine with having it at the BrowserCoordinator level, especially since now
the -start and -stop methods are not no-op and are required to be called.

Rohit: I will make a CL.

[lpromero, 2017/03/16 17:12:30]

https://codereview.chromium.org/2750003004

+
+@property(nonatomic, readwrite, strong)
+    FindInPageViewController* viewController;
+
+@end
+
+@implementation FindInPageCoordinator {
+  std::unique_ptr<FindInPageMediator> mediator_;

[lpromero, 2017/03/14 17:17:56]

_mediator

+}
+
+@synthesize started = _started;
+@synthesize viewController = _viewController;
+
+#pragma mark - Coordinator lifecycle management
+
+- (void)coordinatorDidMoveToParent:(BrowserCoordinator*)parent {
+  DCHECK(self.browser);
+  mediator_ = base::MakeUnique<FindInPageMediator>(
+      &self.browser->web_state_list(), self);
+
+  // Register command handlers with the dispatcher.
+  CommandDispatcher* dispatcher = self.browser->dispatcher();

[rohitrao (ping after 24h), 2017/03/14 01:10:37]

3) Here's an example of command registration using the dispatcher.

+  [dispatcher registerTarget:self forSelector:@selector(showFindInPage)];
+  [dispatcher registerTarget:self forSelector:@selector(findStringInPage:)];
+  [dispatcher registerTarget:self forSelector:@selector(findNextInPage)];
+  [dispatcher registerTarget:self forSelector:@selector(findPreviousInPage)];
+  [dispatcher registerTarget:self forSelector:@selector(hideFindInPage)];
+}
+
+- (void)start {
+  self.started = YES;
+  self.viewController = [[FindInPageViewController alloc] init];
+  self.viewController.dispatcher = static_cast<id>(self.browser->dispatcher());

[rohitrao (ping after 24h), 2017/03/14 01:10:36]

4) Here's an example of casting the dispatcher to |id| and then passing it to
the view controller.

[lpromero, 2017/03/14 17:17:56]

And it's not possible to cast to id<FooCommands>, IIUC? The compiler doesn't
care we could put an id that doesn't implement the protocol?

[rohitrao (ping after 24h), 2017/03/16 14:00:17]

I'm not sure.  I'll try it and see.

[marq (ping after 24h), 2017/03/17 14:45:33]

Protocols aren't really part of the type system in ObjC, so while you can cast
to id<Foo>, there are no guarantees about whether the cast object actually
conforms to Foo. Any time we cast to a protocol, we should at least DCHECK
-conformsToProtocol:.

My assumption was that the dispatcher would explicitly conform to all of the
command protocols, so we can do simple assignment when hooking it up to clients.

[rohitrao (ping after 24h), 2017/03/20 23:37:55]

This is still an open question.  Conforming to the protocol doesn't actually
provide any safety here, because there's no guarantee that anyone has registered
as a handler for a given method and no guarantee on initialization order between
callers and callees.

In the absence of actual safety checks, we couldn't come up with a reason to
have the dispatcher publicly conform to every protocol.

+  mediator_->set_consumer(self.viewController);
+  [super start];
+}
+
+- (void)stop {
+  [super stop];
+  mediator_->set_consumer(nil);

[rohitrao (ping after 24h), 2017/03/14 01:10:37]

5) The consumer (UIVC) is destroyed when this coordinator is stopped, so we need
to push that through to the mediator.

+  self.started = NO;
+  self.viewController = nil;
+}
+
+#pragma mark - Mediator delegate
+
+- (void)createConsumer {
+  DCHECK(!self.started);
+  [self start];
+}
+
+#pragma mark - Command handlers
+
+- (void)showFindInPage {
+  NSLog(@"showFindInPage command handler");

[lpromero, 2017/03/14 17:17:56]

Remove

+  web::WebState* webState = self.browser->web_state_list().GetActiveWebState();
+  FindTabHelper::CreateForWebState(webState, nil);
+  if (!self.started)
+    [self start];
+}
+
+- (void)findStringInPage:(NSString*)searchTerm {
+  NSLog(@"findStringInPage command handler: %@", searchTerm);

[stkhapugin, 2017/03/14 14:51:06]

Can you add metrics logging: UserActions.Counts.Find* here and in next/prev? We
don't log these currently, but other platforms do. 

+  web::WebState* webState = self.browser->web_state_list().GetActiveWebState();
+
+  FindTabHelper* helper = FindTabHelper::FromWebState(webState);
+  helper->StartFinding(searchTerm, ^(FindInPageModel* model) {
+    mediator_->OnFindInPageResultsAvailable(model);

[rohitrao (ping after 24h), 2017/03/14 01:10:36]

6) This is an unfortunate side effect of the FIP API being block-based.  When I
switch it to callbacks or observers, this code will move into the mediator.

+  });
+}
+
+- (void)findNextInPage {
+  NSLog(@"findNextInPage command handler");

[lpromero, 2017/03/14 17:17:56]

Remove

+  if (!self.started)
+    [self start];

[rohitrao (ping after 24h), 2017/03/14 01:10:37]

7) This is an example of a command that can be called when the UI is closed. 
There are two ways to handle this, and I actually think I picked the wrong one.

a) Have the controller layer open the UI and then kick off a model query, which
is what I do here.

b) Have the controller layer kick off a model query, then have the mediator
trigger UI creation when results come back.

I actually think (b) is already hooked up, through the createConsumer call, so I
should be able to remove the call to [self start] from this method.

[lpromero, 2017/03/14 17:17:56]

* When you say controller layer, it's the coordinators, right?
* Regarding (b), you mean because the mediator calls createConsumer which calls
start? Seems hooked up.

[rohitrao (ping after 24h), 2017/03/16 14:00:17]

I use the phrase "controller layer" to collectively refer to UIVCs,
coordinators, and mediators.

+
+  web::WebState* webState = self.browser->web_state_list().GetActiveWebState();
+  FindTabHelper* helper = FindTabHelper::FromWebState(webState);
+  helper->ContinueFinding(FindTabHelper::FORWARD, ^(FindInPageModel* model) {
+    mediator_->OnFindInPageResultsAvailable(model);
+  });
+}
+
+- (void)findPreviousInPage {
+  NSLog(@"findPreviousInPage command handler");

[lpromero, 2017/03/14 17:17:56]

Remove.

+  if (!self.started)
+    [self start];
+
+  web::WebState* webState = self.browser->web_state_list().GetActiveWebState();
+  FindTabHelper* helper = FindTabHelper::FromWebState(webState);
+  helper->ContinueFinding(FindTabHelper::REVERSE, ^(FindInPageModel* model) {
+    mediator_->OnFindInPageResultsAvailable(model);
+  });
+}
+
+- (void)hideFindInPage {
+  NSLog(@"hideFindInPage command handler");
+  if (self.started)
+    [self stop];
+}
+
+@end