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

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

Index: ios/clean/chrome/browser/ui/find_in_page/find_in_page_view_controller.mm
diff --git a/ios/clean/chrome/browser/ui/find_in_page/find_in_page_view_controller.mm b/ios/clean/chrome/browser/ui/find_in_page/find_in_page_view_controller.mm
new file mode 100644
index 0000000000000000000000000000000000000000..4ff81892910cc547618c2bb244a5545e9efe4fbf
--- /dev/null
+++ b/ios/clean/chrome/browser/ui/find_in_page/find_in_page_view_controller.mm
@@ -0,0 +1,110 @@
+// 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_view_controller.h"
+
+#include "base/format_macros.h"
+#include "base/strings/sys_string_conversions.h"
+#include "components/strings/grit/components_strings.h"
+#import "ios/chrome/browser/ui/find_bar/find_bar_view.h"
+#include "ui/base/l10n/l10n_util_mac.h"
+

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

Missing ARC header.

+namespace {
+// For the first |kSearchDelayChars| characters, delay by |kSearchLongDelay|

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

Missing period.

+// For the remaining characters, delay by |kSearchShortDelay|.
+const NSUInteger kSearchDelayChars = 3;
+const NSTimeInterval kSearchLongDelay = 1.0;
+const NSTimeInterval kSearchShortDelay = 0.100;
+}  // namespace
+
+@interface FindInPageViewController ()
+// Redeclare the |view| property to be the FindBarView subclass instead of a
+// generic UIView.
+@property(nonatomic, readwrite, strong) FindBarView* view;

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

Do you need the readwrite?

+
+// Typing delay timer.
+@property(nonatomic, strong) NSTimer* delayTimer;
+@end
+
+@implementation FindInPageViewController
+
+@dynamic view;
+@synthesize delayTimer = _delayTimer;
+@synthesize dispatcher = _dispatcher;
+
+- (void)viewDidLoad {
+  FindBarView* findBarView = [[FindBarView alloc] initWithDarkAppearance:NO];
+  findBarView.backgroundColor = [UIColor whiteColor];
+  [findBarView.inputField addTarget:self
+                             action:@selector(findBarTextDidChange)
+                   forControlEvents:UIControlEventEditingChanged];
+  [findBarView.nextButton addTarget:self
+                             action:@selector(findNextInPage)
+                   forControlEvents:UIControlEventTouchUpInside];

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

10) Example of a VC setting an action on a control and setting the target to
|self|, rather than using the responder chain.

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

This means that the action protocol is either not needed, or can just live in
the feature's ui/ subdirectory -- that is, ui/actions/ can go away.

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

This is a consequence of actions becoming specific to a single view->VC pair.

A huge downside to the code I have here is that all of the actions need to be
set by the VC.  I believe there are two ways to send actions to targets:
1) Send the action to "nil" and have it propagate up the responder chain until
something handles it.
2) Send the action to a specific object, which throws an exception if that
object does not handle it.

I don't think there's a way for a button to send its action to |self| and have
that propagate up to the VC.  But the button also isn't necessarily the first
responder, so there's no guarantee that sending an action to nil will work,
because the button and VC may not be in the responder chain.

The problem with this is that buttons can't configure their actions in
isolation, because they don't have a direct pointer to their VC.  This would
force us to put all of the action setting code in the VC.

[marq (ping after 24h), 2017/04/06 16:13:26]

I agree this is a problem, and it's one of the reasons we were keen on the
responder chain initially; a control could be entirely self-contained and
reusable without having to set it up at all. 

I don't have a good answer, unfortunately.

+  [findBarView.previousButton addTarget:self
+                                 action:@selector(findPreviousInPage)
+                       forControlEvents:UIControlEventTouchUpInside];
+  [findBarView.closeButton addTarget:self
+                              action:@selector(hideFindInPage)
+                    forControlEvents:UIControlEventTouchUpInside];
+  self.view = findBarView;

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

This should be done in -loadView.

+
+  [NSLayoutConstraint activateConstraints:[self subviewConstraints]];
+}
+
+- (NSArray*)subviewConstraints {
+  return @[];
+}
+
+- (void)setCurrentMatch:(NSUInteger)current ofTotalMatches:(NSUInteger)total {
+  NSString* indexStr = [NSString stringWithFormat:@"%" PRIdNS, current];
+  NSString* matchesStr = [NSString stringWithFormat:@"%" PRIdNS, total];
+  NSString* text = l10n_util::GetNSStringF(
+      IDS_FIND_IN_PAGE_COUNT, base::SysNSStringToUTF16(indexStr),
+      base::SysNSStringToUTF16(matchesStr));
+  [static_cast<FindBarView*>(self.view) updateResultsLabelWithText:text];

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

Shouldn't we use base::mac::ObjCCast(Strict) instead?

+}
+
+- (void)textChanged {
+  [self.dispatcher findStringInPage:self.view.inputField.text];
+}
+
+#pragma mark - Actions
+
+- (void)showFindInPage {

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

This seems like it means -showFindInPage either should be optional or not part
of the FindInPageActions protocol.

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

Correct.  I had it in the protocol originally, but then I realized that the find
bar ui never calls "show" (because it's already visible).  This probably belongs
only in the tools menu action protocol.

+  NOTREACHED();
+  [self.dispatcher showFindInPage];
+}
+
+- (void)findNextInPage {
+  [self.dispatcher findNextInPage];

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

11) Example of a VC catching an action and forwarding it to the dispatcher.

+}
+
+- (void)findPreviousInPage {
+  [self.dispatcher findPreviousInPage];
+}
+
+- (void)hideFindInPage {
+  [self.dispatcher hideFindInPage];
+}
+
+- (void)findBarTextDidChange {
+  [self.delayTimer invalidate];
+  NSUInteger length = [self.view.inputField.text length];
+  if (length == 0)
+    return [self textChanged];

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

This would be more readable if on two lines:
if (length == 0) {
  [self textChanged];
  return;
}

+
+  // Delay delivery of text change event.  Use a longer delay when the input
+  // length is short.
+  NSTimeInterval delay =
+      (length > kSearchDelayChars) ? kSearchShortDelay : kSearchLongDelay;
+  self.delayTimer =
+      [NSTimer scheduledTimerWithTimeInterval:delay
+                                       target:self
+                                     selector:@selector(textChanged)
+                                     userInfo:nil
+                                      repeats:NO];
+}
+
+@end