Upstream Android proxy config service.

net/android/java/org/chromium/net/ProxyChangeListener.java

Index: net/android/java/org/chromium/net/ProxyChangeListener.java
diff --git a/net/android/java/org/chromium/net/ProxyChangeListener.java b/net/android/java/org/chromium/net/ProxyChangeListener.java
new file mode 100644
index 0000000000000000000000000000000000000000..d3d6d9697b258c0eec575edf9899a381b386b427
--- /dev/null
+++ b/net/android/java/org/chromium/net/ProxyChangeListener.java
@@ -0,0 +1,91 @@
+// 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.
+
+package org.chromium.net;
+
+import android.content.BroadcastReceiver;
+import android.content.Context;
+import android.content.Intent;
+import android.content.IntentFilter;
+import android.net.Proxy;
+import org.chromium.base.CalledByNative;
+
+// This class partners with native ProxyConfigServiceAndroid to listen for
+// proxy change notifications from Android.
+class ProxyChangeListener {
+    private static final String TAG = "ProxyChangeListener";
+
+    private ProxyChangeListener(Context context) {
+        mContext = context;
+    }
+
+    @CalledByNative
+    static public ProxyChangeListener create(Context context) {
+        return new ProxyChangeListener(context);
+    }
+
+    @CalledByNative
+    static public String getProperty(String property) {
+        return System.getProperty(property);
+    }
+
+    @CalledByNative
+    public void start(int nativePtr) {
+        assert mNativePtr == 0;
+        mNativePtr = nativePtr;
+        registerReceiver();
+    }
+
+    @CalledByNative
+    public void stop() {
+        mNativePtr = 0;
+        unregisterReceiver();
+    }
+
+    private class ProxyReceiver extends BroadcastReceiver {
+        @Override
+        public void onReceive(Context context, Intent intent) {
+            if (intent.getAction().equals(Proxy.PROXY_CHANGE_ACTION)) {
+                proxySettingsChanged();
+            }
+        }
+    }
+
+    private void proxySettingsChanged() {
+        if (mNativePtr == 0)
+            return;
+
+        // Note that this code currently runs on a MESSAGE_LOOP_UI thread, but
+        // the C++ code must run the callbacks on the network thread.
+        nativeProxySettingsChanged(mNativePtr);
+    }
+
+    /**
+     * See net/proxy/proxy_config_service_android.cc
+     */
+    private native void nativeProxySettingsChanged(
+            int nativePtr /* net::ProxyConfigServiceAndroid::JNIDelegate */);

[Ryan Sleevi, 2012/06/04 21:49:18]

Does the JNI generator let you forward declare like this? In C++, you can't
forward declare derived classes (eg: it expects ProxyConfigServiceAndroid is a
namespace, rather than a class).

If this works, its only working because net/proxy/proxy_config_service_android.h
has already been included, which seems dangerous because the JNI file here
cannot express that.

net::JNIDelegate (or net::jni::delegate, or jni::net::delegate, or whatever) -
that is, removing it from being a derived interface and moving it into the
top-level namespace, should avoid this.

[Philippe, 2012/06/05 09:07:54]

Good point.

There is no forward declaration in the generated code. The type
net::ProxyConfigServiceAndroid::JNIDelegate is used only once to do a
reinterpret_cast. Therefore the generated code does rely indeed on the fact that
its client must include the appropriate headers. More importantly, after the
reinterpret_cast, the generated code calls a method of ProxyConfigServiceAndroid
thus can't rely on a forward declaration:

static void ProxySettingsChanged(JNIEnv* env, jobject obj,
    jint nativePtr) {
  DCHECK(nativePtr) << "ProxySettingsChanged";
  net::ProxyConfigServiceAndroid::JNIDelegate* native =
      reinterpret_cast<net::ProxyConfigServiceAndroid::JNIDelegate*>(nativePtr);
  return native->ProxySettingsChanged(env, obj);
}

We are including this generated header only in our cc file. The only risk I can
see is that the JNI generator changes significantly and starts generating
forward declarations. This would be a major change since the current
implementation, calling a method of the user-provided type, can't rely on a
forward declaration (inability to call a method from an incomplete type). In the
worst case, if that happened (seems unlikely), this would produce an error when
compiling proxy_config_service_android.cc. 

I'm just a little concerned about adding some extra friend declarations and more
importantly using the namespace net (I would probably have to find a name more
specific than "JNIDelegate"). I think that keeping the class as it is currently
should be reasonably safe for the reasons I mentioned above.

What do you think?

[Ryan Sleevi, 2012/06/05 16:50:14]

I see. The important take-away here is that all of the code is within the
generated header, rather than being a .h/.cc file. As such, and because this
generated header is only included in the proxy_config_service_android.cc
(avoiding ODR violations), this is "safe."
I think it's fine to leave as-is. This particular edge case of the build system
surprises me, but that's what happens when you glue languages together :)

+
+    private void registerReceiver() {
+        if (mProxyReceiver != null) {
+            return;
+        }
+        IntentFilter filter = new IntentFilter();
+        filter.addAction(Proxy.PROXY_CHANGE_ACTION);
+        mProxyReceiver = new ProxyReceiver();
+        mContext.getApplicationContext().registerReceiver(mProxyReceiver, filter);
+    }
+
+    private void unregisterReceiver() {
+        if (mProxyReceiver == null) {
+            return;
+        }
+        mContext.unregisterReceiver(mProxyReceiver);
+        mProxyReceiver = null;
+    }
+
+    private int mNativePtr;
+    private Context mContext;
+    private ProxyReceiver mProxyReceiver;
+}