Refactor BrowsingHistoryHandler, create BrowsingHistoryService

chrome/browser/ui/webui/browsing_history_handler_unittest.cc

Index: chrome/browser/ui/webui/browsing_history_handler_unittest.cc
diff --git a/chrome/browser/ui/webui/browsing_history_handler_unittest.cc b/chrome/browser/ui/webui/browsing_history_handler_unittest.cc
index 97b9bfcbd424413d4a1f4aa666d6e0020de1a35f..19de320ec4d8a57e273d82c8a8f34f4e076d0ec7 100644
--- a/chrome/browser/ui/webui/browsing_history_handler_unittest.cc
+++ b/chrome/browser/ui/webui/browsing_history_handler_unittest.cc
@@ -7,6 +7,7 @@
 #include <stdint.h>
 #include <memory>
 #include <set>
+#include <utility>

[tsergeant, 2016/11/03 05:45:44]

I might be wrong, but I don't see anything which needs this include?

 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ -19,6 +20,7 @@
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/profile_sync_test_util.h"
+#include "chrome/browser/ui/history_ui_service.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/browser_sync/test_profile_sync_service.h"
 #include "components/history/core/test/fake_web_history_service.h"
@@ -39,42 +41,6 @@
 
 namespace {
 
-struct TestResult {
-  std::string url;
-  int64_t hour_offset;  // Visit time in hours past the baseline time.
-};
-
-// Duplicates on the same day in the local timezone are removed, so set a
-// baseline time in local time.
-const base::Time baseline_time = base::Time::UnixEpoch().LocalMidnight();
-
-// For each item in |results|, create a new Value representing the visit, and
-// insert it into |list_value|.
-void AddQueryResults(
-    TestResult* test_results,
-    int test_results_size,
-    std::vector<BrowsingHistoryHandler::HistoryEntry>* results) {
-  for (int i = 0; i < test_results_size; ++i) {
-    BrowsingHistoryHandler::HistoryEntry entry;
-    entry.time = baseline_time +
-                 base::TimeDelta::FromHours(test_results[i].hour_offset);
-    entry.url = GURL(test_results[i].url);
-    entry.all_timestamps.insert(entry.time.ToInternalValue());
-    results->push_back(entry);
-  }
-}
-
-// Returns true if |result| matches the test data given by |correct_result|,
-// otherwise returns false.
-bool ResultEquals(
-    const BrowsingHistoryHandler::HistoryEntry& result,
-    const TestResult& correct_result) {
-  base::Time correct_time =
-      baseline_time + base::TimeDelta::FromHours(correct_result.hour_offset);
-
-  return result.time == correct_time && result.url == GURL(correct_result.url);
-}
-
 void IgnoreBoolAndDoNothing(bool ignored_argument) {}
 
 class TestSyncService : public browser_sync::TestProfileSyncService {
@@ -105,7 +71,6 @@ class BrowsingHistoryHandlerWithWebUIForTesting
   explicit BrowsingHistoryHandlerWithWebUIForTesting(content::WebUI* web_ui) {
     set_web_ui(web_ui);
   }
-  using BrowsingHistoryHandler::QueryComplete;
 };
 
 }  // namespace
@@ -177,99 +142,13 @@ class BrowsingHistoryHandlerTest : public ::testing::Test {
   std::unique_ptr<content::WebContents> web_contents_;
 };
 
-// Tests that the MergeDuplicateResults method correctly removes duplicate
-// visits to the same URL on the same day.
-// Fails on Android.  http://crbug.com/2345
-#if defined(OS_ANDROID)
-#define MAYBE_MergeDuplicateResults DISABLED_MergeDuplicateResults
-#else
-#define MAYBE_MergeDuplicateResults MergeDuplicateResults
-#endif
-TEST_F(BrowsingHistoryHandlerTest, MAYBE_MergeDuplicateResults) {
-  {
-    // Basic test that duplicates on the same day are removed.
-    TestResult test_data[] = {
-      { "http://google.com", 0 },
-      { "http://google.de", 1 },
-      { "http://google.com", 2 },
-      { "http://google.com", 3 }  // Most recent.
-    };
-    std::vector<BrowsingHistoryHandler::HistoryEntry> results;
-    AddQueryResults(test_data, arraysize(test_data), &results);
-    BrowsingHistoryHandler::MergeDuplicateResults(&results);
-
-    ASSERT_EQ(2U, results.size());
-    EXPECT_TRUE(ResultEquals(results[0], test_data[3]));
-    EXPECT_TRUE(ResultEquals(results[1], test_data[1]));
-  }
-
-  {
-    // Test that a duplicate URL on the next day is not removed.
-    TestResult test_data[] = {
-      { "http://google.com", 0 },
-      { "http://google.com", 23 },
-      { "http://google.com", 24 },  // Most recent.
-    };
-    std::vector<BrowsingHistoryHandler::HistoryEntry> results;
-    AddQueryResults(test_data, arraysize(test_data), &results);
-    BrowsingHistoryHandler::MergeDuplicateResults(&results);
-
-    ASSERT_EQ(2U, results.size());
-    EXPECT_TRUE(ResultEquals(results[0], test_data[2]));
-    EXPECT_TRUE(ResultEquals(results[1], test_data[1]));
-  }
-
-  {
-    // Test multiple duplicates across multiple days.
-    TestResult test_data[] = {
-      // First day.
-      { "http://google.de", 0 },
-      { "http://google.com", 1 },
-      { "http://google.de", 2 },
-      { "http://google.com", 3 },
-
-      // Second day.
-      { "http://google.de", 24 },
-      { "http://google.com", 25 },
-      { "http://google.de", 26 },
-      { "http://google.com", 27 },  // Most recent.
-    };
-    std::vector<BrowsingHistoryHandler::HistoryEntry> results;
-    AddQueryResults(test_data, arraysize(test_data), &results);
-    BrowsingHistoryHandler::MergeDuplicateResults(&results);
-
-    ASSERT_EQ(4U, results.size());
-    EXPECT_TRUE(ResultEquals(results[0], test_data[7]));
-    EXPECT_TRUE(ResultEquals(results[1], test_data[6]));
-    EXPECT_TRUE(ResultEquals(results[2], test_data[3]));
-    EXPECT_TRUE(ResultEquals(results[3], test_data[2]));
-  }
-
-  {
-    // Test that timestamps for duplicates are properly saved.
-    TestResult test_data[] = {
-      { "http://google.com", 0 },
-      { "http://google.de", 1 },
-      { "http://google.com", 2 },
-      { "http://google.com", 3 }  // Most recent.
-    };
-    std::vector<BrowsingHistoryHandler::HistoryEntry> results;
-    AddQueryResults(test_data, arraysize(test_data), &results);
-    BrowsingHistoryHandler::MergeDuplicateResults(&results);
-
-    ASSERT_EQ(2U, results.size());
-    EXPECT_TRUE(ResultEquals(results[0], test_data[3]));
-    EXPECT_TRUE(ResultEquals(results[1], test_data[1]));
-    EXPECT_EQ(3u, results[0].all_timestamps.size());
-    EXPECT_EQ(1u, results[1].all_timestamps.size());
-  }
-}
-
-// Tests that BrowsingHistoryHandler observes WebHistoryService deletions.
+// Tests that BrowsingHistoryHandler is informed about WebHistoryService
+// deletions.
 TEST_F(BrowsingHistoryHandlerTest, ObservingWebHistoryDeletions) {
   base::Callback<void(bool)> callback = base::Bind(&IgnoreBoolAndDoNothing);
 
-  // BrowsingHistoryHandler listens to WebHistoryService history deletions.
+  // BrowsingHistoryHandler is informed about WebHistoryService history
+  // deletions.
   {
     sync_service()->SetSyncActive(true);
     BrowsingHistoryHandlerWithWebUIForTesting handler(web_ui());
@@ -282,8 +161,8 @@ TEST_F(BrowsingHistoryHandlerTest, ObservingWebHistoryDeletions) {
     EXPECT_EQ("historyDeleted", web_ui()->call_data().back()->function_name());
   }
 
-  // BrowsingHistoryHandler will listen to WebHistoryService deletions even if
-  // history sync is activated later.
+  // BrowsingHistoryHandler will be informed about WebHistoryService deletions
+  // even if history sync is activated later.
   {
     sync_service()->SetSyncActive(false);
     BrowsingHistoryHandlerWithWebUIForTesting handler(web_ui());
@@ -331,9 +210,11 @@ TEST_F(BrowsingHistoryHandlerTest, MdTruncatesTitles) {
   results.AppendURLBySwapping(&long_result);
 
   BrowsingHistoryHandlerWithWebUIForTesting handler(web_ui());
+  handler.RegisterMessages();  // Needed to create HistoryUiService.
   web_ui()->ClearTrackedCalls();
 
-  handler.QueryComplete(base::string16(), history::QueryOptions(), &results);
+  handler.history_ui_service_->QueryComplete(
+      base::string16(), history::QueryOptions(), &results);
   ASSERT_FALSE(web_ui()->call_data().empty());
 
   const base::ListValue* arg2;