Added probe to determine if data reduction proxy can be used

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 "base/android/build_info.h"
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/base64.h"
+#include "base/command_line.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/histogram.h"
+#include "base/prefs/pref_service.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/prefs/proxy_prefs.h"
+#include "chrome/browser/prefs/scoped_user_pref_update.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
+#include "jni/DataReductionProxySettingsAndroid_jni.h"
+#include "net/base/load_flags.h"
+#include "net/base/net_errors.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request_status.h"
+#include "url/gurl.h"
+
+using base::android::CheckException;
+using base::android::ConvertJavaStringToUTF8;
+using base::android::ConvertUTF8ToJavaString;
+using base::android::ScopedJavaLocalRef;
+using base::StringPrintf;
+
+
+namespace {
+
+// The c++ definition of enum SpdyProxyAuthState defined in

[mmenke, 2013/09/05 21:34:33]

nit:  Capitalize C++?

[bengr, 2013/09/10 00:56:09]

Done.

+// tools/histograms/histograms.xml.

[mmenke, 2013/09/05 21:34:33]

Think it's worth mentioning that new values should be added at the end.

[bengr, 2013/09/10 00:56:09]

Done.

+enum {
+  CHROME_STARTUP,
+  SPDY_PROXY_AUTH_ON_AT_STARTUP,
+  SPDY_PROXY_AUTH_ON_BY_USER,
+  SPDY_PROXY_AUTH_OFF_BY_USER,
+  // Used by UMA histograms and should always be the last value.
+  NUM_SPDY_PROXY_AUTH_STATE
+};
+
+// SpdyProxyValue switch.
+const char kSpdyProxyValueSwitch[] = "spdy-proxy-auth-value";

[mmenke, 2013/09/05 21:34:33]

Switches should be defined in src/chrome/common/chrome_switches.h

[bengr, 2013/09/10 00:56:09]

Done.

+
+const unsigned int kNumDaysInHistory = 60;

[mmenke, 2013/09/09 15:40:05]

I don't think it's a good idea to have the same constant in 3 different files
(here, tests, and the delegate)

[bengr, 2013/09/10 00:56:09]

I fixed for here and the tests. I'll fix for the delegate if needed in a later
CL once I move the data reduction proxy code from there to here.

On 2013/09/09 15:40:05, mmenke wrote:

+const unsigned int kNumDaysInHistorySummary = 30;

[mmenke, 2013/09/09 16:01:07]

Should have a compile assert that this is less than the number of days in the
history.

[bengr, 2013/09/10 00:56:09]

Done.

+
+}  // namespace
+
+DataReductionProxySettingsAndroid::DataReductionProxySettingsAndroid(
+    JNIEnv* env, jobject obj) :

[mmenke, 2013/09/05 21:34:33]

The ":" should go in the next line, and all following lines should be indented
by two more.

[bengr, 2013/09/10 00:56:09]

Done.

+    has_turned_on_(false),
+    has_turned_off_(false),
+    disabled_by_carrier_(false),
+    enabled_by_user_(false),
+    pref_service_(NULL),
+    local_state_(NULL) {
+}
+
+DataReductionProxySettingsAndroid::~DataReductionProxySettingsAndroid() {
+}
+
+jboolean DataReductionProxySettingsAndroid::IsDataReductionProxyAvailable(
+    JNIEnv* env, jobject obj) {
+  return IsDataReductionProxyAvailable();
+}
+
+jboolean DataReductionProxySettingsAndroid::IsDataReductionProxyPromoAvailable(
+    JNIEnv* env, jobject obj) {
+  return IsDataReductionProxyPromoAvailable();
+}
+
+ScopedJavaLocalRef<jstring>
+DataReductionProxySettingsAndroid::GetDataReductionProxyOrigin(

[mmenke, 2013/09/05 21:34:33]

Confused about why this is different from GetDataReductionProxyOriginHostPort. 
Here we check the switch, there we check the pref...Why?

[bengr, 2013/09/10 00:56:09]

It actually doesn't matter because the switch is mapped to the pref. I've
changed GetDataReductionProxyOriginHostPort to refer to the switch for
consistency.

On 2013/09/05 21:34:33, mmenke wrote:

+    JNIEnv* env, jobject obj) {
+  if (!IsDataReductionProxyAvailable())
+    ConvertUTF8ToJavaString(env, std::string());
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(switches::kSpdyProxyAuthOrigin)) {
+    return ConvertUTF8ToJavaString(env, command_line.GetSwitchValueASCII(
+            switches::kSpdyProxyAuthOrigin));

[mmenke, 2013/09/05 21:34:33]

Think this indentation violates the style guide - all arguments should be lined
up ("If the arguments do not all fit on one line, they should be broken up onto
multiple lines, with each subsequent line aligned with the first argument. Do
not add spaces after the open paren or before the close paren:").

[bengr, 2013/09/10 00:56:09]

Done.

+  }
+#if defined(SPDY_PROXY_AUTH_ORIGIN)
+  return ConvertUTF8ToJavaString(env, SPDY_PROXY_AUTH_ORIGIN);
+#endif
+  return ConvertUTF8ToJavaString(env, std::string());

[mmenke, 2013/09/09 15:40:05]

Suggest replacing the #endif with an #else, to emphasize that this isn't run if
SPDY_PROXY_AUTH_ORIGIN is defined (And the same goes for the next line as well)

[bengr, 2013/09/10 00:56:09]

Done.

+}
+
+ScopedJavaLocalRef<jstring>
+DataReductionProxySettingsAndroid::GetDataReductionProxyValue(
+    JNIEnv* env, jobject obj) {
+  if (!IsDataReductionProxyAvailable())
+    return ConvertUTF8ToJavaString(env, std::string());
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(switches::kSpdyProxyAuthOrigin)) {
+    // If an origin is provided via a switch, then only consider the value
+    // that is provided by a switch. Do not use the preprocessor constant.
+    if (command_line.HasSwitch(kSpdyProxyValueSwitch)) {
+      return ConvertUTF8ToJavaString(env,
+          command_line.GetSwitchValueASCII(kSpdyProxyValueSwitch));

[mmenke, 2013/09/05 21:34:33]

Again, this seems to violate the style guide.

[bengr, 2013/09/10 00:56:09]

Done.

+    }
+  }
+#if defined(SPDY_PROXY_AUTH_VALUE)
+  return ConvertUTF8ToJavaString(env, SPDY_PROXY_AUTH_VALUE);
+#endif
+  return ConvertUTF8ToJavaString(env, std::string());
+}
+
+jlong DataReductionProxySettingsAndroid::GetDataReductionLastUpdateTime(
+    JNIEnv* env, jobject obj) {
+  PrefService* local_state = GetLocalState();
+  int64 last_update_internal =
+      local_state->GetInt64(prefs::kDailyHttpContentLengthLastUpdateDate);
+  base::Time last_update = base::Time::FromInternalValue(last_update_internal);
+  return static_cast<int64>(last_update.ToJsTime());
+}
+
+base::android::ScopedJavaLocalRef<jobject>
+DataReductionProxySettingsAndroid::GetContentLengths(JNIEnv* env,
+                                                 jobject obj) {

[mmenke, 2013/09/09 15:40:05]

nit:  Fix indent.

[bengr, 2013/09/10 00:56:09]

Done.

+  int64 original_content_length;
+  int64 received_content_length;
+  int64 last_update_internal;
+  GetContentLengths(kNumDaysInHistorySummary, &original_content_length,

[mmenke, 2013/09/09 15:40:05]

Wonder about this constant being defined in this file.

[bengr, 2013/09/10 00:56:09]

Done.

+                    &received_content_length, &last_update_internal);
+
+  return Java_ContentLengths_create(env,
+                                    original_content_length,
+                                    received_content_length);
+}
+
+void DataReductionProxySettingsAndroid::AddDefaultProxyBypassRules() {
+  // localhost
+  AddHostToBypass("localhost");

[mmenke, 2013/09/05 21:34:33]

This rule seems to be a subset of the next one.

[bengr, 2013/09/10 00:56:09]

The next one uses shExpMatch underneath, which afaik doesn't use standard
regular expressions. I.e., the pattern will match localhost., but not localhost

Please correct me if I'm wrong.

[mmenke, 2013/09/10 16:17:29]

You're right.  The fact that shExpMatch is written in JS, but doesn't use JS
regexps confused me.

+  AddHostPatternToBypass("localhost.*");
+  AddHostToBypass("127.0.0.1");

[mmenke, 2013/09/05 21:34:33]

What about 192.168.*?  10.*? 172.16.0.0 - 172.31.255.255?

[bengr, 2013/09/10 00:56:09]

I think we didn't bypass these because we were worried about the processing time
of adding too many rules. I've added a TODO to revisit.

+
+  // TODO(bengr): See http://crbug.com/169959. For some reason the data
+  // reduction proxy is breaking the omnibox SearchProvider.  Remove this rule
+  // when this is fixed.
+  AddURLSubstringToBypass("http://www.google.com/complete/search", 0, 37);
+
+  // http://freezone.google.com/*
+  // http://www.freezone.google.com/*
+  // http://accounts.freezone.google.com/*
+  // http://mail.freezone.google.com/*
+  // http://plus.freezone.google.com/*
+  // http://search.freezone.google.com/*
+  // TODO(bengr): Consider being more restrictive and match
+  // "freezone.google.com" and "*.freezone.google.com" instead.
+  AddHostPatternToBypass("*freezone.google.com");
+  // http://freezone.googleusercontent.com/*
+  AddHostPatternToBypass("freezone.googleusercontent.com");
+  // http://g.co/gms/*
+  AddURLPatternToBypass("http://g.co/gms/*");
+  // http://g.co/freezone*
+  AddURLPatternToBypass("http://g.co/freezone*");

[mmenke, 2013/09/05 21:34:33]

Why URLPattern here, but URLSubstring above?  Having both methods seems
unnecessary.

[bengr, 2013/09/10 00:56:09]

Done.

+
+  // Check for proxy availability
+  AddURLPatternToBypass(GetProxyCheckURL());
+}
+
+void DataReductionProxySettingsAndroid::AddURLPatternToBypass(
+    const std::string& pattern) {
+  AddPatternToBypass("url", pattern);
+}
+
+void DataReductionProxySettingsAndroid::AddHostPatternToBypass(
+    const std::string& pattern) {
+  AddPatternToBypass("host", pattern);
+}
+
+void DataReductionProxySettingsAndroid::AddPatternToBypass(
+    const std::string& url_or_host,
+    const std::string& pattern) {
+  bypass_rules_.push_back(
+      StringPrintf("shExpMatch(%s, \"%s\")",
+                   url_or_host.c_str(), pattern.c_str()));
+}
+
+void DataReductionProxySettingsAndroid::AddHostToBypass(
+    const std::string& host) {
+  bypass_rules_.push_back(
+      StringPrintf("host == \"%s\"", host.c_str()));
+}
+
+void DataReductionProxySettingsAndroid::AddURLSubstringToBypass(
+    const std::string& url,
+    int from,
+    int to) {
+  bypass_rules_.push_back(
+      StringPrintf("url.substring(%d, %d) == \"%s\"", from, to, url.c_str()));
+}
+
+bool DataReductionProxySettingsAndroid::IsDataReductionProxyAvailable() {
+  return (base::FieldTrialList::FindFullName(
+      "DataCompressionProxyRollout") == "Enabled");

[mmenke, 2013/09/09 15:40:05]

I think this return statement could be formatted to be more readable.  I believe
everything up to and include the "==" could fit on one line.

[bengr, 2013/09/10 00:56:09]

Done.

+}
+
+bool DataReductionProxySettingsAndroid::IsDataReductionProxyPromoAvailable() {
+  return (IsDataReductionProxyAvailable() && base::FieldTrialList::FindFullName(
+      "DataCompressionProxyPromoVisibility") == "Enabled");

[mmenke, 2013/09/09 15:40:05]

Again, think this could be formatted a bit better.

[bengr, 2013/09/10 00:56:09]

Actually, I'm at a loss for how to format this one any better.

+}
+
+std::string DataReductionProxySettingsAndroid::GetProxyPacScript() {
+  std::string bypass_clause = "(" + JoinString(bypass_rules_, ") || (") + ")";
+
+  // We want to fall back to direct loading when the proxy is unavailable and
+  // only process HTTP traffic, so we concoct a PAC configuration
+  // accordingly. (With a statically configured proxy, proxy failures will
+  // simply result in a connection error presented to users.)
+
+  std::string pac = "function FindProxyForURL(url, host) {"

[mmenke, 2013/09/05 21:34:33]

I'm not an expert, but do we really need a PAC?  Seems like we could just make
our own ProxyResolver for the purpose, and bypass Javascript completely.

[bengr, 2013/09/10 00:56:09]

This code is what we use currently in stable and so I'm a little hesitant to
replace it without significant testing. I added a TODO to re-evaluate our
options.

On 2013/09/05 21:34:33, mmenke wrote:

+      "  if (" + bypass_clause + ") {"
+      "    return \"DIRECT\";"
+      "  } "
+      "  if (url.substring(0, 5) == \"http:\") {"
+      "    return \"HTTPS " + GetDataReductionProxyOriginHostPort() +
+      "; DIRECT\";"
+      "  }"
+      "  return \"DIRECT\";"
+      "}";
+  return pac;
+}
+
+PrefService* DataReductionProxySettingsAndroid::GetPrefService() {
+  if (pref_service_)
+    return pref_service_;
+  return g_browser_process->profile_manager()->GetDefaultProfile()->
+      GetOriginalProfile()->GetPrefs();
+}
+
+PrefService* DataReductionProxySettingsAndroid::GetLocalState() {
+  if (local_state_)
+    return local_state_;
+  return g_browser_process->local_state();
+}
+
+void DataReductionProxySettingsAndroid::set_pref_service(
+    PrefService* pref_service) {
+  pref_service_ = pref_service;
+}
+
+void DataReductionProxySettingsAndroid::set_local_state(
+    PrefService* local_state) {
+  local_state_ = local_state;
+}
+
+std::string
+DataReductionProxySettingsAndroid::GetDataReductionProxyOriginHostPort() {
+  std::string spdy_proxy = "";
+
+  PrefService* local_state = GetLocalState();
+  if (local_state->HasPrefPath(prefs::kSpdyProxyAuthOrigin)) {
+    spdy_proxy = local_state->GetString(prefs::kSpdyProxyAuthOrigin);
+  } else {
+#if defined(SPDY_PROXY_AUTH_ORIGIN)
+    spdy_proxy = SPDY_PROXY_AUTH_ORIGIN;
+#endif
+  }
+  if (spdy_proxy.empty()) {
+    DLOG(ERROR) << "A SPDY proxy has not been set.";
+    return spdy_proxy;
+  }
+  RemoveSchemeAndTrailingSlash(&spdy_proxy);
+  return spdy_proxy;
+}
+
+void DataReductionProxySettingsAndroid::RemoveSchemeAndTrailingSlash(
+    std::string* url) {
+  // Remove a trailing slash from the proxy string if one exists as well as
+  // leading HTTPS scheme.
+  DCHECK(url);
+  unsigned len = url->size();
+  if (len > 0 && (*url)[len - 1] == '/') {
+    url->erase(len - 1, 1);
+  }
+  std::string https_scheme("https://");
+  if (len > https_scheme.length() &&
+      url->compare(0, https_scheme.length(), https_scheme) == 0) {
+    url->erase(0, https_scheme.length());
+  }
+}
+
+void DataReductionProxySettingsAndroid::ResetDataReductionStatistics() {
+  PrefService* prefs = GetLocalState();
+  if (!prefs)
+    return;
+  ListPrefUpdate original_update(prefs, prefs::kDailyHttpOriginalContentLength);
+  ListPrefUpdate received_update(prefs, prefs::kDailyHttpReceivedContentLength);
+  original_update->Clear();
+  received_update->Clear();
+  for (size_t i = 0; i < kNumDaysInHistory; ++i) {
+    original_update->AppendString(base::Int64ToString(0));
+    received_update->AppendString(base::Int64ToString(0));
+  }
+}
+
+void DataReductionProxySettingsAndroid::InitDataReductionProxySettings(
+    JNIEnv* env,
+    jobject obj) {
+  // Disable the proxy is it's not meant to be available.
+  if (!IsDataReductionProxyAvailable()) return;
+
+  AddDefaultProxyBypassRules();
+  net::NetworkChangeNotifier::AddIPAddressObserver(this);
+
+  PrefService* prefs = GetPrefService();
+  CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  bool spdy_proxy_enabled = prefs->GetBoolean(prefs::kSpdyProxyAuthEnabled);
+
+  // Setting the kEnableSpdyProxyAuth switch has the same effect as enabling
+  // the feature via settings, in that once set, the preference will be sticky
+  // across instances of Chrome. Disabling the feature can only be done through
+  // the settings menu.
+  UMA_HISTOGRAM_ENUMERATION("SpdyProxyAuth.State", CHROME_STARTUP,
+                            NUM_SPDY_PROXY_AUTH_STATE);
+  if (command_line.HasSwitch(switches::kEnableSpdyProxyAuth) ||
+      spdy_proxy_enabled) {
+    SetDataReductionProxyEnabled(NULL, NULL, true);
+  } else {
+    LOG(WARNING) << "SPDY proxy OFF at startup.";
+  }
+}
+
+jboolean DataReductionProxySettingsAndroid::IsDataReductionProxyEnabled(
+    JNIEnv* env, jobject obj) {
+  return GetPrefService()->GetBoolean(prefs::kSpdyProxyAuthEnabled);
+}
+
+jboolean DataReductionProxySettingsAndroid::IsDataReductionProxyManaged(
+    JNIEnv* env, jobject obj) {
+  return GetPrefService()->IsManagedPreference(prefs::kSpdyProxyAuthEnabled);
+}
+
+void DataReductionProxySettingsAndroid::SetDataReductionProxyEnabled(
+    JNIEnv* env,
+    jobject obj,
+    jboolean enabled) {
+

[mmenke, 2013/09/09 15:40:05]

nit:  Remove blank line.

[bengr, 2013/09/10 00:56:09]

Done.

+  // Disable the proxy is it's not meant to be available.

[mmenke, 2013/09/09 15:40:05]

This comment doesn't seem to be accurate.  We don't do anything if it's "not
meant" to be available (Also think "not meant" is a bit ambiguous)

[bengr, 2013/09/10 00:56:09]

Done.

+  if (!IsDataReductionProxyAvailable()) return;

[mmenke, 2013/09/09 15:40:05]

return should be on its own line.

[bengr, 2013/09/10 00:56:09]

Done.

+
+  // Check if the proxy has been disabled explicitly by the carrier.
+  CheckDataReductionProxyIsAvailable(GetProxyCheckURL());
+
+  PrefService* prefs = GetPrefService();
+
+  prefs->SetBoolean(prefs::kSpdyProxyAuthEnabled, enabled);
+
+  if (enabled && !prefs->GetBoolean(prefs::kSpdyProxyAuthWasEnabledBefore)) {
+    prefs->SetBoolean(prefs::kSpdyProxyAuthWasEnabledBefore, true);
+    ResetDataReductionStatistics();
+  }
+
+  std::string spdy_proxy_origin = GetDataReductionProxyOriginHostPort();
+
+  // Configure use of the data reduction proxy if it is enabled and the proxy
+  // origin is non-empty.
+  enabled_by_user_= enabled && spdy_proxy_origin != "";
+  SetProxyPac(enabled_by_user_, !env);
+
+}
+
+void DataReductionProxySettingsAndroid::SetProxyPac(bool enable_spdy_proxy,
+                                                bool at_startup) {

[mmenke, 2013/09/09 15:40:05]

nit:  Fix indent

[mmenke, 2013/09/09 15:40:05]

nit:  Fix indent.

[bengr, 2013/09/10 00:56:09]

Done.

[bengr, 2013/09/10 00:56:09]

Done.

+  PrefService* prefs = GetPrefService();
+  DCHECK(prefs);
+  // Keys duplicated from proxy_config_dictionary.cc
+  // TODO(bengr): Move these to proxy_config_dictionary.h and reuse them here.
+  const char kProxyMode[] = "mode";
+  const char kProxyPacURL[] = "pac_url";
+  const char kAtStartup[] = "at startup";
+  const char kByUser[] = "by user action";
+
+  DictionaryPrefUpdate update(prefs, prefs::kProxy);
+  DictionaryValue* dict = update.Get();
+  if (enable_spdy_proxy) {
+    LOG(WARNING) << "SPDY proxy ON " << (at_startup ? kAtStartup : kByUser);
+    // Convert to a data URI and update the PAC settings.
+    std::string base64_pac;
+    base::Base64Encode(GetProxyPacScript(), &base64_pac);
+
+    dict->SetString(kProxyPacURL,
+                    "data:application/x-ns-proxy-autoconfig;base64," +
+                    base64_pac);
+    dict->SetString(kProxyMode,
+                    ProxyModeToString(ProxyPrefs::MODE_PAC_SCRIPT));
+
+    if (at_startup) {
+      UMA_HISTOGRAM_ENUMERATION("SpdyProxyAuth.State",
+                                SPDY_PROXY_AUTH_ON_AT_STARTUP,
+                                NUM_SPDY_PROXY_AUTH_STATE);
+    } else if (!has_turned_on_) {
+      // SPDY proxy auth is turned on by user action for the first time in
+      // this session.
+      UMA_HISTOGRAM_ENUMERATION("SpdyProxyAuth.State",
+                                SPDY_PROXY_AUTH_ON_BY_USER,
+                                NUM_SPDY_PROXY_AUTH_STATE);
+      has_turned_on_ = true;
+    }
+  } else {
+    LOG(WARNING) << "SPDY proxy OFF " << (at_startup ? kAtStartup : kByUser);
+    dict->SetString(kProxyMode, ProxyModeToString(ProxyPrefs::MODE_SYSTEM));
+    dict->SetString(kProxyPacURL, "");
+
+    if (!at_startup && !has_turned_off_) {
+      UMA_HISTOGRAM_ENUMERATION("SpdyProxyAuth.State",
+                                SPDY_PROXY_AUTH_OFF_BY_USER,
+                                NUM_SPDY_PROXY_AUTH_STATE);
+      has_turned_off_ = true;
+    }
+  }
+}
+
+void DataReductionProxySettingsAndroid::OnIPAddressChanged() {
+  CheckDataReductionProxyIsAvailable(GetProxyCheckURL());
+}
+
+void DataReductionProxySettingsAndroid::CheckDataReductionProxyIsAvailable(
+    std::string url) {

[mmenke, 2013/09/09 15:40:05]

This should be a const std::string&, no?

[bengr, 2013/09/10 00:56:09]

Done.

+  fetcher_.reset(net::URLFetcher::Create(0, GURL(url),
+                                         net::URLFetcher::GET, this));
+  fetcher_->SetLoadFlags(net::LOAD_DISABLE_CACHE |
+                         net::LOAD_IGNORE_ALL_CERT_ERRORS);

[mmenke, 2013/09/09 15:40:05]

Why are we ignoring cert errors?

[bengr, 2013/09/10 00:56:09]

Done.

+  Profile* profile = g_browser_process->profile_manager()->
+      GetDefaultProfile();
+  fetcher_->SetRequestContext(profile->GetRequestContext());
+  // Configure to max_retries at most kMaxRetries times for 5xx errors.
+  static const int kMaxRetries = 5;
+  fetcher_->SetMaxRetriesOn5xx(kMaxRetries);
+  fetcher_->Start();
+}
+
+void DataReductionProxySettingsAndroid::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  net::URLRequestStatus status = source->GetStatus();
+  if (status.status() == net::URLRequestStatus::FAILED &&
+      status.error() == net::ERR_INTERNET_DISCONNECTED) {
+    return;
+  }
+
+  std::string response;
+  source->GetResponseAsString(&response);
+
+  if ("OK" == response.substr(0, 2)) {
+    DVLOG(1) << "The data reduction proxy is not blocked.";
+
+    // The user enabled the proxy, but sometime previously in the session,
+    // the carrier had disabled the proxy. Now that the carrier is enabling
+    // it, configure it to the user's desires.
+    if (enabled_by_user_ && disabled_by_carrier_)
+      SetProxyPac(true, false);
+    disabled_by_carrier_ = false;
+    return;
+  }
+  DVLOG(1) << "The data reduction proxy is blocked.";
+  // Disable the proxy.
+  if (enabled_by_user_ && !disabled_by_carrier_)
+    SetProxyPac(false, false);
+  disabled_by_carrier_ = true;
+}
+
+std::string DataReductionProxySettingsAndroid::GetProxyCheckURL() {
+  std::string origin = "http://" + GetDataReductionProxyOriginHostPort();
+  GURL gurl(origin);
+  return gurl.scheme() + "://enabled." + gurl.host() + gurl.path() + "connect";

[mmenke, 2013/09/09 15:40:05]

There's a class call GURL::Replacements, or something like that, for this
purpose.  I think it's a lot clearer to use it than a roll-your-own solution.

[bengr, 2013/09/10 00:56:09]

Done.

+}
+
+ScopedJavaLocalRef<jlongArray>
+DataReductionProxySettingsAndroid::GetDailyContentLengths(
+    JNIEnv* env, const char* pref_name) {
+ jlongArray result = env->NewLongArray(kNumDaysInHistory);

[mmenke, 2013/09/09 16:01:07]

nit:  Fix indent.

[bengr, 2013/09/10 00:56:09]

Done.

+  PrefService* local_state = GetLocalState();
+  if (!local_state)
+    return ScopedJavaLocalRef<jlongArray>(env, result);
+
+  const ListValue* list_value = local_state->GetList(pref_name);

[mmenke, 2013/09/09 16:01:07]

This guaranteed to exist and be a list?

[bengr, 2013/09/10 00:56:09]

Yes. We'll hit a NOTREACHED if the pref isn't registered or the type isn't
base::Value::TYPE_LIST.

+  if (list_value->GetSize() != kNumDaysInHistory)
+    return ScopedJavaLocalRef<jlongArray>(env, result);
+
+  jlong jval[kNumDaysInHistory];
+  for (size_t i = 0; i < kNumDaysInHistory; ++i) {
+    int64 val = 0;
+    std::string pref_value;
+    bool rv = list_value->GetString(i, &pref_value);
+    DCHECK(rv);
+    if (rv) {
+      rv = base::StringToInt64(pref_value, &val);
+      DCHECK(rv);
+    }

[mmenke, 2013/09/09 16:01:07]

Suggest just making the body of this loop a function, and using it here and in
GetContentLengths...

[bengr, 2013/09/10 00:56:09]

Done.

+    jval[i] = val;
+  }
+  env->SetLongArrayRegion(result, 0, kNumDaysInHistory, jval);
+  return ScopedJavaLocalRef<jlongArray>(env, result);
+}
+
+ScopedJavaLocalRef<jlongArray>
+DataReductionProxySettingsAndroid::GetDailyOriginalContentLengths(
+    JNIEnv* env, jobject obj) {
+  return GetDailyContentLengths(env, prefs::kDailyHttpOriginalContentLength);
+}
+
+ScopedJavaLocalRef<jlongArray>
+DataReductionProxySettingsAndroid::GetDailyReceivedContentLengths(
+    JNIEnv* env, jobject obj) {
+  return GetDailyContentLengths(env, prefs::kDailyHttpReceivedContentLength);
+}
+
+void DataReductionProxySettingsAndroid::GetContentLengths(
+    int days,
+    int64* original_content_length,
+    int64* received_content_length,
+    int64* last_update_time) {

[mmenke, 2013/09/09 16:01:07]

Should DCHECK that days is no more than kDaysInHsitory.

[bengr, 2013/09/10 00:56:09]

Done.

+  PrefService* local_state = GetLocalState();
+  if (!local_state) {
+    *original_content_length = 0L;
+    *received_content_length = 0L;
+    *last_update_time = 0L;
+    return;
+  }
+
+  const ListValue* original_list =
+      local_state->GetList(prefs::kDailyHttpOriginalContentLength);
+  const ListValue* received_list =
+      local_state->GetList(prefs::kDailyHttpReceivedContentLength);
+  int64 last_update_internal =
+      local_state->GetInt64(prefs::kDailyHttpContentLengthLastUpdateDate);
+
+  if (original_list->GetSize() != kNumDaysInHistory ||
+      received_list->GetSize() != kNumDaysInHistory) {

[mmenke, 2013/09/09 16:01:07]

Are we guaranteed these will exists and be lists?

[bengr, 2013/09/10 00:56:09]

Yes. We'll hit a NOTREACHED if the pref isn't registered or the type isn't
base::Value::TYPE_LIST.

+    *original_content_length = 0L;
+    *received_content_length = 0L;
+    *last_update_time = 0L;
+    return;
+  }
+
+  int64 orig = 0L;
+  int64 recv = 0L;
+  DCHECK_GE(static_cast<int>(kNumDaysInHistory), days);
+  DCHECK_LE(0, days);
+  // We include days from the end of the list going backwards.
+  for (int i = 0; i < days; ++i) {
+    int read_index = kNumDaysInHistory - 1 - i;
+    std::string result("0");

[mmenke, 2013/09/09 16:01:07]

Initialization to 0 doesn't seem to serve any purpose here, since you check
return values.

[bengr, 2013/09/10 00:56:09]

Done.

+    int64 val;
+    if (original_list->GetString(read_index, &result)) {
+      if (base::StringToInt64(result, &val))
+        orig +=val;

[mmenke, 2013/09/09 16:01:07]

nit:  Add space.

[bengr, 2013/09/10 00:56:09]

Done.

+      else
+        DCHECK(false);

[mmenke, 2013/09/09 16:01:07]

These should be NOTREACHED().

[bengr, 2013/09/10 00:56:09]

Done.

+    } else {
+      DCHECK(false);
+    }
+    if (received_list->GetString(read_index, &result)) {
+      if (base::StringToInt64(result, &val))
+        recv +=val;

[mmenke, 2013/09/09 16:01:07]

nit:  Add space.

[bengr, 2013/09/10 00:56:09]

Done.

+      else
+        DCHECK(false);
+    } else {
+      DCHECK(false);
+    }
+  }
+  *original_content_length = orig;
+  *received_content_length = recv;
+  *last_update_time = last_update_internal;
+}
+
+static jint Init(JNIEnv* env, jobject obj) {

[mmenke, 2013/09/09 16:01:07]

This does seem to be used anywere.

[bengr, 2013/09/10 00:56:09]

This is used by generated jni code. I've added a comment.

+  DataReductionProxySettingsAndroid* settings =
+      new DataReductionProxySettingsAndroid(env, obj);
+  return reinterpret_cast<jint>(settings);
+}
+
+// static
+bool DataReductionProxySettingsAndroid::Register(JNIEnv* env) {
+  bool register_natives_impl_result = RegisterNativesImpl(env);
+  return register_natives_impl_result;
+}