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

chrome/browser/browsing_data/browsing_data_counter.h

 // 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.
 
 #ifndef CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_COUNTER_H_
 #define CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_COUNTER_H_
 
 #include <stdint.h>
 #include <string>
 
 #include "base/callback.h"
 #include "base/prefs/pref_member.h"
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
 #include "chrome/browser/profiles/profile.h"
 
 class BrowsingDataCounter {
  public:
   typedef uint64_t ResultInt;

[Bernhard Bauer, 2015/11/03 10:11:19]

Hm... I guess you need 64 bits? But unless you actually need the value to be
unsigned, prefer a signed type (see
https://google.github.io/styleguide/cppguide.html#Integer_Types).

[msramek, 2015/11/03 10:55:46]

I definitely need more than 32 bits, as I'm summing the sizes of two caches,
which are both 32-bits numbers. And I also heard that we might get rid of this
limit for caches (and max size will be 64 bit).

But I guess that nothing I'm counting will actually reach the size where I would
be worried about that last bit, so I guess signed is ok.

-  typedef base::Callback<void(bool, ResultInt)> Callback;
+
+  class Result {

[Bernhard Bauer, 2015/11/03 10:11:20]

Add a couple of comments what these classes are for? In particular, that Result
can be used for a pending result.

[msramek, 2015/11/03 10:55:45]

Done.

+   public:
+    explicit Result(const BrowsingDataCounter* source);
+    virtual ~Result();
+
+    const BrowsingDataCounter* source() const { return source_; }
+    virtual bool finished() const;

[Bernhard Bauer, 2015/11/03 10:11:20]

Use CamelCase for non-trivial methods (which includes anything virtual).

[msramek, 2015/11/03 10:55:45]

Done.

+    virtual const ResultInt value() const;

[Bernhard Bauer, 2015/11/03 10:11:20]

The const isn't really necessary if you return a straight value.

[msramek, 2015/11/03 10:55:45]

Done. It was originally a reference, I had to remove it otherwise I wouldn't be
able to "return 0" in Result (without defining additional member). But it's just
an int, so that's not a problem.

...also it's apparently a compilation error on Mac.

+
+   private:
+    const BrowsingDataCounter* source_;

[Bernhard Bauer, 2015/11/03 10:11:19]

DISALLOW_COPY_AND_ASSIGN (you don't want this assignable, right?)

[msramek, 2015/11/03 10:55:45]

Done. I need to get used to this.

+  };
+
+  class FinishedResult : public Result {
+   public:
+    FinishedResult(const BrowsingDataCounter* source, ResultInt value);
+    ~FinishedResult() override;
+
+    bool finished() const override;
+    const ResultInt value() const override;
+
+   private:
+    ResultInt value_;
+  };
+
+  typedef base::Callback<void(scoped_ptr<Result>)> Callback;
 
   BrowsingDataCounter();
   virtual ~BrowsingDataCounter();
 
   // Should be called once to initialize this class.
   void Init(Profile* profile,
             const Callback& callback);
 
   // Name of the preference associated with this counter.
   virtual const std::string& GetPrefName() const = 0;
 
   // The profile associated with this counter.
   Profile* GetProfile() const;
 
   // Restarts the counter. Will be called automatically if the counting needs
   // to be restarted, e.g. when the deletion preference changes state or when
   // we are notified of data changes.
   void Restart();
 
  protected:
   // Should be called from |Count| by any overriding class to indicate that
-  // counting is finished and report the |result|.
-  void ReportResult(ResultInt result);
+  // counting is finished and report |value| as the result.
+  void ReportResult(ResultInt value);
+
+  // A convenience overload of the previous method that allows subclasses to
+  // provide a custom |result|.
+  void ReportResult(scoped_ptr<Result> result);
 
   // Calculates the beginning of the counting period as |period_| before now.
   base::Time GetPeriodStart();
 
  private:
   // Called after the class is initialized by calling |Init|.
   virtual void OnInitialized();
 
   // Count the data.
   virtual void Count() = 0;
@@ skipping 11 matching lines @@
 
   // The integer preference describing the time period for which this data type
   // is to be deleted.
   IntegerPrefMember period_;
 
   // Whether this class was properly initialized by calling |Init|.
   bool initialized_ = false;
 };
 
 #endif  // CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_COUNTER_H_