ExternalDataUseObserver JNI integration

chrome/browser/android/data_usage/external_data_use_observer.cc

 // 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.
 
 #include "chrome/browser/android/data_usage/external_data_use_observer.h"
 
 #include <utility>
 
 #include "base/android/jni_string.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/field_trial.h"
+#include "base/strings/string_number_conversions.h"
 #include "components/data_usage/core/data_use.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "jni/ExternalDataUseObserver_jni.h"
 #include "third_party/re2/re2/re2.h"
 #include "url/gurl.h"
 
 using base::android::ConvertUTF8ToJavaString;
 using base::android::ToJavaArrayOfStrings;
 
+namespace {
+
+const char kExternalDataUseObserverFieldTrial[] = "ExternalDataUseObserver";
+
+// Default duration after which matching rules are periodically fetched. May be
+// overridden by the field trial.
+const int kDefaultFetchMatchingRulesDurationSeconds = 60 * 15;  // 15 minutes

[sclittle, 2015/11/06 23:42:49]

nit: add period at the end of the comment.

[tbansal1, 2015/11/07 01:32:43]

Done.

+
+// Default value of the minimum number of bytes that should be buffered before
+// a data use report is submitted. May be overridden by the field trial.
+const int64_t kDefaultDataUseReportMinBytes = 100 * 1000;  // 100 KB.

[sclittle, 2015/11/06 23:42:48]

nit: 1 KB = 1024 bytes

[tbansal1, 2015/11/07 01:32:43]

Done.

+
+}  // namespace
+
 namespace chrome {
 
 namespace android {
 
 ExternalDataUseObserver::ExternalDataUseObserver(
     data_usage::DataUseAggregator* data_use_aggregator,
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
     : data_use_aggregator_(data_use_aggregator),
       matching_rules_fetch_pending_(false),
       submit_data_report_pending_(false),
       registered_as_observer_(false),
       io_task_runner_(io_task_runner),
       ui_task_runner_(ui_task_runner),
       previous_report_time_(base::Time::Now()),
+      last_matching_rules_fetch_time_(base::TimeTicks::Now()),
+      total_bytes_buffered_(0),
+      fetch_matching_rules_duration_(base::TimeDelta::FromSeconds(
+          kDefaultFetchMatchingRulesDurationSeconds)),
+      data_use_report_min_bytes_(kDefaultDataUseReportMinBytes),
       io_weak_factory_(this),
       ui_weak_factory_(this) {
   DCHECK(data_use_aggregator_);
   DCHECK(io_task_runner_);
   DCHECK(ui_task_runner_);
+
+  PopulateFieldTrialParams();
+
   ui_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&ExternalDataUseObserver::CreateJavaObjectOnUIThread,
                  GetUIWeakPtr()));
 
   ui_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&ExternalDataUseObserver::FetchMatchingRulesOnUIThread,
                  GetUIWeakPtr()));
 
   matching_rules_fetch_pending_ = true;
   data_use_aggregator_->AddObserver(this);
   registered_as_observer_ = true;
 }
 
+const std::string
+ExternalDataUseObserver::GetExternalDataUseObserverFieldTrialName() const {
+  return kExternalDataUseObserverFieldTrial;
+}
+
+void ExternalDataUseObserver::PopulateFieldTrialParams() {
+  std::string field_trial = GetExternalDataUseObserverFieldTrialName();
+
+  int32_t fetch_matching_rules_duration_seconds = -1;
+  std::string variation_value = variations::GetVariationParamValue(
+      field_trial, "fetch_matching_rules_duration_seconds");
+  if (!variation_value.empty() &&
+      base::StringToInt(variation_value,
+                        &fetch_matching_rules_duration_seconds)) {
+    fetch_matching_rules_duration_ =
+        base::TimeDelta::FromSeconds(fetch_matching_rules_duration_seconds);
+  }
+  DCHECK_GE(fetch_matching_rules_duration_, base::TimeDelta());
+
+  uint64_t data_use_report_min_bytes = 0;

[sclittle, 2015/11/06 23:42:50]

Use an int64_t here instead of a uint64_t.

[tbansal1, 2015/11/07 01:32:43]

Done.

+  variation_value = variations::GetVariationParamValue(
+      field_trial, "data_use_report_min_bytes");
+  if (!variation_value.empty() &&
+      base::StringToUint64(variation_value, &data_use_report_min_bytes)) {
+    data_use_report_min_bytes_ = data_use_report_min_bytes;

[sclittle, 2015/11/06 23:42:50]

Please don't convert a uint64_t to an int64_t without a static cast or
something.

[tbansal1, 2015/11/07 01:32:43]

Done.

+  }
+  DCHECK_GE(data_use_report_min_bytes_, 0);
+}
+
 void ExternalDataUseObserver::CreateJavaObjectOnUIThread() {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
   JNIEnv* env = base::android::AttachCurrentThread();
   j_external_data_use_observer_.Reset(Java_ExternalDataUseObserver_create(
       env, base::android::GetApplicationContext(),
       reinterpret_cast<intptr_t>(this)));
   DCHECK(!j_external_data_use_observer_.is_null());
 }
 
 ExternalDataUseObserver::~ExternalDataUseObserver() {

[sclittle, 2015/11/06 23:42:49]

To help prevent losing any information, could we pass any remaining data use to
the Java observer here in the destructor?

It wouldn't be a guarantee, because another report could be being submitted
right at this time anyways, but it could at least help.

[tbansal1, 2015/11/07 01:32:43]

The data use reports are asynchronously submitted on UI thread by the Java
object. 
The posted function on UI thread may get called after the destructor has
finished. This
makes it impossible to post any reports from the destructor.

   DCHECK(thread_checker_.CalledOnValidThread());
 
   JNIEnv* env = base::android::AttachCurrentThread();
   if (!j_external_data_use_observer_.is_null()) {
     Java_ExternalDataUseObserver_onDestroy(env,
                                            j_external_data_use_observer_.obj());
   }
   if (registered_as_observer_)
     data_use_aggregator_->RemoveObserver(this);
 }
@@ skipping 72 matching lines @@
 
   submit_data_report_pending_ = false;
 
   SubmitBufferedDataUseReport();
 }
 
 void ExternalDataUseObserver::OnDataUse(
     const std::vector<const data_usage::DataUse*>& data_use_sequence) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  if (base::TimeTicks::Now() - last_matching_rules_fetch_time_ >=
+      fetch_matching_rules_duration_) {
+    last_matching_rules_fetch_time_ = base::TimeTicks::Now();

[sclittle, 2015/11/06 23:42:50]

Could you add a quick comment around here describing what this does?

[tbansal1, 2015/11/07 01:32:43]

Done.

+    ui_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&ExternalDataUseObserver::FetchMatchingRulesOnUIThread,
+                   GetUIWeakPtr()));
+  }
+
   if (matching_rules_fetch_pending_) {
     // TODO(tbansal): Buffer reports.
   }
 
   std::string label;
 
   for (const data_usage::DataUse* data_use : data_use_sequence) {
     if (!Matches(data_use->url, &label))
       continue;
 
@@ skipping 17 matching lines @@
   DCHECK_LE(0, data_use->rx_bytes);
   DCHECK_LE(0, data_use->tx_bytes);
   if (data_use->rx_bytes < 0 || data_use->tx_bytes < 0)
     return;
 
   DataUseReportKey data_use_report_key =
       DataUseReportKey(label, data_use->connection_type, data_use->mcc_mnc);
 
   DataUseReport report = DataUseReport(start_time, end_time, data_use->rx_bytes,
                                        data_use->tx_bytes);
+  total_bytes_buffered_ += (data_use->rx_bytes + data_use->tx_bytes);
 
   // Check if the |data_use_report_key| is already in the buffered reports.
   DataUseReports::iterator it =
       buffered_data_reports_.find(data_use_report_key);
   if (it == buffered_data_reports_.end()) {
     // Limit the buffer size.
     if (buffered_data_reports_.size() == kMaxBufferSize) {
       // TODO(tbansal): Add UMA to track impact of lost reports.
       // Remove the first entry.

[sclittle, 2015/11/06 23:42:49]

Why do you remove the first entry? They aren't in any order anyways, since it's
a hash map, so why not just ignore the new entry?

If you really want to remove the oldest entry, then you should use a linked hash
map:
https://code.google.com/p/chromium/codesearch#chromium/src/net/base/linked_ha...

[tbansal1, 2015/11/07 01:32:43]

Done.

+      total_bytes_buffered_ -=
+          (buffered_data_reports_.begin()->second.bytes_downloaded +
+           buffered_data_reports_.begin()->second.bytes_uploaded);
       buffered_data_reports_.erase(buffered_data_reports_.begin());
     }
     buffered_data_reports_.insert(std::make_pair(data_use_report_key, report));
   } else {
     DataUseReport existing_report = DataUseReport(it->second);
     DataUseReport merged_report = DataUseReport(
         std::min(existing_report.start_time, report.start_time),
         std::max(existing_report.end_time, report.end_time),
         existing_report.bytes_downloaded + report.bytes_downloaded,
         existing_report.bytes_uploaded + report.bytes_uploaded);
     buffered_data_reports_.erase(it);
     buffered_data_reports_.insert(
         std::make_pair(data_use_report_key, merged_report));
   }
 
     DCHECK_LE(buffered_data_reports_.size(),

[sclittle, 2015/11/06 23:42:49]

nit: fix indentation

[tbansal1, 2015/11/07 01:32:43]

Done.

               static_cast<size_t>(kMaxBufferSize));
 }
 
 void ExternalDataUseObserver::SubmitBufferedDataUseReport() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (submit_data_report_pending_ || buffered_data_reports_.empty())
     return;
 
+  if (total_bytes_buffered_ < data_use_report_min_bytes_)
+    return;
+
   // TODO(tbansal): Keep buffering until enough data has been received.
 
   // Send one data use report.
   DataUseReports::iterator it = buffered_data_reports_.begin();
   DataUseReportKey key = it->first;
   DataUseReport report = it->second;
 
   // Remove the entry from the map.
   buffered_data_reports_.erase(it);
+  total_bytes_buffered_ -= (report.bytes_downloaded + report.bytes_uploaded);
 
   submit_data_report_pending_ = true;
 
   ui_task_runner_->PostTask(
       FROM_HERE, base::Bind(&ExternalDataUseObserver::ReportDataUseOnUIThread,
                             GetIOWeakPtr(), key, report));
 }
 
 void ExternalDataUseObserver::ReportDataUseOnUIThread(
     const DataUseReportKey& key,
@@ skipping 87 matching lines @@
   return label_;
 }
 
 bool RegisterExternalDataUseObserver(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace android
 
 }  // namespace chrome