Remove dead browsing_data code and add HistoryCounter test

Remove dead code from components/browsing_data.
Add test for output of HistoryCounter.

Bug: 843995
Change-Id: I44361907de445728f4783c2206857684414f8954
Reviewed-on: https://chromium-review.googlesource.com/1064117Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561010}

components/browsing_data/content/conditional_cache_counting_helper.cc

@@ -66,10 +66,6 @@ ConditionalCacheCountingHelper::CountAndDestroySelfWhenFinished(
   return weak_ptr_factory_.GetWeakPtr();
 }
 
-bool ConditionalCacheCountingHelper::IsFinished() {
-  return is_finished_;
-}
-
 void ConditionalCacheCountingHelper::Finished() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!is_finished_);

components/browsing_data/content/conditional_cache_counting_helper.h

@@ -42,8 +42,6 @@ class ConditionalCacheCountingHelper {
   base::WeakPtr<ConditionalCacheCountingHelper> CountAndDestroySelfWhenFinished(
       const CacheCountCallback& result_callback);
 
-  bool IsFinished();
-
  private:
   enum class CacheState {
     NONE,

components/browsing_data/core/browsing_data_utils_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/browsing_data/core/counters/autofill_counter.h"
+#include "components/browsing_data/core/counters/history_counter.h"
 #include "components/browsing_data/core/counters/passwords_counter.h"
 #include "components/browsing_data/core/pref_names.h"
 #include "components/password_manager/core/browser/test_password_store.h"
@@ -19,6 +20,8 @@
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace browsing_data {
+
 namespace {
 
 class FakeWebDataService : public autofill::AutofillWebDataService {
@@ -51,7 +54,7 @@ class BrowsingDataUtilsTest : public testing::Test {
 
 // Tests the complex output of the Autofill counter.
 TEST_F(BrowsingDataUtilsTest, AutofillCounterResult) {
-  browsing_data::AutofillCounter counter(
+  AutofillCounter counter(
       scoped_refptr<FakeWebDataService>(new FakeWebDataService()), nullptr);
 
   // Test all configurations of zero and nonzero partial results for datatypes.
@@ -80,7 +83,7 @@ TEST_F(BrowsingDataUtilsTest, AutofillCounterResult) {
   };
 
   for (const TestCase& test_case : kTestCases) {
-    browsing_data::AutofillCounter::AutofillResult result(
+    AutofillCounter::AutofillResult result(
         &counter, test_case.num_suggestions, test_case.num_credit_cards,
         test_case.num_addresses, test_case.sync_enabled);
 
@@ -99,7 +102,7 @@ TEST_F(BrowsingDataUtilsTest, AutofillCounterResult) {
 TEST_F(BrowsingDataUtilsTest, PasswordsCounterResult) {
   scoped_refptr<password_manager::TestPasswordStore> store(
       new password_manager::TestPasswordStore());
-  browsing_data::PasswordsCounter counter(
+  PasswordsCounter counter(
       scoped_refptr<password_manager::PasswordStore>(store), nullptr);
 
   const struct TestCase {
@@ -113,8 +116,8 @@ TEST_F(BrowsingDataUtilsTest, PasswordsCounterResult) {
   };
 
   for (const TestCase& test_case : kTestCases) {
-    browsing_data::BrowsingDataCounter::SyncResult result(
-        &counter, test_case.num_passwords, test_case.is_synced);
+    BrowsingDataCounter::SyncResult result(&counter, test_case.num_passwords,
+                                           test_case.is_synced);
     SCOPED_TRACE(base::StringPrintf("Test params: %d password(s), %d is_synced",
                                     test_case.num_passwords,
                                     test_case.is_synced));
@@ -123,3 +126,45 @@ TEST_F(BrowsingDataUtilsTest, PasswordsCounterResult) {
   }
   store->ShutdownOnUIThread();
 }
+
+// Tests the output of the History counter.
+TEST_F(BrowsingDataUtilsTest, HistoryCounterResult) {
+  history::HistoryService history_service;
+  HistoryCounter counter(&history_service,
+                         HistoryCounter::GetUpdatedWebHistoryServiceCallback(),
+                         nullptr);
+  counter.Init(prefs(), ClearBrowsingDataTab::ADVANCED, base::DoNothing());
+
+  const struct TestCase {
+    int num_history;
+    int is_sync_enabled;
+    int has_sync_visits;
+    std::string expected_output;
+  } kTestCases[] = {
+      // No sync, no synced visits:
+      {0, false, false, "None"},
+      {1, false, false, "1 item"},
+      {5, false, false, "5 items"},
+      // Sync but not synced visits:
+      {0, true, false, "None"},
+      {1, true, false, "1 item"},
+      {5, true, false, "5 items"},
+      // Sync and synced visits:
+      {0, true, true, "At least 1 item on synced devices"},
+      {1, true, true, "1 item (and more on synced devices)"},
+      {5, true, true, "5 items (and more on synced devices)"},
+  };
+
+  for (const TestCase& test_case : kTestCases) {
+    HistoryCounter::HistoryResult result(&counter, test_case.num_history,
+                                         test_case.is_sync_enabled,
+                                         test_case.has_sync_visits);
+    SCOPED_TRACE(
+        base::StringPrintf("Test params: %d history, %d has_synced_visits",
+                           test_case.num_history, test_case.has_sync_visits));
+    base::string16 output = browsing_data::GetCounterTextFromResult(&result);
+    EXPECT_EQ(output, base::ASCIIToUTF16(test_case.expected_output));
+  }
+}
+
+}  // namespace browsing_data
\ No newline at end of file

components/browsing_data/core/history_notice_utils.cc

@@ -53,12 +53,6 @@ class MergeBooleanCallbacks {
 
 namespace browsing_data {
 
-namespace testing {
-
-bool g_override_other_forms_of_browsing_history_query = false;
-
-}  // namespace testing
-
 void ShouldShowNoticeAboutOtherFormsOfBrowsingHistory(
     const syncer::SyncService* sync_service,
     history::WebHistoryService* history_service,
@@ -106,14 +100,6 @@ void ShouldPopupDialogAboutOtherFormsOfBrowsingHistory(
     history::WebHistoryService* history_service,
     version_info::Channel channel,
     base::Callback<void(bool)> callback) {
-  // If the query for other forms of browsing history is overriden for testing,
-  // the conditions are identical with
-  // ShouldShowNoticeAboutOtherFormsOfBrowsingHistory.
-  if (testing::g_override_other_forms_of_browsing_history_query) {
-    ShouldShowNoticeAboutOtherFormsOfBrowsingHistory(
-        sync_service, history_service, callback);
-    return;
-  }
 
   if (!sync_service ||
       !sync_service->IsSyncActive() ||

components/browsing_data/core/history_notice_utils.h

@@ -23,16 +23,6 @@ enum class Channel;
 
 namespace browsing_data {
 
-namespace testing {
-
-// TODO(crbug.com/595332): A boolean flag indicating that
-// ShouldShowNoticeAboutOtherFormsOfBrowsingHistory() should skip the query
-// for other forms of browsing history and just assume some such forms were
-// found. Used only for testing. The default is false.
-extern bool g_override_other_forms_of_browsing_history_query;
-
-}  // testing
-
 // Whether the Clear Browsing Data UI should show a notice about the existence
 // of other forms of browsing history stored in user's account. The response
 // is returned in a |callback|.

components/browsing_data/core/history_notice_utils_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/callback.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
 #include "components/history/core/test/fake_web_history_service.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/driver/fake_sync_service.h"
@@ -26,8 +27,6 @@ class TestSyncService : public syncer::FakeSyncService {
   // Getters (FakeSyncService implementation). ---------------------------------
   bool IsSyncActive() const override { return sync_active_; }
 
-  bool IsLocalSyncEnabled() const override { return false; }
-
   syncer::ModelTypeSet GetActiveDataTypes() const override {
     return active_data_types_;
   }
@@ -79,45 +78,28 @@ class HistoryNoticeUtilsTest : public ::testing::Test {
 
   void ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(
       bool expected_test_case_result) {
-    bool got_result = false;
-
+    bool result;
+    base::RunLoop run_loop;
     ShouldPopupDialogAboutOtherFormsOfBrowsingHistory(
-        sync_service_.get(),
-        history_service_.get(),
-        version_info::Channel::STABLE,
-        base::Bind(
-            &HistoryNoticeUtilsTest::Callback,
-            base::Unretained(this),
-            base::Unretained(&got_result)));
-
-    if (!got_result) {
-      run_loop_.reset(new base::RunLoop());
-      run_loop_->Run();
-    }
+        sync_service_.get(), history_service_.get(),
+        version_info::Channel::STABLE, base::BindLambdaForTesting([&](bool r) {
+          result = r;
+          run_loop.Quit();
+        }));
+    run_loop.Run();
 
     // Process the DeleteSoon() called on MergeBooleanCallbacks, otherwise
     // this it will be considered to be leaked.
     base::RunLoop().RunUntilIdle();
 
-    EXPECT_EQ(expected_test_case_result, result_);
+    EXPECT_EQ(expected_test_case_result, result);
   }
 
  private:
-  void Callback(bool* got_result, bool result) {
-    *got_result = true;
-    result_ = result;
-
-    if (run_loop_)
-      run_loop_->Quit();
-  }
-
   scoped_refptr<net::URLRequestContextGetter> url_request_context_;
   std::unique_ptr<TestSyncService> sync_service_;
   std::unique_ptr<history::FakeWebHistoryService> history_service_;
 
-  std::unique_ptr<base::RunLoop> run_loop_;
-  bool result_;
-
   base::MessageLoop message_loop_;
 };