Change the function name to match downstream

chrome/browser/android/data_usage/external_data_use_observer.h

 // Copyright 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_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_
 #define CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_
 
 #include <jni.h>
 #include <stdint.h>
 
@@ skipping 52 matching lines @@
 
   // Called by Java when new matching rules have been fetched. This may be
   // called on a different thread.  |app_package_name| is the package name of
   // the app that should be matched. |domain_path_regex| is the regex to be used
   // for matching URLs. |label| is the label that must be applied to data
   // reports corresponding to the matching rule, and must
   // uniquely identify the matching rule. Each element in |label| must have
   // non-zero length. The three vectors should have equal length. The vectors
   // may be empty which implies that no matching rules are active. Must be
   // called on UI thread.
-  void FetchMatchingRulesCallback(
+  void FetchMatchingRulesDone(
       JNIEnv* env,
       jobject obj,
       const base::android::JavaParamRef<jobjectArray>& app_package_name,
       const base::android::JavaParamRef<jobjectArray>& domain_path_regex,
       const base::android::JavaParamRef<jobjectArray>& label);
 
   // Called by Java when the reporting of data usage has finished.  This may be
   // called on a different thread. |success| is true if the request was
   // successfully submitted to the external data use observer by Java. Must be
   // called on UI thread.
@@ skipping 14 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, BufferSize);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
                            PeriodicFetchMatchingRules);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, BufferDataUseReports);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, Variations);
 
   // DataUseReportKey is a unique identifier for a data use report.
   struct DataUseReportKey {
     DataUseReportKey(const std::string& label,
                      net::NetworkChangeNotifier::ConnectionType connection_type,
-                     const std::string& mcc_mnc)
-        : label(label), connection_type(connection_type), mcc_mnc(mcc_mnc) {}
+                     const std::string& mcc_mnc);
+    virtual ~DataUseReportKey();

[sclittle, 2015/11/10 19:21:57]

nit: this doesn't have to be virtual, and the class is simple enough to just
leave out the destructor here if you want.

[tbansal1, 2015/11/11 21:18:38]

Done.

 
-    bool operator==(const DataUseReportKey& other) const {
-      return (label == other.label &&
-              connection_type == other.connection_type &&
-              mcc_mnc == other.mcc_mnc);
-    }
-
-    virtual ~DataUseReportKey() {}
+    bool operator==(const DataUseReportKey& other) const;
 
     // Label provided by the matching rules.
     const std::string label;
 
     // Type of network used by the request.
     const net::NetworkChangeNotifier::ConnectionType connection_type;
 
     // mcc_mnc operator of the provider of the SIM as obtained from
     // TelephonyManager#getNetworkOperator() Java API in Android.
     const std::string mcc_mnc;
   };
 
   // DataUseReport is paired with a  DataUseReportKey object. DataUseReport
   // contains the bytes send/received during a specific interval. Only the bytes
   // from the data use reports that have the |label|, |connection_type|, and
   // |mcc_mnc| specified in the corresponding DataUseReportKey object are
   // counted in the DataUseReport.
   struct DataUseReport {
     // |start_time| and |end_time| are the start and end timestamps (in UTC
     // since the standard Java epoch of 1970-01-01 00:00:00) of the interval
     // that this data report covers. |bytes_downloaded| and |bytes_uploaded| are
     // the total bytes received and send during this interval.
     DataUseReport(const base::Time& start_time,
                   const base::Time& end_time,
                   int64_t bytes_downloaded,
-                  int64_t bytes_uploaded)
-        : start_time(start_time),
-          end_time(end_time),
-          bytes_downloaded(bytes_downloaded),
-          bytes_uploaded(bytes_uploaded) {}
-
-    virtual ~DataUseReport() {}
+                  int64_t bytes_uploaded);
+    virtual ~DataUseReport();

[sclittle, 2015/11/10 19:21:57]

nit: this doesn't have to be virtual either, and could be left out if you want.

[tbansal1, 2015/11/11 21:18:38]

Done.

 
     // Start time of |this| data report (in UTC since the standard Java epoch of
     // 1970-01-01 00:00:00).
     const base::Time start_time;
 
     // End time of |this| data report (in UTC since the standard Java epoch of
     // 1970-01-01 00:00:00)
     const base::Time end_time;
 
     // Number of bytes downloaded and uploaded by Chromium from |start_time| to
     // |end_time|.
     const int64_t bytes_downloaded;
     const int64_t bytes_uploaded;
   };
 
   // Class that implements hash operator on DataUseReportKey.
   class DataUseReportKeyHash {
    public:
     // A simple heuristical hash function that satisifes the property that two
     // equal data structures have the same hash value. The hash is computed by
     // hashing individual variables and combining them using prime numbers.
     // Prime numbers are used for multiplication because the number of buckets

[sclittle, 2015/11/10 19:21:57]

nit: Perhaps the justification for using prime numbers and stuff (i.e. this line
and the lines below) should be moved to the implementation of the method? It
seems a bit more relevant there than here.

[tbansal1, 2015/11/11 21:18:38]

Done.

     // used by map is always an even number. Using a prime number ensures that
     // for two different DataUseReportKey objects (say |j| and |k|), if the
     // hash value of |k.label| is equal to hash value of |j.mcc_mnc|, then |j|
     // and |k| map to different buckets. Large prime numbers are used so that
     // hash value is spread over a larger range.
-    size_t operator()(const DataUseReportKey& k) const {
-      std::hash<std::string> hash_function;
-      size_t hash = 1;
-      hash = hash * 23 + hash_function(k.label);
-      hash = hash * 43 + k.connection_type;
-      hash = hash * 83 + hash_function(k.mcc_mnc);
-      return hash;
-    }
+    size_t operator()(const DataUseReportKey& k) const;
   };
 
   typedef base::hash_map<DataUseReportKey, DataUseReport, DataUseReportKeyHash>
       DataUseReports;
 
   // Stores the matching rules.
   class MatchingRule {
    public:
     MatchingRule(const std::string& app_package_name,
                  scoped_ptr<re2::RE2> pattern,
@@ skipping 21 matching lines @@
   static const size_t kMaxBufferSize = 100;
 
   // Creates Java object. Must be called on the UI thread.
   void CreateJavaObjectOnUIThread();
 
   // data_usage::DataUseAggregator::Observer implementation:
   void OnDataUse(const std::vector<const data_usage::DataUse*>&
                      data_use_sequence) override;
 
   // Fetches matching rules from Java. Must be called on the UI thread. Returns
-  // result asynchronously on UI thread via FetchMatchingRulesCallback.
+  // result asynchronously on UI thread via FetchMatchingRulesDone.
   void FetchMatchingRulesOnUIThread() const;
 
-  // Called by FetchMatchingRulesCallback on IO thread when new matching rules
+  // Called by FetchMatchingRulesDone on IO thread when new matching rules
   // Adds |data_use| to buffered reports. |data_use| is the data use report
   // received from DataUseAggregator. |data_use| should not be null. |label| is
   // a non-empty label that applies to |data_use|. |start_time| and |end_time|
   // are the start, and end times of the interval during which bytes reported in
   // |data_use| went over the network.
   void BufferDataUseReport(const data_usage::DataUse* data_use,
                            const std::string& label,
                            const base::Time& start_time,
                            const base::Time& end_time);
 
   // Submits the first data report among the buffered data reports in
   // |buffered_data_reports_|. Since an unordered_map is used to buffer the
   // reports, the order of reports may change. The reports are buffered in an
   // arbitrary order and there are no guarantees that the next report to be
   // submitted is the oldest one buffered.
   void SubmitBufferedDataUseReport();
 
-  // Called by |FetchMatchingRulesCallback| on IO thread when new matching rules
-  // have been fetched.
-  void FetchMatchingRulesCallbackOnIOThread(
+  // Called by FetchMatchingRulesDone on IO thread when new matching rules have
+  // been fetched.
+  void FetchMatchingRulesDoneOnIOThread(
       const std::vector<std::string>& app_package_name,
       const std::vector<std::string>& domain_path_regex,
       const std::vector<std::string>& label);
 
   // Reports data use to Java. Must be called on the UI thread. Returns
   // result asynchronously on UI thread via OnReportDataUseDone.
   void ReportDataUseOnUIThread(const DataUseReportKey& key,
                                const DataUseReport& report) const;
 
   // Called by OnReportDataUseDone on IO thread when a data use report has
   // been submitted.
   void OnReportDataUseDoneOnIOThread(bool success);
 
-  // Called by FetchMatchingRulesCallbackIO to register multiple
+  // Called by FetchMatchingRulesDoneOnIOThread to register multiple
   // case-insensitive regular expressions. If the url of the data use request
   // matches any of the regular expression, the observation is passed to the
   // Java listener.
   void RegisterURLRegexes(const std::vector<std::string>& app_package_name,
                           const std::vector<std::string>& domain_path_regex,
                           const std::vector<std::string>& label);
 
   // Returns true if the |gurl| matches the registered regular expressions.
   // |label| must not be null. If a match is found, the |label| is set to the
   // matching rule's label.
   bool Matches(const GURL& gurl, std::string* label) const;
 
   // Return the weak pointer to |this| to be used on IO and UI thread,
   // respectively.
   base::WeakPtr<ExternalDataUseObserver> GetIOWeakPtr();
   base::WeakPtr<ExternalDataUseObserver> GetUIWeakPtr();
 
   // Aggregator that sends data use observations to |this|.
   data_usage::DataUseAggregator* data_use_aggregator_;
 
   // Java listener that provides regular expressions to |this|. The regular
   // expressions are applied to the request URLs, and filtered data use is
   // notified to |j_external_data_use_observer_|.
   base::android::ScopedJavaGlobalRef<jobject> j_external_data_use_observer_;
 
-  // True if callback from |FetchMatchingRulesCallback| is currently pending.
+  // True if callback from |FetchMatchingRulesDone| is currently pending.
   bool matching_rules_fetch_pending_;
 
   // True if callback from |SubmitDataUseReportCallback| is currently pending.
   bool submit_data_report_pending_;
 
   // Contains matching rules.
   ScopedVector<MatchingRule> matching_rules_;
 
   // Buffered data reports that need to be submitted to the Java data use
   // observer.
@@ skipping 37 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(ExternalDataUseObserver);
 };
 
 bool RegisterExternalDataUseObserver(JNIEnv* env);
 
 }  // namespace android
 
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_