Make BrowsingDataHandler observe WebHistoryService deletions

chrome/browser/ui/webui/browsing_history_handler_unittest.cc

 // Copyright (c) 2013 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 "chrome/browser/ui/webui/browsing_history_handler.h"
 
 #include <stdint.h>
+#include <set>
 
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/history/web_history_service_factory.h"
+#include "chrome/browser/signin/fake_profile_oauth2_token_service_builder.h"
+#include "chrome/browser/signin/fake_signin_manager_builder.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
+#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/test/base/testing_profile.h"
+#include "components/history/core/test/fake_web_history_service.h"
+#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
+#include "components/signin/core/browser/fake_signin_manager.h"
+#include "components/signin/core/browser/test_signin_client.h"
+#include "components/sync/base/model_type.h"
+#include "components/sync/driver/fake_sync_service.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_web_ui.h"
+#include "net/http/http_status_code.h"
+#include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 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.
@@ skipping 19 matching lines @@
 // 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 ProfileSyncService {
+ public:
+  explicit TestSyncService(Profile* profile)
+      : ProfileSyncService(CreateProfileSyncServiceParamsForTest(profile)),
+        sync_active_(false) {}
+
+  bool IsSyncActive() const override { return true; }
+
+  syncer::ModelTypeSet GetActiveDataTypes() const override {
+    return syncer::ModelTypeSet::All();
+  }
+
+  void set_sync_active(bool active) { sync_active_ = active; }
+
+ private:
+  bool sync_active_;

[tsergeant, 2016/08/24 08:47:04]

sync_active_ doesn't appear to be actually used anywhere? It seems as though the
second section of the test below passes because WebHistoryService is always
available, and not because of any changes to the Sync Service.

[msramek, 2016/08/24 12:16:07]

Thanks for spotting - I forgot the hardcoded "true" there.

I added |sync_active_| to IsSyncActive(), and also a NotifyObservers() call to
SetSyncActive(). I changed the class to inherit from TestProfileSyncService,
since NotifyObservers() is private in the regular ProfileSyncService.

I also added a third section to the test as a "control group" to prevent this
mistake from happening again.

Finally, I changed |sync_active_| to initialize to true, otherwise the
WebHistoryServiceFactory::GetForProfile() call in the test constructor would
return nullptr. This rigid relationship between ProfileSyncService and
WebHistoryService is actually the reason why we need this complicated test in
the first place.

+};
+
+class BrowsingHistoryHandlerWithWebUIForTesting
+    : public BrowsingHistoryHandler {
+ public:
+  explicit BrowsingHistoryHandlerWithWebUIForTesting(content::WebUI* web_ui) {
+    set_web_ui(web_ui);
+  }
+};
+
 }  // namespace
 
+class BrowsingHistoryHandlerTest : public ::testing::Test {
+ public:
+  void SetUp() override {
+    TestingProfile::Builder builder;
+    builder.AddTestingFactory(ProfileOAuth2TokenServiceFactory::GetInstance(),
+                              &BuildFakeProfileOAuth2TokenService);
+    builder.AddTestingFactory(SigninManagerFactory::GetInstance(),
+                              &BuildFakeSigninManagerBase);
+    builder.AddTestingFactory(ProfileSyncServiceFactory::GetInstance(),
+                              &BuildFakeSyncService);
+    builder.AddTestingFactory(WebHistoryServiceFactory::GetInstance(),
+                              &BuildFakeWebHistoryService);
+    profile_ = builder.Build();
+
+    sync_service_ = static_cast<TestSyncService*>(
+        ProfileSyncServiceFactory::GetForProfile(profile_.get()));
+    web_history_service_ = static_cast<history::FakeWebHistoryService*>(
+        WebHistoryServiceFactory::GetForProfile(profile_.get()));
+    web_history_service_->SetupFakeResponse(true /* success */, net::HTTP_OK);
+
+    web_contents_.reset(content::WebContents::Create(
+        content::WebContents::CreateParams(profile_.get())));
+    web_ui_.reset(new content::TestWebUI);
+    web_ui_->set_web_contents(web_contents_.get());
+  }
+
+  void TearDown() override {
+    web_contents_.reset();
+    web_ui_.reset();
+    profile_.reset();
+  }
+
+  Profile* profile() { return profile_.get(); }
+  TestSyncService* sync_service() { return sync_service_; }
+  history::WebHistoryService* web_history_service() {
+    return web_history_service_;
+  }
+  content::TestWebUI* web_ui() { return web_ui_.get(); }
+
+ private:
+  static std::unique_ptr<KeyedService> BuildFakeSyncService(
+      content::BrowserContext* context) {
+    return base::MakeUnique<TestSyncService>(
+        static_cast<TestingProfile*>(context));
+  }
+
+  static std::unique_ptr<KeyedService> BuildFakeWebHistoryService(
+      content::BrowserContext* context) {
+    Profile* profile = static_cast<TestingProfile*>(context);
+
+    return base::MakeUnique<history::FakeWebHistoryService>(
+        ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
+        SigninManagerFactory::GetForProfile(profile),
+        profile->GetRequestContext());
+  }
+
+  content::TestBrowserThreadBundle thread_bundle_;
+  std::unique_ptr<TestingProfile> profile_;
+  TestSyncService* sync_service_;
+  history::FakeWebHistoryService* web_history_service_;
+  std::unique_ptr<content::TestWebUI> web_ui_;
+  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(BrowsingHistoryHandlerTest, MAYBE_MergeDuplicateResults) {
+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);
@@ skipping 58 matching lines @@
     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.
+TEST_F(BrowsingHistoryHandlerTest, ObservingWebHistoryDeletions) {
+  base::Callback<void(bool)> callback = base::Bind(&IgnoreBoolAndDoNothing);
+
+  // BrowsingHistoryHandler listens to WebHistoryService history deletions.
+  {
+    sync_service()->set_sync_active(true);
+    BrowsingHistoryHandlerWithWebUIForTesting handler(web_ui());
+    handler.RegisterMessages();
+
+    web_history_service()->ExpireHistoryBetween(std::set<GURL>(), base::Time(),
+                                                base::Time::Max(), callback);
+
+    EXPECT_EQ(1U, web_ui()->call_data().size());
+    EXPECT_EQ("historyDeleted", web_ui()->call_data().back()->function_name());
+  }
+
+  // BrowsingHistoryHandler will listen to WebHistoryService deletions even if
+  // history sync is activated later.
+  {
+    sync_service()->set_sync_active(false);
+    BrowsingHistoryHandlerWithWebUIForTesting handler(web_ui());
+    handler.RegisterMessages();
+    sync_service()->set_sync_active(true);
+
+    web_history_service()->ExpireHistoryBetween(std::set<GURL>(), base::Time(),
+                                                base::Time::Max(), callback);
+
+    EXPECT_EQ(2U, web_ui()->call_data().size());
+    EXPECT_EQ("historyDeleted", web_ui()->call_data().back()->function_name());
+  }
+}