Create a new Data Saver settings page that adds the site breakdown

chrome/browser/net/spdyproxy/data_reduction_proxy_settings_android.cc

 // Copyright 2013 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/net/spdyproxy/data_reduction_proxy_settings_android.h"
 
 #include <stdint.h>
 
+#include <algorithm>

[sclittle, 2017/04/07 20:44:16]

nit: remove unnecessary include

[megjablon, 2017/04/08 01:52:57]

Done.

+#include <string>
+#include <unordered_map>

[sclittle, 2017/04/07 20:44:16]

nit: remove unnecessary include

[megjablon, 2017/04/08 01:52:57]

Done.

+#include <vector>

[sclittle, 2017/04/07 20:44:16]

nit: Move this include to the .h file

[megjablon, 2017/04/08 01:52:57]

Done.

+
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
+#include "base/time/time.h"

[sclittle, 2017/04/07 20:44:16]

nit: Is this include necessary?

[megjablon, 2017/04/08 01:52:57]

Done.

 #include "base/values.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/pref_names.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_compression_stats.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h"
+#include "components/data_reduction_proxy/core/browser/data_usage_store.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h"
 #include "jni/DataReductionProxySettings_jni.h"
 #include "net/proxy/proxy_server.h"
 #include "url/gurl.h"
 
 
 using base::android::ConvertUTF8ToJavaString;
 using base::android::JavaParamRef;
 using base::android::ScopedJavaLocalRef;
 using data_reduction_proxy::DataReductionProxySettings;
 
-DataReductionProxySettingsAndroid::DataReductionProxySettingsAndroid() {
+namespace {
+
+constexpr int kBucketsPerDay =

[sclittle, 2017/04/07 20:44:16]

minor style nit: I'd recommend explicitly making this a size_t, since that's how
it's used below, but this isn't a big deal.

[megjablon, 2017/04/08 01:52:57]

Done.

+    24 * 60 / data_reduction_proxy::kDataUsageBucketLengthInMinutes;
+
+struct DataUsageForHost {
+  DataUsageForHost() : data_used(0), original_size(0) {}
+
+  int64_t data_used;
+  int64_t original_size;
+};
+
+}  // namespace
+
+DataReductionProxySettingsAndroid::DataReductionProxySettingsAndroid()
+    : weak_factory_(this) {}
+
+DataReductionProxySettingsAndroid::DataReductionProxySettingsAndroid(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& obj)
+    : weak_factory_(this) {
+  j_settings_obj_.Reset(env, obj);
 }
 
 DataReductionProxySettingsAndroid::~DataReductionProxySettingsAndroid() {
 }
 
 jboolean DataReductionProxySettingsAndroid::IsDataReductionProxyPromoAllowed(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
   return Settings()->PromoAllowed();
 }
@@ skipping 16 matching lines @@
     jboolean enabled) {
   Settings()->SetDataReductionProxyEnabled(enabled);
 }
 
 jlong DataReductionProxySettingsAndroid::GetDataReductionLastUpdateTime(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
   return Settings()->GetDataReductionLastUpdateTime();
 }
 
+void DataReductionProxySettingsAndroid::ClearDataSavingStatistics(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& obj) {
+  Settings()->ClearDataSavingStatistics();
+}
+
 base::android::ScopedJavaLocalRef<jobject>
 DataReductionProxySettingsAndroid::GetContentLengths(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
   int64_t original_content_length;
   int64_t received_content_length;
   int64_t last_update_internal;
   Settings()->GetContentLengths(
       data_reduction_proxy::kNumDaysInHistorySummary,
       &original_content_length,
@@ skipping 88 matching lines @@
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
   data_reduction_proxy::DataReductionProxyEventStore* event_store =
       Settings()->GetEventStore();
   if (!event_store)
     return ConvertUTF8ToJavaString(env, std::string());
 
   return ConvertUTF8ToJavaString(env, event_store->SanitizedLastBypassEvent());
 }
 
+void DataReductionProxySettingsAndroid::QueryDataUsage(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj,
+    const JavaParamRef<jobject>& j_result_obj,
+    jint num_days) {
+  DCHECK(num_days <= data_reduction_proxy::kDataUsageHistoryNumDays);
+  j_query_result_obj_.Reset(env, j_result_obj);
+  num_day_for_query_ = num_days;
+  Settings()
+      ->data_reduction_proxy_service()
+      ->compression_stats()
+      ->GetHistoricalDataUsage(base::Bind(
+          &DataReductionProxySettingsAndroid::OnQueryDataUsageComplete,
+          weak_factory_.GetWeakPtr()));
+}
+
+void DataReductionProxySettingsAndroid::OnQueryDataUsageComplete(
+    std::unique_ptr<std::vector<data_reduction_proxy::DataUsageBucket>>
+        data_usage) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+
+  std::map<base::StringPiece, DataUsageForHost> per_site_usage_map;

[sclittle, 2017/04/07 20:44:16]

nit: Could you add includes for <map> and base/strings/string_piece.h?

[megjablon, 2017/04/08 01:52:57]

I've gotten too use to Eclipse taking care of this in Java :P

+
+  // Data usage is sorted chronologically with the last entry corresponding to
+  // |base::Time::Now()|.
+  const size_t num_buckets_to_display = num_day_for_query_ * kBucketsPerDay;
+  for (auto data_usage_it = data_usage->size() > num_buckets_to_display
+                                ? data_usage->end() - num_buckets_to_display
+                                : data_usage->begin();
+       data_usage_it != data_usage->end(); ++data_usage_it) {
+    for (const auto& connection_usage : (*data_usage_it).connection_usage()) {

[sclittle, 2017/04/07 20:44:16]

micro style nit: you could use data_usage_it->connection_usage() instead of
(*data_usage_it).connection_usage(), since they're equivalent.

[megjablon, 2017/04/08 01:52:57]

Done.

+      for (const auto& site_usage : connection_usage.site_usage()) {
+        DataUsageForHost& usage = per_site_usage_map[site_usage.hostname()];
+        usage.data_used += site_usage.data_used();
+        usage.original_size += site_usage.original_size();
+      }
+    }
+  }
+
+  for (const auto& site_bucket : per_site_usage_map) {
+    Java_DataReductionProxySettings_createDataUseItemAndAddToList(

[sclittle, 2017/04/07 20:44:16]

In order to send all the counts at once instead of calling into Java for each
item, could you change the Java method signature to take in 3 arrays, one for
each of hostnames, data_used counts, and original_size counts?

You could use stuff like base::android::ToJavaLongArray(...) to construct those
arrays. WDYT?

[megjablon, 2017/04/08 01:52:57]

I'm not a huge fan of this option. It doesn't seem clean to send over three
different arrays where each index is expected to match up. Also we then have to
construct the objects in Java or keep passing around all three of the arrays
there.

[sclittle, 2017/04/11 00:36:19]

Acknowledged.

+        env, j_query_result_obj_.obj(),
+        ConvertUTF8ToJavaString(env, site_bucket.first),
+        (long)site_bucket.second.data_used,

[sclittle, 2017/04/07 20:44:16]

Do you need to cast these to long here? |long| in C++ is often just 32-bit, but
I think |long| is 64-bit in Java.

What happens if the number of bytes is > max 32-bit in (i.e. >2GB)? Even if this
cast to long here is removed, does it get truncated to 32-bits at some other
point in the process of calling into the Java method?

[megjablon, 2017/04/08 01:52:57]

Removed cast. Tried it out and it works as expected with longs.

+        (long)site_bucket.second.original_size);
+  }
+

[gone, 2017/04/07 20:21:58]

Should you check if either of the java object pointers are null at this point? 
Will this break if you call QueryDataUsage twice in a row quickly, like if you
enter the prefs page, leave it, then re-enter it?

[megjablon, 2017/04/08 01:52:57]

The Settings object is a singleton. It has the callback to the site breakdown
and checks that that's not null before giving it the list.

+  Java_DataReductionProxySettings_onQueryDataUsageComplete(
+      env, j_settings_obj_.obj(), j_query_result_obj_.obj());
+
+  j_query_result_obj_.Release();
+}
+
 // Used by generated jni code.
 static jlong Init(JNIEnv* env, const JavaParamRef<jobject>& obj) {
-  return reinterpret_cast<intptr_t>(new DataReductionProxySettingsAndroid());
+  return reinterpret_cast<intptr_t>(
+      new DataReductionProxySettingsAndroid(env, obj));
 }