Upstream Android proxy config service.

net/proxy/proxy_config_service_android.cc

Index: net/proxy/proxy_config_service_android.cc
diff --git a/net/proxy/proxy_config_service_android.cc b/net/proxy/proxy_config_service_android.cc
new file mode 100644
index 0000000000000000000000000000000000000000..eb486ed2fc1cb930cc2261c913e16d17b5eb4d5b
--- /dev/null
+++ b/net/proxy/proxy_config_service_android.cc
@@ -0,0 +1,258 @@
+// Copyright (c) 2012 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 "net/proxy/proxy_config_service_android.h"
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
+#include "base/memory/ref_counted.h"
+#include "base/string_tokenizer.h"
+#include "base/string_util.h"
+#include "jni/proxy_change_listener_jni.h"
+#include "net/proxy/proxy_config.h"
+
+#include <sys/system_properties.h>
+
+using base::android::AttachCurrentThread;
+using base::android::ConvertUTF8ToJavaString;
+using base::android::ConvertJavaStringToUTF8;
+using base::android::CheckException;
+using base::android::ClearException;
+using base::android::GetMethodID;
+
+namespace net {
+
+namespace {
+
+ProxyServer ConstructProxyServer(ProxyServer::Scheme scheme,
+                                 const std::string& proxy_host,
+                                 const std::string& proxy_port) {
+  DCHECK(!proxy_host.empty());
+  if (proxy_port.empty())
+    return ProxyServer::FromURI(proxy_host, scheme);
+  return ProxyServer::FromURI(proxy_host + ":" + proxy_port, scheme);

[Ryan Sleevi, 2012/05/07 17:12:20]

What happens if |proxy_host| is an IPv6 host (a:b)?

net::HostPortPair::ToString() handles this, so you should instead pass around
one of those.

[Philippe, 2012/05/09 11:48:40]

Done.

+}
+
+ProxyServer LookupProxy(
+    const std::string& prefix,

[Ryan Sleevi, 2012/05/07 17:12:20]

Is it valid for prefix to be .empty()?

DCHECK for programmer error?

[Philippe, 2012/05/09 11:48:40]

Done.

+    ProxyConfigServiceAndroid::Delegate* delegate,
+    ProxyServer::Scheme scheme) {
+  std::string proxy_host = delegate->GetProperty(prefix + ".proxyHost");
+  if (!proxy_host.empty()) {
+    const std::string proxy_port = delegate->GetProperty(prefix + ".proxyPort");

[Ryan Sleevi, 2012/05/07 17:12:20]

I don't believe I've seen in Chromium code the optimistic const-ing here. I
don't believe the style guide prohibits it, but as far as local style goes, it's
very rare to see this.

I'm guessing you're doing this simply for local documentation sake, as the
compiler should be able to optimize fine regardless of the annotation?

I don't feel strongly about this, but I did want to point it out that it's
uncommon.

[Philippe, 2012/05/09 11:48:40]

It's mainly for local documentation sake as you said. I think it lets the reader
more quickly catch the mutable state. It seems that it is more common indeed in
Google3 code than in Chromium. I removed the consts here (that I previously
added) as they don't add much value in this particular case (very restricted
scope). However I think that "const" really adds value when used to make
class/struct state immutable.

+    return ConstructProxyServer(scheme, proxy_host, proxy_port);
+  }
+  // Fall back to default proxy, if any.
+  proxy_host = delegate->GetProperty("proxyHost");
+  if (!proxy_host.empty()) {
+    const std::string proxy_port = delegate->GetProperty("proxyPort");

[Ryan Sleevi, 2012/05/07 17:12:20]

You check !proxy_host.empty(), but never check proxy_port.empty(). And then in
ConstructProxyServer, you always concat the two.

So if a proxy server was 'foo' with a port of '' for a scheme of http, your
returned setup would be 'HTTP foo:', which is a bit of an odd construction.

Same for all other occurrences of this pattern.

[John Knottenbelt, 2012/05/08 11:43:12]

ConstructProxyServer checks for empty port to avoid the trailing ':'

On 2012/05/07 17:12:20, Ryan Sleevi wrote:

+    return ConstructProxyServer(scheme, proxy_host, proxy_port);
+  }
+  return ProxyServer();
+}
+
+ProxyServer LookupSocksProxy(
+    ProxyConfigServiceAndroid::Delegate* delegate) {
+  const std::string proxy_host = delegate->GetProperty("socksProxyHost");
+  if (!proxy_host.empty()) {
+    const std::string proxy_port = delegate->GetProperty("socksProxyPort");
+    return ConstructProxyServer(ProxyServer::SCHEME_SOCKS5, proxy_host,
+                                proxy_port);
+  }
+  return ProxyServer();
+}
+
+void AddBypassRules(
+    const std::string& scheme,
+    ProxyConfigServiceAndroid::Delegate* delegate,
+    ProxyBypassRules* bypass_rules) {
+  // The format of a hostname pattern is a list of hostnames that are separated
+  // by | and that use * as a wildcard. For example, setting the
+  // http.nonProxyHosts property to *.android.com|*.kernel.org will cause
+  // requests to http://developer.android.com to be made without a proxy.
+  const std::string non_proxy_hosts =
+      delegate->GetProperty(scheme + ".nonProxyHosts");
+  if (non_proxy_hosts.empty())
+    return;
+  StringTokenizer tokenizer(non_proxy_hosts, "|");
+  while (tokenizer.GetNext()) {
+    const std::string token = tokenizer.token();
+    std::string pattern;
+    TrimWhitespaceASCII(token, TRIM_ALL, &pattern);
+    if (pattern.empty())
+      continue;
+    // '?' is not one of the specified pattern characters above.
+    DCHECK_EQ(std::string::npos, pattern.find('?'));
+    bypass_rules->AddRuleForHostname(scheme, pattern, -1);

[Ryan Sleevi, 2012/05/07 17:12:20]

This method returns bool. As it stands, you'll simply drop that host - eg:
*.foo|something_invalid|*.bar becomes "*.foo" and "*.bar" in the bypass rules

Is this what you want? Should you make AddBypassRules return a bool so that
GetProxyRules will also fail?

[John Knottenbelt, 2012/05/08 11:43:12]

The default java.net.ProxySelector (
eloper.android.com/reference/java/net/ProxySelector.html ) implementation in
Android is tolerant to invalid parts, so I think we should try to be too.

On 2012/05/07 17:12:20, Ryan Sleevi wrote:

+  }
+}
+
+// returns true if a valid proxy was found.
+bool GetProxyRules(
+    ProxyConfigServiceAndroid::Delegate* delegate,
+    ProxyConfig::ProxyRules* rules) {
+  // See libcore/luni/src/main/java/java/net/ProxySelectorImpl.java for the
+  // semantics we're trying to match.
+  rules->type = ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME;
+  rules->proxy_for_http = LookupProxy("http", delegate,
+                                      ProxyServer::SCHEME_HTTP);
+  rules->proxy_for_https = LookupProxy("https", delegate,
+                                       ProxyServer::SCHEME_HTTPS);
+  rules->proxy_for_ftp = LookupProxy("ftp", delegate, ProxyServer::SCHEME_HTTP);
+  rules->fallback_proxy = LookupSocksProxy(delegate);
+  rules->bypass_rules.Clear();
+  AddBypassRules("ftp", delegate, &rules->bypass_rules);
+  AddBypassRules("http", delegate, &rules->bypass_rules);
+  AddBypassRules("https", delegate, &rules->bypass_rules);
+  return rules->proxy_for_http.is_valid() ||
+      rules->proxy_for_https.is_valid() ||
+      rules->proxy_for_ftp.is_valid() ||
+      rules->fallback_proxy.is_valid();
+};
+
+void GetLatestProxyConfigInternal(
+    ProxyConfigServiceAndroid::Delegate* delegate,
+    ProxyConfig* config) {
+  if (!GetProxyRules(delegate, &config->proxy_rules()))
+    *config = ProxyConfig::CreateDirect();
+}
+
+// This class implements the link to Android's proxy broadcast mechanism.
+class DelegateImpl : public ProxyConfigServiceAndroid::Delegate {
+ public:
+  DelegateImpl() : service_(NULL) {}
+
+  virtual void Start(ProxyConfigServiceAndroid* service) OVERRIDE {
+    DCHECK(!service_);
+    JNIEnv* env = AttachCurrentThread();
+    service_ = service;
+    if (java_proxy_change_listener_android_.is_null()) {
+      java_proxy_change_listener_android_.Reset(
+          Java_ProxyChangeListener_create(
+              env, base::android::GetApplicationContext()));
+      CHECK(!java_proxy_change_listener_android_.is_null());
+    }
+    Java_ProxyChangeListener_start(
+        env,
+        java_proxy_change_listener_android_.obj(),
+        reinterpret_cast<jint>(service));
+  }
+
+  virtual void Stop() OVERRIDE {
+    // The java object will be NULL if Stop() was already called (e.g. after a
+    // Start()), or if Start() was never called. Stop() is called from the
+    // destructor of ProxyConfigServiceAndroid.
+    if (java_proxy_change_listener_android_.is_null())
+      return;
+    JNIEnv* env = AttachCurrentThread();
+    Java_ProxyChangeListener_stop(env,
+                                  java_proxy_change_listener_android_.obj());
+  }
+
+  virtual std::string GetProperty(const std::string& property) OVERRIDE {
+    // Use Java System.getProperty to get configuration information.
+    // Question: Conversion to/from UTF8 ok here?
+    JNIEnv* env = AttachCurrentThread();
+    ScopedJavaLocalRef<jstring> str = ConvertUTF8ToJavaString(env, property);
+    ScopedJavaLocalRef<jstring> result =
+        Java_ProxyChangeListener_getProperty(env, str.obj());
+    return result.is_null() ? "" : ConvertJavaStringToUTF8(env, result.obj());
+  }
+
+ private:
+  ProxyConfigServiceAndroid* service_;
+  base::android::ScopedJavaGlobalRef<jobject>
+      java_proxy_change_listener_android_;
+};
+
+}  // namespace
+
+struct ProxyConfigServiceAndroid::SharedState
+    : public base::RefCountedThreadSafe<SharedState> {
+  SharedState(Delegate* delegate) : delegate_(delegate) {
+    DCHECK(delegate);
+  }
+
+  // Non-reassignable pointer thus safely readable by any thread.
+  const scoped_ptr<Delegate> delegate_;
+  // Note that this is only accessed from the observer thread.
+  ObserverList<Observer> observers_;
+};
+
+ProxyConfigServiceAndroid::ProxyConfigServiceAndroid(
+    scoped_refptr<base::MessageLoopProxy> observer_loop)
+    : shared_state_(new SharedState(new DelegateImpl)),
+      observer_loop_(observer_loop) {
+  DCHECK(observer_loop_.get());
+}
+
+ProxyConfigServiceAndroid::ProxyConfigServiceAndroid(
+    scoped_refptr<base::MessageLoopProxy> observer_loop,
+    ProxyConfigServiceAndroid::Delegate* delegate)
+    : shared_state_(new SharedState(delegate)),
+      observer_loop_(observer_loop) {
+  DCHECK(observer_loop_.get());
+}
+
+ProxyConfigServiceAndroid::~ProxyConfigServiceAndroid() {
+  DCHECK(OnObserverThread());

[Ryan Sleevi, 2012/05/07 17:12:20]

I think it's still very subtle/unexpected that this can be created on thread A,
B, or C but must always be destroyed on thread X.

[John Knottenbelt, 2012/05/08 11:43:12]

I think it should be OK to relax this requirement now that the SharedState
struct has been introduced. We can just schedule a call to the stop method on
the observer thread if we're not already on it.

If there are still observers there would then be a possibility that observers
can get notifications after this object is destroyed. 

Ryan can you confirm if is an invariant that all observers are removed by the
time the Proxy Service object is destroyed? If so, we could add a DCHECK to that
effect in the method that runs on the observer thread.



On 2012/05/07 17:12:20, Ryan Sleevi wrote:

[Philippe, 2012/05/09 11:48:40]

I'm waiting for Ryan's feedback before I address this issue.

+  shared_state_->delegate_->Stop();
+}
+
+// static
+bool ProxyConfigServiceAndroid::Init(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+void ProxyConfigServiceAndroid::AddObserver(Observer* observer) {
+  DCHECK(OnObserverThread());
+  if (shared_state_->observers_.size() == 0)
+    shared_state_->delegate_->Start(this);
+  // Note that any callbacks from the delegate will result in a scheduled task
+  // on this same thread, so there is no race between delegate_->Start() and
+  // AddObserver().
+  shared_state_->observers_.AddObserver(observer);
+}
+
+void ProxyConfigServiceAndroid::RemoveObserver(Observer* observer) {
+  DCHECK(OnObserverThread());
+  shared_state_->observers_.RemoveObserver(observer);
+  if (shared_state_->observers_.size() == 0) {
+    shared_state_->delegate_->Stop();
+  }
+}
+
+ProxyConfigService::ConfigAvailability
+ProxyConfigServiceAndroid::GetLatestProxyConfig(ProxyConfig* config) {
+  if (!config)
+    return ProxyConfigService::CONFIG_UNSET;
+  GetLatestProxyConfigInternal(shared_state_->delegate_.get(), config);
+  return ProxyConfigService::CONFIG_VALID;
+}
+
+void ProxyConfigServiceAndroid::ProxySettingsChanged() {
+  observer_loop_->PostTask(
+      FROM_HERE, base::Bind(&ProxySettingsChangedCallback, shared_state_));
+}
+
+// static
+void ProxyConfigServiceAndroid::ProxySettingsChangedCallback(
+    scoped_refptr<SharedState> callback_state) {
+  // Allow another task to be scheduled (before running observers).
+  ProxyConfig config;
+  GetLatestProxyConfigInternal(callback_state->delegate_.get(), &config);
+  FOR_EACH_OBSERVER(Observer, callback_state->observers_,
+                    OnProxyConfigChanged(config,
+                                         ProxyConfigService::CONFIG_VALID));
+}
+
+bool ProxyConfigServiceAndroid::OnObserverThread() const {
+  return observer_loop_->BelongsToCurrentThread();
+}
+
+} // namespace net