Separate data use reporting logic in ExternalDataUseObserver

chrome/browser/android/data_usage/external_data_use_observer.h

Index: chrome/browser/android/data_usage/external_data_use_observer.h
diff --git a/chrome/browser/android/data_usage/external_data_use_observer.h b/chrome/browser/android/data_usage/external_data_use_observer.h
index 671b1d3d25783a92b9efe198b3c83aed14794e59..9037c2fcbc1aad26bccca8fc682e43f48e184dac 100644
--- a/chrome/browser/android/data_usage/external_data_use_observer.h
+++ b/chrome/browser/android/data_usage/external_data_use_observer.h
@@ -5,27 +5,13 @@
 #ifndef CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_
 #define CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_
 
-#include <stddef.h>
-#include <stdint.h>
-
-#include <memory>
-#include <string>
-#include <vector>
-
-#include "base/containers/hash_tables.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
-#include "chrome/browser/android/data_usage/data_use_tab_model.h"
 #include "components/data_usage/core/data_use_aggregator.h"
-#include "net/base/network_change_notifier.h"
-
-#if defined(OS_ANDROID)
-#include "base/android/application_status_listener.h"
-#endif
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -39,31 +25,20 @@ namespace chrome {
 
 namespace android {
 
+class DataUseTabModel;
 class ExternalDataUseObserverBridge;
+class ExternalDataUseReporter;
 
 // This class allows platform APIs that are external to Chromium to observe how
 // much data is used by Chromium on the current Android device. This class
 // registers as a data use observer with DataUseAggregator (as long as there is
-// at least one valid matching rule is present), filters the received
-// observations by applying the regex matching to the URLs of the requests, and
-// notifies the filtered data use to the platform. This class is not thread
-// safe, and must only be accessed on IO thread.
+// at least one valid matching rule is present), collects a batch of data
+// use observations and passes them to ExternalDataUseReporter for labeling the
+// data usage and reporting to the platform. This class also fetches the
+// matching rules from external platform APIs, on demand and periodically. This
+// class is not thread safe, and must only be accessed on IO thread.
 class ExternalDataUseObserver : public data_usage::DataUseAggregator::Observer {
  public:
-  // Result of data usage report submission.  This enum must remain synchronized
-  // with the enum of the same name in metrics/histograms/histograms.xml.
-  enum DataUsageReportSubmissionResult {
-    // Submission of data use report to the external observer was successful.
-    DATAUSAGE_REPORT_SUBMISSION_SUCCESSFUL = 0,
-    // Submission of data use report to the external observer returned error.
-    DATAUSAGE_REPORT_SUBMISSION_FAILED = 1,
-    // Submission of data use report to the external observer timed out.
-    DATAUSAGE_REPORT_SUBMISSION_TIMED_OUT = 2,
-    // Data use report was lost before an attempt was made to submit it.
-    DATAUSAGE_REPORT_SUBMISSION_LOST = 3,
-    DATAUSAGE_REPORT_SUBMISSION_MAX = 4
-  };
-
   // External data use observer field trial name.
   static const char kExternalDataUseObserverFieldTrial[];
 
@@ -77,11 +52,6 @@ class ExternalDataUseObserver : public data_usage::DataUseAggregator::Observer {
   // caller does not owns the returned pointer.
   DataUseTabModel* GetDataUseTabModel() const;
 
-  // Called by ExternalDataUseObserverBridge::OnReportDataUseDone when a data
-  // use report has been submitted. |success| is true if the request was
-  // successfully submitted to the external data use observer by Java.
-  void OnReportDataUseDone(bool success);
-
   // Called by ExternalDataUseObserverBridge. |should_register| is true if
   // |this| should register as a data use observer.
   void ShouldRegisterAsDataUseObserver(bool should_register);
@@ -91,140 +61,33 @@ class ExternalDataUseObserver : public data_usage::DataUseAggregator::Observer {
 
   base::WeakPtr<ExternalDataUseObserver> GetWeakPtr();
 
+  // Called by ExternalDataUseObserverBridge::OnReportDataUseDone when a data
+  // use report has been submitted. |success| is true if the request was
+  // successfully submitted to the external data use observer by Java.
+  // TODO(rajendrant): Move this callback to ExternalDataUseReporter. In order
+  // to move this, ExternalDataUseObserverBridge needs to hold a pointer to
+  // ExternalDataUseReporter. This is currently not doable, since this creates
+  // a circular dependency between ExternalDataUseReporter, DataUseTabModel and
+  // ExternalDataUseObserverBridge, which creates issues during desctruction.
+  void OnReportDataUseDone(bool success);
+
  private:
   friend class DataUseTabModelTest;
   friend class DataUseUITabModelTest;
   friend class ExternalDataUseObserverTest;
-  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, BufferDataUseReports);
-  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, BufferSize);
-  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, DataUseReportTimedOut);
-#if defined(OS_ANDROID)
-  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
-                           DataUseReportingOnApplicationStatusChange);
-#endif
-  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, HashFunction);
+  friend class ExternalDataUseReporterTest;

[tbansal1, 2016/07/15 16:26:30]

reporter tests should not be poking in EDUO.

[Raj, 2016/07/15 18:54:41]

Done. Created GetExternalDataUseReporterForTests() to go around it.

   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
                            MatchingRuleFetchOnControlAppInstall);
-  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, MultipleMatchingRules);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
                            PeriodicFetchMatchingRules);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
                            RegisteredAsDataUseObserver);
-  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, ReportsMergedCorrectly);
-  FRIEND_TEST_ALL_PREFIXES(DataUseUITabModelTest, ReportTabEventsTest);
-  FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest,
-                           TimestampsMergedCorrectly);
   FRIEND_TEST_ALL_PREFIXES(ExternalDataUseObserverTest, Variations);
-
-  // DataUseReportKey is a unique identifier for a data use report.
-  struct DataUseReportKey {
-    DataUseReportKey(const std::string& label,
-                     const std::string& tag,
-                     net::NetworkChangeNotifier::ConnectionType connection_type,
-                     const std::string& mcc_mnc);
-
-    DataUseReportKey(const DataUseReportKey& other);
-
-    bool operator==(const DataUseReportKey& other) const;
-
-    // Label provided by the matching rules.
-    const std::string label;
-
-    // Tag to report for the data usage.
-    const std::string tag;
-
-    // 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 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.
-    size_t operator()(const DataUseReportKey& k) const;
-  };
-
-  typedef base::hash_map<DataUseReportKey, DataUseReport, DataUseReportKeyHash>
-      DataUseReports;
-
-  // Maximum buffer size. If an entry needs to be added to the buffer that has
-  // size |kMaxBufferSize|, then the oldest entry will be removed.
-  static const size_t kMaxBufferSize;
+  FRIEND_TEST_ALL_PREFIXES(DataUseUITabModelTest, ReportTabEventsTest);
 
   // data_usage::DataUseAggregator::Observer implementation:
   void OnDataUse(const data_usage::DataUse& data_use) override;
 
-  // Called by DataUseTabModel when tracking info has been retrieved for the
-  // |data_use| object. |tracking_info_valid| is true if |tracking_info| is
-  // populated that applies to the |data_use| object. |tracking_info| is owned
-  // by the caller.
-  void DataUseTrackingInfoRetrieved(
-      const data_usage::DataUse& data_use,
-      const base::Time& start_time,
-      const base::Time& end_time,
-      const DataUseTabModel::TrackingInfo* tracking_info,
-      bool tracking_info_valid);
-
-  // Adds |data_use| to buffered reports. |data_use| is the data use report
-  // received from DataUseAggregator. |label| is a non-empty label that applies
-  // to |data_use|. |tag| is the tag to be applied for this 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 std::string& tag,
-                           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. |immediate| indicates whether to
-  // submit the report immediately or to wait until |data_use_report_min_bytes_|
-  // unreported bytes are buffered.
-  void SubmitBufferedDataUseReport(bool immediate);
-
-#if defined(OS_ANDROID)
-  // Called whenever the application transitions from foreground to background
-  // or vice versa.
-  void OnApplicationStateChange(base::android::ApplicationState new_state);
-#endif
-
   // Aggregator that sends data use observations to |this|.
   data_usage::DataUseAggregator* data_use_aggregator_;
 
@@ -237,49 +100,19 @@ class ExternalDataUseObserver : public data_usage::DataUseAggregator::Observer {
   // but afterwards, should only be accessed on UI thread.
   DataUseTabModel* data_use_tab_model_;
 
-  // Time when the currently pending data use report was submitted.
-  // |last_data_report_submitted_ticks_| is null if no data use report is
-  // currently pending.
-  base::TimeTicks last_data_report_submitted_ticks_;
-
-  // |pending_report_bytes_| is the total byte count in the data use report that
-  // is currently pending.
-  int64_t pending_report_bytes_;
-
-  // Buffered data reports that need to be submitted to the
-  // |external_data_use_observer_bridge_|.
-  DataUseReports buffered_data_reports_;
+  // Labels, buffers and reports the data usage. It is owned by |this|. It is
+  // created on IO thread but afterwards, should only be accessed on UI thread.
+  ExternalDataUseReporter* external_data_use_reporter_;
 
   // |ui_task_runner_| is used to call methods on UI thread.
   scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
 
-  // Time when the data use reports were last received from DataUseAggregator.
-  base::Time previous_report_time_;
-
   // Time when the matching rules were last fetched.
   base::TimeTicks last_matching_rules_fetch_time_;
 
-  // Total number of bytes transmitted or received across all the buffered
-  // reports.
-  int64_t total_bytes_buffered_;
-
   // Duration after which matching rules are periodically fetched.
   const base::TimeDelta fetch_matching_rules_duration_;
 
-  // Minimum number of bytes that should be buffered before a data use report is
-  // submitted.
-  const int64_t data_use_report_min_bytes_;
-
-  // If a data use report is pending for more than |data_report_submit_timeout_|
-  // duration, it is considered as timed out.
-  const base::TimeDelta data_report_submit_timeout_;
-
-#if defined(OS_ANDROID)
-  // Listens to when Chromium gets backgrounded and submits buffered data use
-  // reports.
-  std::unique_ptr<base::android::ApplicationStatusListener> app_state_listener_;
-#endif
-
   // True if |this| is currently registered as a data use observer.
   bool registered_as_data_use_observer_;