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

chrome/browser/browsing_data/cache_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.
 //
 // Note that this file only tests the basic behavior of the cache counter, as in
 // when it counts and when not, when result is nonzero and when not. It does not
 // test whether the result of the counting is correct. This is the
 // responsibility of a lower layer, and is tested in
 // DiskCacheBackendTest.CalculateSizeOfAllEntries in net_unittests.
 
@@ skipping 111 matching lines @@
   }
 
   // General completion callback.
   void Callback() {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     if (run_loop_)
       run_loop_->Quit();
   }
 
   // Callback from the counter.
-  void CountingCallback(bool finished, BrowsingDataCounter::ResultInt count) {
+  void CountingCallback(scoped_ptr<BrowsingDataCounter::Result> result) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
-    finished_ = finished;
-    result_ = count;
-    if (run_loop_ && finished)
+
+    finished_ = result->Finished();
+    result_ = result->Value();

[Bernhard Bauer, 2015/11/03 11:37:02]

...aand this would crash now, no?

[msramek, 2015/11/03 11:57:06]

Hm :/ If I need to guard access to Value() with Finished(), it's not that much
different than as if I had to also cast to FinishedResult.

My preference would be to keep it simple and and just remove NOTREACHED, since
unfinished computation implies that the result value cannot be relied on.
Currently, it's up to the UI layer to understand and interpret that.

The other option is to actually move Value() to FinishedResult. It will be
cleaner, but more code on the callsites.

[Bernhard Bauer, 2015/11/03 12:07:08]

Or store the result instead of the two values.

[msramek, 2015/11/03 13:15:39]

If I store it as a Result, I'll still have to do the conversion in GetResult()
and DCHECKs. So it should be converted here when we get the callback anyway. So
I did that, and I left the members stored separately.

In clear_browser_data_handler.cc, I just inlined the two casts, as I find that
more readable than nesting the whole "if (Finished())" branch one level deeper.

+
+    if (run_loop_ && finished_)
       run_loop_->Quit();
   }
 
   BrowsingDataCounter::ResultInt GetResult() {
     DCHECK(finished_);
     return result_;
   }
 
  private:
   enum CacheEntryCreationStep {
@@ skipping 115 matching lines @@
   SetDeletionPeriodPref(BrowsingDataRemover::FOUR_WEEKS);
   WaitForIOThread();
   EXPECT_EQ(result, GetResult());
 
   SetDeletionPeriodPref(BrowsingDataRemover::EVERYTHING);
   WaitForIOThread();
   EXPECT_EQ(result, GetResult());
 }
 
 }  // namespace