Polish the result communication and display of the browsing data counters.

chrome/browser/browsing_data/history_counter_browsertest.cc

 // Copyright (c) 2015 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/browsing_data/history_counter.h"
 
 #include "base/prefs/pref_service.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/history/history_service_factory.h"
@@ skipping 44 matching lines @@
   void SetDeletionPeriodPref(BrowsingDataRemover::TimePeriod period) {
     browser()->profile()->GetPrefs()->SetInteger(
         prefs::kDeleteTimePeriod, static_cast<int>(period));
   }
 
   void WaitForCounting() {
     run_loop_.reset(new base::RunLoop());
     run_loop_->Run();
   }
 
-  BrowsingDataCounter::ResultInt GetResult() {
+  BrowsingDataCounter::ResultInt GetLocalResult() {
     DCHECK(finished_);
-    return result_;
+    return local_result_;
   }
 
-  void Callback(bool finished, BrowsingDataCounter::ResultInt count) {
-    finished_ = finished;
-    result_ = count;
-    if (run_loop_ && finished)
+  bool HasSyncedVisits() {
+    DCHECK(finished_);
+    return has_synced_visits_;
+  }
+
+  void Callback(scoped_ptr<BrowsingDataCounter::Result> result) {
+    finished_ = result->Finished();
+    local_result_ = result->Value();
+
+    if (finished_) {
+      HistoryCounter::HistoryResult* history_result =
+          static_cast<HistoryCounter::HistoryResult*>(result.get());
+
+      has_synced_visits_ = history_result->has_synced_visits();
+    }
+
+    if (run_loop_ && finished_)
       run_loop_->Quit();
   }
 
  private:
   scoped_ptr<base::RunLoop> run_loop_;
   history::HistoryService* service_;
   base::Time time_;
 
   bool finished_;
-  BrowsingDataCounter::ResultInt result_;
+  BrowsingDataCounter::ResultInt local_result_;
+  bool has_synced_visits_;
 };
 
 // Tests that the counter considers duplicate visits from the same day
 // to be a single item.
 IN_PROC_BROWSER_TEST_F(HistoryCounterTest, DuplicateVisits) {
   AddVisit("https://www.google.com");   // 1 item
   AddVisit("https://www.google.com");
   AddVisit("https://www.chrome.com");   // 2 items
   AddVisit("https://www.chrome.com");
   AddVisit("https://www.chrome.com");
@@ skipping 13 matching lines @@
   AddVisit("https://www.google.com");
   AddVisit("https://www.chrome.com");
 
   HistoryCounter counter;
   counter.Init(browser()->profile(),
                base::Bind(&HistoryCounterTest::Callback,
                           base::Unretained(this)));
   counter.Restart();
 
   WaitForCounting();
-  EXPECT_EQ(7u, GetResult());
+  EXPECT_EQ(7u, GetLocalResult());
 }
 
 // Tests that the counter starts counting automatically when the deletion
 // pref changes to true.
 IN_PROC_BROWSER_TEST_F(HistoryCounterTest, PrefChanged) {
   SetHistoryDeletionPref(false);
   AddVisit("https://www.google.com");
   AddVisit("https://www.chrome.com");
 
   HistoryCounter counter;
   counter.Init(browser()->profile(),
                base::Bind(&HistoryCounterTest::Callback,
                           base::Unretained(this)));
   SetHistoryDeletionPref(true);
 
   WaitForCounting();
-  EXPECT_EQ(2u, GetResult());
+  EXPECT_EQ(2u, GetLocalResult());
 }
 
 // Tests that the counter does not count history if the deletion
 // preference is false.
 IN_PROC_BROWSER_TEST_F(HistoryCounterTest, PrefIsFalse) {
   SetHistoryDeletionPref(false);
   AddVisit("https://www.google.com");
 
   HistoryCounter counter;
   counter.Init(browser()->profile(),
@@ skipping 28 matching lines @@
   AddVisit("https://www.example.com");
   AddVisit("https://www.example.com");
 
   HistoryCounter counter;
   counter.Init(browser()->profile(),
                base::Bind(&HistoryCounterTest::Callback,
                           base::Unretained(this)));
 
   SetDeletionPeriodPref(BrowsingDataRemover::LAST_HOUR);
   WaitForCounting();
-  EXPECT_EQ(1u, GetResult());
+  EXPECT_EQ(1u, GetLocalResult());
 
   SetDeletionPeriodPref(BrowsingDataRemover::LAST_DAY);
   WaitForCounting();
-  EXPECT_EQ(1u, GetResult());
+  EXPECT_EQ(1u, GetLocalResult());
 
   SetDeletionPeriodPref(BrowsingDataRemover::LAST_WEEK);
   WaitForCounting();
-  EXPECT_EQ(5u, GetResult());
+  EXPECT_EQ(5u, GetLocalResult());
 
   SetDeletionPeriodPref(BrowsingDataRemover::FOUR_WEEKS);
   WaitForCounting();
-  EXPECT_EQ(8u, GetResult());
+  EXPECT_EQ(8u, GetLocalResult());
 
   SetDeletionPeriodPref(BrowsingDataRemover::EVERYTHING);
   WaitForCounting();
-  EXPECT_EQ(9u, GetResult());
+  EXPECT_EQ(9u, GetLocalResult());
 }
 
 class FakeWebHistoryService : public history::WebHistoryService {
  public:
 
   class Request : public history::WebHistoryService::Request {
    public:
     Request(FakeWebHistoryService* service,
             bool emulate_success,
             int emulate_response_code,
@@ skipping 121 matching lines @@
   // for testing.
   scoped_ptr<FakeWebHistoryService> service(
       new FakeWebHistoryService(browser()->profile()));
 
   HistoryCounter counter;
   counter.SetWebHistoryServiceForTesting(service.get());
   counter.Init(browser()->profile(),
                base::Bind(&HistoryCounterTest::Callback,
                           base::Unretained(this)));
 
-  // No entries locally and no entries in Sync - the result should be zero.
+  // No entries locally and no entries in Sync.
   service->SetRequestOptions(true /* success */, net::HTTP_OK);
   counter.Restart();
   WaitForCounting();
-  EXPECT_EQ(0u, GetResult());
+  EXPECT_EQ(0u, GetLocalResult());
+  EXPECT_FALSE(HasSyncedVisits());
 
   // No entries locally. There are some entries in Sync, but they are out of the
-  // time range. The result should still be zero.
+  // time range.
   SetDeletionPeriodPref(BrowsingDataRemover::LAST_HOUR);
   service->AddSyncedVisit(
       "www.google.com", GetCurrentTime() - base::TimeDelta::FromHours(2));
   service->AddSyncedVisit(
       "www.chrome.com", GetCurrentTime() - base::TimeDelta::FromHours(2));
   service->SetRequestOptions(true /* success */, net::HTTP_OK);
   counter.Restart();
   WaitForCounting();
-  EXPECT_EQ(0u, GetResult());
+  EXPECT_EQ(0u, GetLocalResult());
+  EXPECT_FALSE(HasSyncedVisits());
 
-  // No entries locally, but some entries in Sync - the result should be
-  // kOnlySyncedHistory.
+  // No entries locally, but some entries in Sync.
   service->AddSyncedVisit("www.google.com", GetCurrentTime());
   service->SetRequestOptions(true /* success */, net::HTTP_OK);
   counter.Restart();
   WaitForCounting();
-  EXPECT_EQ(HistoryCounter::kOnlySyncedHistory, GetResult());
+  EXPECT_EQ(0u, GetLocalResult());
+  EXPECT_TRUE(HasSyncedVisits());
 
   // To err on the safe side, if the server request fails, we assume that there
   // might be some items on the server.
   service->SetRequestOptions(true /* success */,
                              net::HTTP_INTERNAL_SERVER_ERROR);
   counter.Restart();
   WaitForCounting();
-  EXPECT_EQ(HistoryCounter::kOnlySyncedHistory, GetResult());
+  EXPECT_EQ(0u, GetLocalResult());
+  EXPECT_TRUE(HasSyncedVisits());
 
   // Same when the entire query fails.
   service->SetRequestOptions(false /* success */,
                              net::HTTP_INTERNAL_SERVER_ERROR);
   counter.Restart();
   WaitForCounting();
-  EXPECT_EQ(HistoryCounter::kOnlySyncedHistory, GetResult());
+  EXPECT_EQ(0u, GetLocalResult());
+  EXPECT_TRUE(HasSyncedVisits());
 
-  // If the local count is nonzero, the server result has no influence on the
-  // output, whether its nonempty...
+  // Nonzero local count, nonempty sync.
   AddVisit("https://www.google.com");
   AddVisit("https://www.chrome.com");
   service->SetRequestOptions(true /* success */, net::HTTP_OK);
   counter.Restart();
   WaitForCounting();
-  EXPECT_EQ(2u, GetResult());
+  EXPECT_EQ(2u, GetLocalResult());
+  EXPECT_TRUE(HasSyncedVisits());
 
-  // ...or empty.
+  // Nonzero local count, empty sync.
   service->ClearSyncedVisits();
   service->SetRequestOptions(true /* success */, net::HTTP_OK);
   counter.Restart();
   WaitForCounting();
-  EXPECT_EQ(2u, GetResult());
+  EXPECT_EQ(2u, GetLocalResult());
+  EXPECT_FALSE(HasSyncedVisits());
 }
 
 }  // namespace