Upstream Android proxy config service.

net/proxy/proxy_config_service_android.h

+// 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.
+
+#ifndef NET_PROXY_PROXY_CONFIG_SERVICE_ANDROID_H_
+#define NET_PROXY_PROXY_CONFIG_SERVICE_ANDROID_H_
+#pragma once
+
+#include <string>
+
+#include "base/android/scoped_java_ref.h"

[Ryan Sleevi, 2012/05/14 18:00:25]

nit: I don't see this being used in this header

[Philippe, 2012/05/15 16:12:48]

Done.

+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"

[Ryan Sleevi, 2012/05/14 18:00:25]

nit: nor this?

[Philippe, 2012/05/15 16:12:48]

Done.

+#include "base/observer_list.h"

[Ryan Sleevi, 2012/05/14 18:00:25]

or this

[Philippe, 2012/05/15 16:12:48]

Indeed. It seems that I forgot to remove these includes after I added the new
SharedState class. 

+#include "net/base/net_export.h"
+#include "net/proxy/proxy_config.h"

[Ryan Sleevi, 2012/05/14 18:00:25]

nit: You can forward declare net::ProxyConfig - it's only used as a pointer
(line 57)

[Philippe, 2012/05/15 16:12:48]

Done.

+#include "net/proxy/proxy_config_service.h"
+
+namespace base {
+class SingleThreadTaskRunner;
+}
+
+namespace net {
+
+class NET_EXPORT ProxyConfigServiceAndroid : public ProxyConfigService {
+ public:
+  // Delegate abstracts access to the platform.
+  // Owned by ProxyConfigServiceAndroid.
+  class Delegate {
+   public:
+    virtual ~Delegate() {}
+
+    // Notifies the platform that the proxy config service started. The caller
+    // keeps ownership of |service|.
+    virtual void Start(ProxyConfigServiceAndroid* service) = 0;
+    virtual void Stop() = 0;
+
+    // Returns the value of the property identified by the provided key. If it
+    // was not found, an empty string is returned. Note that this interface
+    // does not let you distinguish an empty property from a non-existing
+    // property.
+    virtual std::string GetProperty(const std::string& property) = 0;
+  };
+
+  ProxyConfigServiceAndroid(
+      scoped_refptr<base::SingleThreadTaskRunner> observer_runner);
+  virtual ~ProxyConfigServiceAndroid();
+
+  // Register JNI bindings.
+  static bool Register(JNIEnv* env);
+
+  // ProxyConfigService:
+  // Observer-related operations must be called on the observer thread.
+  virtual void AddObserver(Observer* observer) OVERRIDE;
+  virtual void RemoveObserver(Observer* observer) OVERRIDE;

[Ryan Sleevi, 2012/05/14 18:00:25]

BUG? AddObserver/RemoveObserver are called on the IO thread, not the UI thread -
at least, this is true for ProxyConfigServiceMac and ProxyConfigServiceLinux.

It sounds like the TaskRunner you take should be for the JNI runner, not the
observer runner.

[Philippe, 2012/05/15 16:12:48]

I might be missing something but the observer thread is the IO thread. I can
rename observer_runner_ if you want (io_runner_?). 

+
+  virtual ConfigAvailability GetLatestProxyConfig(ProxyConfig* config) OVERRIDE;
+
+  void ProxySettingsChanged();
+
+  // Called from Java to signal that the proxy settings have changed.
+  void ProxySettingsChanged(JNIEnv*, jobject) { ProxySettingsChanged(); }

[Ryan Sleevi, 2012/05/14 18:00:25]

Avoid overloads (Line 59 & 62) -
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

[Philippe, 2012/05/15 16:12:48]

Done.

+
+ private:
+  friend class ProxyConfigServiceAndroidTest;
+
+  // For tests; takes ownership of |delegate|.
+  ProxyConfigServiceAndroid(
+      scoped_refptr<base::SingleThreadTaskRunner> observer_runner,

[Ryan Sleevi, 2012/05/14 18:00:25]

Same here - take a naked pointer.

[Philippe, 2012/05/15 16:12:48]

Done.

+      Delegate* delegate);
+
+  // Structure holding the state used by both this class and the asynchronous
+  // callback running on the observer thread. Instances of SharedState are
+  // referenced through a shared pointer so that we handle the case where
+  // ProxyConfigServiceAndroid gets deleted before the asynchronous callback
+  // (using this state) runs.
+  struct SharedState;
+
+  // Called on observer thread.
+  static void ProxySettingsChangedCallback(
+      scoped_refptr<SharedState> callback_state);

[Ryan Sleevi, 2012/05/14 18:00:25]

const scoped_refptr<SharedState>& callback_state

[Philippe, 2012/05/15 16:12:48]

I might be wrong, but it seems safer in this particular case to make a copy of
the scoped_refptr since ProxyConfigServiceAndroid can be deleted before the
asynchronous task runs (which is why we have SharedState). In that case
&shared_state_ would be a pointer to a "random" place.

[Philippe, 2012/05/16 08:01:06]

I was wrong. I forgot when I wrote the previous message that the scoped_refptr
is already copied by Bind() when creating the callback. 

+  bool OnObserverThread() const;
+
+  const scoped_refptr<SharedState> shared_state_;
+
+  // Task runner of the thread on which observers are notified whenever proxy
+  // settings change.
+  const scoped_refptr<base::SingleThreadTaskRunner> observer_runner_;

[Ryan Sleevi, 2012/05/14 18:00:25]

DISALLOW_COPY_AND_ASSIGN(ProxyConfigServiceAndroid)

[Philippe, 2012/05/15 16:12:48]

Done.

+};
+
+} // namespace net
+
+#endif // NET_PROXY_PROXY_CONFIG_SERVICE_ANDROID_H_