Implement the basic testing infrastructure for FindRequestManager.

content/browser/find_request_manager_browsertest.cc

Index: content/browser/find_request_manager_browsertest.cc
diff --git a/content/browser/find_request_manager_browsertest.cc b/content/browser/find_request_manager_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c57ddffd9df2ff6629e8a3ab28640fb838b52ed3
--- /dev/null
+++ b/content/browser/find_request_manager_browsertest.cc
@@ -0,0 +1,135 @@
+// Copyright 2016 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.
+
+#include "base/command_line.h"
+#include "base/strings/utf_string_conversions.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/common/content_switches.h"
+#include "content/public/test/content_browser_test.h"
+#include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "content/shell/browser/shell.h"
+#include "content/test/test_find_request_manager.h"
+#include "net/dns/mock_host_resolver.h"
+#include "third_party/WebKit/public/web/WebFindOptions.h"
+
+namespace content {
+
+using FindResults = TestFindRequestManager::FindResults;
+
+class FindRequestManagerTest : public ContentBrowserTest {
+ public:
+  FindRequestManagerTest()
+      : find_request_manager_(nullptr) {}
+
+ protected:

[nasko, 2016/04/15 19:26:50]

What is the goal of the protected section here? This class is not inherited as
far as I can see.

[paulmeyer, 2016/04/18 19:11:22]

All of the individual tests themselves inherit from this class, so methods in
the "protected" section are accessible to the individual tests. Some other
browser tests seem to do the same:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/ne...

+  void LoadAndWait(const std::string& url) {
+    WindowedNotificationObserver load_observer(

[nasko, 2016/04/15 19:26:50]

Use TestNavigationObserver whenever possible. It does not rely on
NotificationService, which is deprecated, and more closely monitors navigations.

[paulmeyer, 2016/04/18 19:11:22]

Done.

+        NOTIFICATION_LOAD_STOP, Source<NavigationController>(
+            &shell()->web_contents()->GetController()));
+    NavigateToURL(shell(), embedded_test_server()->GetURL("a.com", url));
+    load_observer.Wait();
+  }
+
+  TestFindRequestManager* find_request_manager() {
+    return find_request_manager_;
+  }
+
+ private:
+  void SetUpOnMainThread() override {

[nasko, 2016/04/15 19:26:50]

No need for publicly inherited methods to be private.

[paulmeyer, 2016/04/18 19:11:22]

I've always believed in giving the most limited access possible to class
members. I get that in this case it doesn't really limit the access though, so I
can move them to public.

[nasko, 2016/04/18 21:02:39]

However, you still can call those methods through a base class pointer, as the
inheritance is public, so it doesn't really enforce access very well.

+    host_resolver()->AddRule("*", "127.0.0.1");
+    ASSERT_TRUE(embedded_test_server()->Start());
+
+    WebContentsImpl* contents =
+        static_cast<WebContentsImpl*>(shell()->web_contents());
+    find_request_manager_ = new TestFindRequestManager(contents);
+    contents->find_request_manager_.reset(find_request_manager_);
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(switches::kSitePerProcess);

[nasko, 2016/04/15 19:26:50]

IsolateAllSitesForTesting(command_line); which does the same thing, but it
abstracts it out in a method we can easily affect with single change.

[paulmeyer, 2016/04/18 19:11:22]

Done.

+  }
+
+  TestFindRequestManager* find_request_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(FindRequestManagerTest);
+};
+
+// Test basic find-in-page functionality (such as searching forward and
+// backward) and check for correct results at each step.
+IN_PROC_BROWSER_TEST_F(FindRequestManagerTest, Basic) {
+  LoadAndWait("/find_in_page.html");
+
+  blink::WebFindOptions options;
+  find_request_manager()->Find(1, base::UTF8ToUTF16("result"), options);

[nasko, 2016/04/15 19:26:50]

I know this file is trying to target the FindRequestManager for testing, but I
wonder if we will have any tests that would exercise find-in-page through the
WebContents interface. Since all code will invoke the FRM through WebContents,
should this test invoke searching the same way?

[paulmeyer, 2016/04/18 19:11:22]

The tests now access WebContents::Find() and check results via
WebContentsDelegate.

[nasko, 2016/04/18 21:02:39]

Awesome! Thanks!

+  find_request_manager()->WaitForFinalReply();
+
+  FindResults results = find_request_manager()->GetFindResults();
+  EXPECT_EQ(1, results.request_id);
+  EXPECT_EQ(19, results.number_of_matches);
+  EXPECT_EQ(1, results.active_match_ordinal);
+
+  options.findNext = true;
+  for (int i = 2; i <= 10; ++i) {
+    find_request_manager()->Find(i, base::UTF8ToUTF16("result"), options);

[nasko, 2016/04/15 19:26:50]

Would using |L"result"| work instead of |base::UTF8ToUTF16("result")|? It might
be a bit more readable.

[paulmeyer, 2016/04/18 19:11:22]

I tried it and it actually doesn't work.

+    find_request_manager()->WaitForFinalReply();
+    results = find_request_manager()->GetFindResults();
+
+    EXPECT_EQ(i, results.request_id);
+    EXPECT_EQ(19, results.number_of_matches);
+    EXPECT_EQ(i, results.active_match_ordinal);
+  }
+
+  options.forward = false;
+  for (int i = 11; i <= 15; ++i) {
+    find_request_manager()->Find(i, base::UTF8ToUTF16("result"), options);
+    find_request_manager()->WaitForFinalReply();
+    results = find_request_manager()->GetFindResults();
+
+    EXPECT_EQ(i, results.request_id);
+    EXPECT_EQ(19, results.number_of_matches);
+    EXPECT_EQ(20 - i, results.active_match_ordinal);
+  }
+}
+
+// Tests searching for a word character-by-character, as would typically be done
+// by a user typing into the find bar.
+IN_PROC_BROWSER_TEST_F(FindRequestManagerTest, CharacterByCharacter) {
+  LoadAndWait("/find_in_page.html");
+
+  blink::WebFindOptions default_options;
+  find_request_manager()->Find(1, base::UTF8ToUTF16("r"), default_options);
+  find_request_manager()->Find(2, base::UTF8ToUTF16("re"), default_options);
+  find_request_manager()->Find(3, base::UTF8ToUTF16("res"), default_options);
+  find_request_manager()->Find(4, base::UTF8ToUTF16("resu"), default_options);
+  find_request_manager()->Find(5, base::UTF8ToUTF16("resul"), default_options);
+  find_request_manager()->Find(6, base::UTF8ToUTF16("result"), default_options);
+  find_request_manager()->WaitForFinalReply();
+
+  FindResults results = find_request_manager()->GetFindResults();
+  EXPECT_EQ(6, results.request_id);
+  EXPECT_EQ(19, results.number_of_matches);
+  EXPECT_EQ(1, results.active_match_ordinal);
+}
+
+// Test sending a large number of find requests subsequently.
+IN_PROC_BROWSER_TEST_F(FindRequestManagerTest, RapidFire) {
+  LoadAndWait("/find_in_page.html");
+
+  blink::WebFindOptions options;
+  find_request_manager()->Find(1, base::UTF8ToUTF16("result"), options);
+
+  options.findNext = true;
+  for (int i = 2; i <= 1000; ++i)
+    find_request_manager()->Find(i, base::UTF8ToUTF16("result"), options);
+  find_request_manager()->WaitForFinalReply();
+  FindResults results = find_request_manager()->GetFindResults();
+
+  EXPECT_EQ(1000, results.request_id);
+  EXPECT_EQ(19, results.number_of_matches);
+  EXPECT_EQ(1000 % results.number_of_matches, results.active_match_ordinal);
+}
+
+}  // namespace content