Commit 6815aefe authored by pauljensen's avatar pauljensen Committed by Commit bot

[Cronet] Move initialization to a new thread rather than the UI thread.

The UI thread is generally overbooked (esp at app startup time) so
avoiding it can greatly improve Cronet startup time.  The UI thread
was used for initialization previously because it simplified the
NetworkChangeNotifierAutoDetect and ProxyChangeListener logic because
BroadcastReceiver onReceived callbacks always happen on the UI thread
so those classes could be completely single-threaded.  This change
leaves those classes single-threaded except for their onRecieved()
methods which now immediately post to the new initialization thread
and check whether their BroadcastReceivers are currently registered.

BUG=709336
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2812963002
Cr-Commit-Position: refs/heads/master@{#469578}
parent 6bd1fc8a
...@@ -53,9 +53,9 @@ const base::android::RegistrationMethod kCronetRegisteredMethods[] = { ...@@ -53,9 +53,9 @@ const base::android::RegistrationMethod kCronetRegisteredMethods[] = {
{"NetAndroid", net::android::RegisterJni}, {"NetAndroid", net::android::RegisterJni},
}; };
// MessageLoop on the main thread, which is where objects that receive Java // MessageLoop on the init thread, which is where objects that receive Java
// notifications generally live. // notifications generally live.
base::MessageLoop* g_main_message_loop = nullptr; base::MessageLoop* g_init_message_loop = nullptr;
net::NetworkChangeNotifier* g_network_change_notifier = nullptr; net::NetworkChangeNotifier* g_network_change_notifier = nullptr;
...@@ -76,6 +76,11 @@ bool NativeInit() { ...@@ -76,6 +76,11 @@ bool NativeInit() {
} // namespace } // namespace
bool OnInitThread() {
DCHECK(g_init_message_loop);
return g_init_message_loop == base::MessageLoop::current();
}
// Checks the available version of JNI. Also, caches Java reflection artifacts. // Checks the available version of JNI. Also, caches Java reflection artifacts.
jint CronetOnLoad(JavaVM* vm, void* reserved) { jint CronetOnLoad(JavaVM* vm, void* reserved) {
base::android::InitVM(vm); base::android::InitVM(vm);
...@@ -91,7 +96,7 @@ void CronetOnUnLoad(JavaVM* jvm, void* reserved) { ...@@ -91,7 +96,7 @@ void CronetOnUnLoad(JavaVM* jvm, void* reserved) {
base::android::LibraryLoaderExitHook(); base::android::LibraryLoaderExitHook();
} }
void CronetInitOnMainThread(JNIEnv* env, const JavaParamRef<jclass>& jcaller) { void CronetInitOnInitThread(JNIEnv* env, const JavaParamRef<jclass>& jcaller) {
#if !BUILDFLAG(USE_PLATFORM_ICU_ALTERNATIVES) #if !BUILDFLAG(USE_PLATFORM_ICU_ALTERNATIVES)
base::i18n::InitializeICU(); base::i18n::InitializeICU();
#endif #endif
...@@ -101,9 +106,8 @@ void CronetInitOnMainThread(JNIEnv* env, const JavaParamRef<jclass>& jcaller) { ...@@ -101,9 +106,8 @@ void CronetInitOnMainThread(JNIEnv* env, const JavaParamRef<jclass>& jcaller) {
// configuration information. // configuration information.
base::CommandLine::Init(0, nullptr); base::CommandLine::Init(0, nullptr);
DCHECK(!base::MessageLoop::current()); DCHECK(!base::MessageLoop::current());
DCHECK(!g_main_message_loop); DCHECK(!g_init_message_loop);
g_main_message_loop = new base::MessageLoopForUI(); g_init_message_loop = new base::MessageLoop();
base::MessageLoopForUI::current()->Start();
DCHECK(!g_network_change_notifier); DCHECK(!g_network_change_notifier);
net::NetworkChangeNotifier::SetFactory( net::NetworkChangeNotifier::SetFactory(
new net::NetworkChangeNotifierFactoryAndroid()); new net::NetworkChangeNotifierFactoryAndroid());
......
...@@ -12,6 +12,10 @@ namespace cronet { ...@@ -12,6 +12,10 @@ namespace cronet {
jint CronetOnLoad(JavaVM* vm, void* reserved); jint CronetOnLoad(JavaVM* vm, void* reserved);
void CronetOnUnLoad(JavaVM* jvm, void* reserved); void CronetOnUnLoad(JavaVM* jvm, void* reserved);
// Returns true when running on initialization thread.
// Only callable after initialization thread is started.
bool OnInitThread();
} // namespace cronet } // namespace cronet
#endif // COMPONENTS_CRONET_ANDROID_CRONET_LIBRARY_LOADER_H_ #endif // COMPONENTS_CRONET_ANDROID_CRONET_LIBRARY_LOADER_H_
...@@ -37,6 +37,7 @@ ...@@ -37,6 +37,7 @@
#include "base/values.h" #include "base/values.h"
#include "components/cronet/android/cert/cert_verifier_cache_serializer.h" #include "components/cronet/android/cert/cert_verifier_cache_serializer.h"
#include "components/cronet/android/cert/proto/cert_verification.pb.h" #include "components/cronet/android/cert/proto/cert_verification.pb.h"
#include "components/cronet/android/cronet_library_loader.h"
#include "components/cronet/histogram_manager.h" #include "components/cronet/histogram_manager.h"
#include "components/cronet/url_request_context_config.h" #include "components/cronet/url_request_context_config.h"
#include "components/prefs/pref_change_registrar.h" #include "components/prefs/pref_change_registrar.h"
...@@ -84,16 +85,16 @@ class NetLogWithNetworkChangeEvents { ...@@ -84,16 +85,16 @@ class NetLogWithNetworkChangeEvents {
net::NetLog* net_log() { return &net_log_; } net::NetLog* net_log() { return &net_log_; }
// This function registers with the NetworkChangeNotifier and so must be // This function registers with the NetworkChangeNotifier and so must be
// called *after* the NetworkChangeNotifier is created. Should only be // called *after* the NetworkChangeNotifier is created. Should only be
// called on the UI thread as it is not thread-safe and the UI thread is // called on the init thread as it is not thread-safe and the init thread is
// the thread the NetworkChangeNotifier is created on. This function is // the thread the NetworkChangeNotifier is created on. This function is
// not thread-safe because accesses to |net_change_logger_| are not atomic. // not thread-safe because accesses to |net_change_logger_| are not atomic.
// There might be multiple CronetEngines each with a network thread so // There might be multiple CronetEngines each with a network thread so
// so the UI thread is used. |g_net_log_| also outlives the network threads // so the init thread is used. |g_net_log_| also outlives the network threads
// so it would be unsafe to receive callbacks on the network threads without // so it would be unsafe to receive callbacks on the network threads without
// a complicated thread-safe reference-counting system to control callback // a complicated thread-safe reference-counting system to control callback
// registration. // registration.
void EnsureInitializedOnMainThread() { void EnsureInitializedOnInitThread() {
DCHECK(base::MessageLoopForUI::IsCurrent()); DCHECK(cronet::OnInitThread());
if (net_change_logger_) if (net_change_logger_)
return; return;
net_change_logger_.reset(new net::LoggingNetworkChangeObserver(&net_log_)); net_change_logger_.reset(new net::LoggingNetworkChangeObserver(&net_log_));
...@@ -504,7 +505,7 @@ CronetURLRequestContextAdapter::~CronetURLRequestContextAdapter() { ...@@ -504,7 +505,7 @@ CronetURLRequestContextAdapter::~CronetURLRequestContextAdapter() {
StopNetLogOnNetworkThread(); StopNetLogOnNetworkThread();
} }
void CronetURLRequestContextAdapter::InitRequestContextOnMainThread( void CronetURLRequestContextAdapter::InitRequestContextOnInitThread(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jobject>& jcaller) { const JavaParamRef<jobject>& jcaller) {
base::android::ScopedJavaGlobalRef<jobject> jcaller_ref; base::android::ScopedJavaGlobalRef<jobject> jcaller_ref;
...@@ -518,7 +519,7 @@ void CronetURLRequestContextAdapter::InitRequestContextOnMainThread( ...@@ -518,7 +519,7 @@ void CronetURLRequestContextAdapter::InitRequestContextOnMainThread(
// TODO(csharrison) Architect the wrapper better so we don't need to cast for // TODO(csharrison) Architect the wrapper better so we don't need to cast for
// android ProxyConfigServices. // android ProxyConfigServices.
android_proxy_config_service->set_exclude_pac_url(true); android_proxy_config_service->set_exclude_pac_url(true);
g_net_log.Get().EnsureInitializedOnMainThread(); g_net_log.Get().EnsureInitializedOnInitThread();
GetNetworkTaskRunner()->PostTask( GetNetworkTaskRunner()->PostTask(
FROM_HERE, FROM_HERE,
base::Bind(&CronetURLRequestContextAdapter::InitializeOnNetworkThread, base::Bind(&CronetURLRequestContextAdapter::InitializeOnNetworkThread,
......
...@@ -58,8 +58,8 @@ class CronetURLRequestContextAdapter ...@@ -58,8 +58,8 @@ class CronetURLRequestContextAdapter
~CronetURLRequestContextAdapter() override; ~CronetURLRequestContextAdapter() override;
// Called on main Java thread to initialize URLRequestContext. // Called on init Java thread to initialize URLRequestContext.
void InitRequestContextOnMainThread( void InitRequestContextOnInitThread(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller); const base::android::JavaParamRef<jobject>& jcaller);
...@@ -107,8 +107,8 @@ class CronetURLRequestContextAdapter ...@@ -107,8 +107,8 @@ class CronetURLRequestContextAdapter
// Default net::LOAD flags used to create requests. // Default net::LOAD flags used to create requests.
int default_load_flags() const { return default_load_flags_; } int default_load_flags() const { return default_load_flags_; }
// Called on main Java thread to initialize URLRequestContext. // Called on init Java thread to initialize URLRequestContext.
void InitRequestContextOnMainThread(); void InitRequestContextOnInitThread();
// Configures the network quality estimator to observe requests to localhost, // Configures the network quality estimator to observe requests to localhost,
// to use smaller responses when estimating throughput, and to disable the // to use smaller responses when estimating throughput, and to disable the
......
...@@ -6,6 +6,7 @@ package org.chromium.net.impl; ...@@ -6,6 +6,7 @@ package org.chromium.net.impl;
import android.content.Context; import android.content.Context;
import android.os.Handler; import android.os.Handler;
import android.os.HandlerThread;
import android.os.Looper; import android.os.Looper;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
...@@ -15,7 +16,7 @@ import org.chromium.base.annotations.JNINamespace; ...@@ -15,7 +16,7 @@ import org.chromium.base.annotations.JNINamespace;
import org.chromium.net.NetworkChangeNotifier; import org.chromium.net.NetworkChangeNotifier;
/** /**
* CronetLibraryLoader loads and initializes native library on main thread. * CronetLibraryLoader loads and initializes native library on init thread.
*/ */
@JNINamespace("cronet") @JNINamespace("cronet")
@VisibleForTesting @VisibleForTesting
...@@ -24,14 +25,18 @@ public class CronetLibraryLoader { ...@@ -24,14 +25,18 @@ public class CronetLibraryLoader {
private static final Object sLoadLock = new Object(); private static final Object sLoadLock = new Object();
private static final String LIBRARY_NAME = "cronet." + ImplVersion.getCronetVersion(); private static final String LIBRARY_NAME = "cronet." + ImplVersion.getCronetVersion();
private static final String TAG = CronetLibraryLoader.class.getSimpleName(); private static final String TAG = CronetLibraryLoader.class.getSimpleName();
// Thread used for initialization work and processing callbacks for
// long-lived global singletons. This thread lives forever as things like
// the global singleton NetworkChangeNotifier live on it and are never killed.
private static final HandlerThread sInitThread = new HandlerThread("CronetInit");
// Has library loading commenced? Setting guarded by sLoadLock. // Has library loading commenced? Setting guarded by sLoadLock.
private static volatile boolean sLibraryLoaded = false; private static volatile boolean sLibraryLoaded = false;
// Has ensureMainThreadInitialized() completed? Only accessed on main thread. // Has ensureInitThreadInitialized() completed?
private static volatile boolean sMainThreadInitDone = false; private static volatile boolean sInitThreadInitDone = false;
/** /**
* Ensure that native library is loaded and initialized. Can be called from * Ensure that native library is loaded and initialized. Can be called from
* any thread, the load and initialization is performed on main thread. * any thread, the load and initialization is performed on init thread.
*/ */
public static void ensureInitialized( public static void ensureInitialized(
final Context applicationContext, final CronetEngineBuilderImpl builder) { final Context applicationContext, final CronetEngineBuilderImpl builder) {
...@@ -55,35 +60,36 @@ public class CronetLibraryLoader { ...@@ -55,35 +60,36 @@ public class CronetLibraryLoader {
sLibraryLoaded = true; sLibraryLoaded = true;
} }
if (!sMainThreadInitDone) { if (!sInitThreadInitDone) {
// Init native Chromium CronetEngine on Main UI thread. if (!sInitThread.isAlive()) {
Runnable task = new Runnable() { sInitThread.start();
}
postToInitThread(new Runnable() {
@Override @Override
public void run() { public void run() {
ensureInitializedOnMainThread(applicationContext); ensureInitializedOnInitThread(applicationContext);
} }
}; });
// Run task immediately or post it to the UI thread.
if (Looper.getMainLooper() == Looper.myLooper()) {
task.run();
} else {
// The initOnMainThread will complete on the main thread prior
// to other tasks posted to the main thread.
new Handler(Looper.getMainLooper()).post(task);
}
} }
} }
} }
/** /**
* Ensure that the main thread initialization has completed. Can only be called from * Returns {@code true} if running on the initialization thread.
* the main thread. Ensures that the NetworkChangeNotifier is initialzied and the */
* main thread native MessageLoop is initialized. private static boolean onInitThread() {
return sInitThread.getLooper() == Looper.myLooper();
}
/**
* Ensure that the init thread initialization has completed. Can only be called from
* the init thread. Ensures that the NetworkChangeNotifier is initialzied and the
* init thread native MessageLoop is initialized.
*/ */
static void ensureInitializedOnMainThread(Context context) { static void ensureInitializedOnInitThread(Context context) {
assert sLibraryLoaded; assert sLibraryLoaded;
assert Looper.getMainLooper() == Looper.myLooper(); assert onInitThread();
if (sMainThreadInitDone) { if (sInitThreadInitDone) {
return; return;
} }
NetworkChangeNotifier.init(context); NetworkChangeNotifier.init(context);
...@@ -97,11 +103,22 @@ public class CronetLibraryLoader { ...@@ -97,11 +103,22 @@ public class CronetLibraryLoader {
// NetworkChangeNotifierAndroid is created, so as to avoid receiving // NetworkChangeNotifierAndroid is created, so as to avoid receiving
// the undesired initial network change observer notification, which // the undesired initial network change observer notification, which
// will cause active requests to fail with ERR_NETWORK_CHANGED. // will cause active requests to fail with ERR_NETWORK_CHANGED.
nativeCronetInitOnMainThread(); nativeCronetInitOnInitThread();
sMainThreadInitDone = true; sInitThreadInitDone = true;
}
/**
* Run {@code r} on the initialization thread.
*/
static void postToInitThread(Runnable r) {
if (onInitThread()) {
r.run();
} else {
new Handler(sInitThread.getLooper()).post(r);
}
} }
// Native methods are implemented in cronet_library_loader.cc. // Native methods are implemented in cronet_library_loader.cc.
private static native void nativeCronetInitOnMainThread(); private static native void nativeCronetInitOnInitThread();
private static native String nativeGetCronetVersion(); private static native String nativeGetCronetVersion();
} }
...@@ -5,8 +5,6 @@ ...@@ -5,8 +5,6 @@
package org.chromium.net.impl; package org.chromium.net.impl;
import android.os.ConditionVariable; import android.os.ConditionVariable;
import android.os.Handler;
import android.os.Looper;
import android.os.Process; import android.os.Process;
import org.chromium.base.Log; import org.chromium.base.Log;
...@@ -161,25 +159,19 @@ public class CronetUrlRequestContext extends CronetEngineBase { ...@@ -161,25 +159,19 @@ public class CronetUrlRequestContext extends CronetEngineBase {
mNetworkQualityEstimatorEnabled = builder.networkQualityEstimatorEnabled(); mNetworkQualityEstimatorEnabled = builder.networkQualityEstimatorEnabled();
} }
// Init native Chromium URLRequestContext on main UI thread. // Init native Chromium URLRequestContext on init thread.
Runnable task = new Runnable() { CronetLibraryLoader.postToInitThread(new Runnable() {
@Override @Override
public void run() { public void run() {
CronetLibraryLoader.ensureInitializedOnMainThread(builder.getContext()); CronetLibraryLoader.ensureInitializedOnInitThread(builder.getContext());
synchronized (mLock) { synchronized (mLock) {
// mUrlRequestContextAdapter is guaranteed to exist until // mUrlRequestContextAdapter is guaranteed to exist until
// initialization on main and network threads completes and // initialization on init and network threads completes and
// initNetworkThread is called back on network thread. // initNetworkThread is called back on network thread.
nativeInitRequestContextOnMainThread(mUrlRequestContextAdapter); nativeInitRequestContextOnInitThread(mUrlRequestContextAdapter);
} }
} }
}; });
// Run task immediately or post it to the UI thread.
if (Looper.getMainLooper() == Looper.myLooper()) {
task.run();
} else {
new Handler(Looper.getMainLooper()).post(task);
}
} }
@VisibleForTesting @VisibleForTesting
...@@ -251,7 +243,7 @@ public class CronetUrlRequestContext extends CronetEngineBase { ...@@ -251,7 +243,7 @@ public class CronetUrlRequestContext extends CronetEngineBase {
throw new IllegalThreadStateException("Cannot shutdown from network thread."); throw new IllegalThreadStateException("Cannot shutdown from network thread.");
} }
} }
// Wait for init to complete on main and network thread (without lock, // Wait for init to complete on init and network thread (without lock,
// so other thread could access it). // so other thread could access it).
mInitCompleted.block(); mInitCompleted.block();
...@@ -711,7 +703,7 @@ public class CronetUrlRequestContext extends CronetEngineBase { ...@@ -711,7 +703,7 @@ public class CronetUrlRequestContext extends CronetEngineBase {
private native void nativeGetCertVerifierData(long nativePtr); private native void nativeGetCertVerifierData(long nativePtr);
@NativeClassQualifiedName("CronetURLRequestContextAdapter") @NativeClassQualifiedName("CronetURLRequestContextAdapter")
private native void nativeInitRequestContextOnMainThread(long nativePtr); private native void nativeInitRequestContextOnInitThread(long nativePtr);
@NativeClassQualifiedName("CronetURLRequestContextAdapter") @NativeClassQualifiedName("CronetURLRequestContextAdapter")
private native void nativeConfigureNetworkQualityEstimatorForTesting(long nativePtr, private native void nativeConfigureNetworkQualityEstimatorForTesting(long nativePtr,
......
...@@ -2984,6 +2984,7 @@ if (is_android) { ...@@ -2984,6 +2984,7 @@ if (is_android) {
sources = [ sources = [
"android/javatests/src/org/chromium/net/AndroidKeyStoreTestUtil.java", "android/javatests/src/org/chromium/net/AndroidKeyStoreTestUtil.java",
"android/javatests/src/org/chromium/net/AndroidNetworkLibraryTestUtil.java", "android/javatests/src/org/chromium/net/AndroidNetworkLibraryTestUtil.java",
"android/javatests/src/org/chromium/net/AndroidProxyConfigServiceTestUtil.java",
"test/android/javatests/src/org/chromium/net/test/DummySpnegoAuthenticator.java", "test/android/javatests/src/org/chromium/net/test/DummySpnegoAuthenticator.java",
"test/android/javatests/src/org/chromium/net/test/EmbeddedTestServerImpl.java", "test/android/javatests/src/org/chromium/net/test/EmbeddedTestServerImpl.java",
] ]
......
...@@ -115,6 +115,7 @@ android_library("net_javatests") { ...@@ -115,6 +115,7 @@ android_library("net_javatests") {
java_files = [ java_files = [
"javatests/src/org/chromium/net/AndroidKeyStoreTestUtil.java", "javatests/src/org/chromium/net/AndroidKeyStoreTestUtil.java",
"javatests/src/org/chromium/net/AndroidNetworkLibraryTestUtil.java", "javatests/src/org/chromium/net/AndroidNetworkLibraryTestUtil.java",
"javatests/src/org/chromium/net/AndroidProxyConfigServiceTestUtil.java",
"javatests/src/org/chromium/net/AndroidProxySelectorTest.java", "javatests/src/org/chromium/net/AndroidProxySelectorTest.java",
"javatests/src/org/chromium/net/NetErrorsTest.java", "javatests/src/org/chromium/net/NetErrorsTest.java",
"javatests/src/org/chromium/net/NetworkChangeNotifierTest.java", "javatests/src/org/chromium/net/NetworkChangeNotifierTest.java",
......
...@@ -26,12 +26,14 @@ import android.net.NetworkRequest; ...@@ -26,12 +26,14 @@ import android.net.NetworkRequest;
import android.net.wifi.WifiInfo; import android.net.wifi.WifiInfo;
import android.net.wifi.WifiManager; import android.net.wifi.WifiManager;
import android.os.Build; import android.os.Build;
import android.os.Handler;
import android.os.Looper;
import android.telephony.TelephonyManager; import android.telephony.TelephonyManager;
import org.chromium.base.ApplicationState; import org.chromium.base.ApplicationState;
import org.chromium.base.ApplicationStatus; import org.chromium.base.ApplicationStatus;
import org.chromium.base.BuildConfig;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
...@@ -367,7 +369,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -367,7 +369,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
} }
String getWifiSsid() { String getWifiSsid() {
// Synchronized because this method can be called on multiple threads (e.g. UI thread // Synchronized because this method can be called on multiple threads (e.g. mLooper
// from a private caller, and another thread calling a public API like // from a private caller, and another thread calling a public API like
// getCurrentNetworkState) and is otherwise racy. // getCurrentNetworkState) and is otherwise racy.
synchronized (mLock) { synchronized (mLock) {
...@@ -409,7 +411,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -409,7 +411,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
// This class gets called back by ConnectivityManager whenever networks come // This class gets called back by ConnectivityManager whenever networks come
// and go. It gets called back on a special handler thread // and go. It gets called back on a special handler thread
// ConnectivityManager creates for making the callbacks. The callbacks in // ConnectivityManager creates for making the callbacks. The callbacks in
// turn post to the UI thread where mObserver lives. // turn post to mLooper where mObserver lives.
@TargetApi(Build.VERSION_CODES.LOLLIPOP) @TargetApi(Build.VERSION_CODES.LOLLIPOP)
private class MyNetworkCallback extends NetworkCallback { private class MyNetworkCallback extends NetworkCallback {
// If non-null, this indicates a VPN is in place for the current user, and no other // If non-null, this indicates a VPN is in place for the current user, and no other
...@@ -484,7 +486,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -484,7 +486,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
final long netId = networkToNetId(network); final long netId = networkToNetId(network);
@ConnectionType @ConnectionType
final int connectionType = mConnectivityManagerDelegate.getConnectionType(network); final int connectionType = mConnectivityManagerDelegate.getConnectionType(network);
ThreadUtils.postOnUiThread(new Runnable() { runOnThread(new Runnable() {
@Override @Override
public void run() { public void run() {
mObserver.onNetworkConnect(netId, connectionType); mObserver.onNetworkConnect(netId, connectionType);
...@@ -508,7 +510,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -508,7 +510,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
// so forward the new ConnectionType along to observer. // so forward the new ConnectionType along to observer.
final long netId = networkToNetId(network); final long netId = networkToNetId(network);
final int connectionType = mConnectivityManagerDelegate.getConnectionType(network); final int connectionType = mConnectivityManagerDelegate.getConnectionType(network);
ThreadUtils.postOnUiThread(new Runnable() { runOnThread(new Runnable() {
@Override @Override
public void run() { public void run() {
mObserver.onNetworkConnect(netId, connectionType); mObserver.onNetworkConnect(netId, connectionType);
...@@ -522,7 +524,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -522,7 +524,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
return; return;
} }
final long netId = networkToNetId(network); final long netId = networkToNetId(network);
ThreadUtils.postOnUiThread(new Runnable() { runOnThread(new Runnable() {
@Override @Override
public void run() { public void run() {
mObserver.onNetworkSoonToDisconnect(netId); mObserver.onNetworkSoonToDisconnect(netId);
...@@ -535,7 +537,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -535,7 +537,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
if (ignoreNetworkDueToVpn(network)) { if (ignoreNetworkDueToVpn(network)) {
return; return;
} }
ThreadUtils.postOnUiThread(new Runnable() { runOnThread(new Runnable() {
@Override @Override
public void run() { public void run() {
mObserver.onNetworkDisconnect(networkToNetId(network)); mObserver.onNetworkDisconnect(networkToNetId(network));
...@@ -553,7 +555,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -553,7 +555,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
} }
@ConnectionType @ConnectionType
final int newConnectionType = convertToConnectionType(getCurrentNetworkState()); final int newConnectionType = convertToConnectionType(getCurrentNetworkState());
ThreadUtils.postOnUiThread(new Runnable() { runOnThread(new Runnable() {
@Override @Override
public void run() { public void run() {
mObserver.onConnectionTypeChanged(newConnectionType); mObserver.onConnectionTypeChanged(newConnectionType);
...@@ -597,10 +599,16 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -597,10 +599,16 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
protected abstract void destroy(); protected abstract void destroy();
} }
private static final String TAG = "NetworkChangeNotifierAutoDetect"; private static final String TAG = NetworkChangeNotifierAutoDetect.class.getSimpleName();
private static final int UNKNOWN_LINK_SPEED = -1; private static final int UNKNOWN_LINK_SPEED = -1;
// {@link Looper} for the thread this object lives on.
private final Looper mLooper;
// Used to post to the thread this object lives on.
private final Handler mHandler;
// {@link IntentFilter} for incoming global broadcast {@link Intent}s this object listens for.
private final NetworkConnectivityIntentFilter mIntentFilter; private final NetworkConnectivityIntentFilter mIntentFilter;
// Notifications are sent to this {@link Observer}.
private final Observer mObserver; private final Observer mObserver;
private final RegistrationPolicy mRegistrationPolicy; private final RegistrationPolicy mRegistrationPolicy;
...@@ -675,16 +683,17 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -675,16 +683,17 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
} }
/** /**
* Constructs a NetworkChangeNotifierAutoDetect. Should only be called on UI thread. * Constructs a NetworkChangeNotifierAutoDetect. Lives on calling thread, receives broadcast
* notifications on the UI thread and forwards the notifications to be processed on the calling
* thread.
* @param policy The RegistrationPolicy which determines when this class should watch * @param policy The RegistrationPolicy which determines when this class should watch
* for network changes (e.g. see (@link RegistrationPolicyAlwaysRegister} and * for network changes (e.g. see (@link RegistrationPolicyAlwaysRegister} and
* {@link RegistrationPolicyApplicationStatus}). * {@link RegistrationPolicyApplicationStatus}).
*/ */
@TargetApi(Build.VERSION_CODES.LOLLIPOP) @TargetApi(Build.VERSION_CODES.LOLLIPOP)
public NetworkChangeNotifierAutoDetect(Observer observer, RegistrationPolicy policy) { public NetworkChangeNotifierAutoDetect(Observer observer, RegistrationPolicy policy) {
// Since BroadcastReceiver is always called back on UI thread, ensure mLooper = Looper.myLooper();
// running on UI thread so notification logic can be single-threaded. mHandler = new Handler(mLooper);
ThreadUtils.assertOnUiThread();
mObserver = observer; mObserver = observer;
mConnectivityManagerDelegate = mConnectivityManagerDelegate =
new ConnectivityManagerDelegate(ContextUtils.getApplicationContext()); new ConnectivityManagerDelegate(ContextUtils.getApplicationContext());
...@@ -713,6 +722,25 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -713,6 +722,25 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
mShouldSignalObserver = true; mShouldSignalObserver = true;
} }
private boolean onThread() {
return mLooper == Looper.myLooper();
}
private void assertOnThread() {
if (BuildConfig.DCHECK_IS_ON && !onThread()) {
throw new IllegalStateException(
"Must be called on NetworkChangeNotifierAutoDetect thread.");
}
}
private void runOnThread(Runnable r) {
if (onThread()) {
r.run();
} else {
mHandler.post(r);
}
}
/** /**
* Allows overriding the ConnectivityManagerDelegate for tests. * Allows overriding the ConnectivityManagerDelegate for tests.
*/ */
...@@ -741,6 +769,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -741,6 +769,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
} }
public void destroy() { public void destroy() {
assertOnThread();
mRegistrationPolicy.destroy(); mRegistrationPolicy.destroy();
unregister(); unregister();
} }
...@@ -749,7 +778,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -749,7 +778,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
* Registers a BroadcastReceiver in the given context. * Registers a BroadcastReceiver in the given context.
*/ */
public void register() { public void register() {
ThreadUtils.assertOnUiThread(); assertOnThread();
if (mRegistered) return; if (mRegistered) return;
if (mShouldSignalObserver) { if (mShouldSignalObserver) {
...@@ -790,6 +819,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -790,6 +819,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
* Unregisters a BroadcastReceiver in the given context. * Unregisters a BroadcastReceiver in the given context.
*/ */
public void unregister() { public void unregister() {
assertOnThread();
if (!mRegistered) return; if (!mRegistered) return;
ContextUtils.getApplicationContext().unregisterReceiver(this); ContextUtils.getApplicationContext().unregisterReceiver(this);
mRegistered = false; mRegistered = false;
...@@ -997,15 +1027,23 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -997,15 +1027,23 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
// BroadcastReceiver // BroadcastReceiver
@Override @Override
public void onReceive(Context context, Intent intent) { public void onReceive(Context context, Intent intent) {
if (mIgnoreNextBroadcast) { runOnThread(new Runnable() {
mIgnoreNextBroadcast = false; @Override
return; public void run() {
} // Once execution begins on the correct thread, make sure unregister() hasn't
final NetworkState networkState = getCurrentNetworkState(); // been called in the mean time. Ignore the broadcast if unregister() was called.
if (ConnectivityManager.CONNECTIVITY_ACTION.equals(intent.getAction())) { if (!mRegistered) {
connectionTypeChanged(networkState); return;
maxBandwidthChanged(networkState); }
} if (mIgnoreNextBroadcast) {
mIgnoreNextBroadcast = false;
return;
}
final NetworkState networkState = getCurrentNetworkState();
connectionTypeChanged(networkState);
maxBandwidthChanged(networkState);
}
});
} }
private void connectionTypeChanged(NetworkState networkState) { private void connectionTypeChanged(NetworkState networkState) {
......
...@@ -11,9 +11,12 @@ import android.content.IntentFilter; ...@@ -11,9 +11,12 @@ import android.content.IntentFilter;
import android.net.Proxy; import android.net.Proxy;
import android.net.Uri; import android.net.Uri;
import android.os.Build; import android.os.Build;
import android.os.Handler;
import android.os.Looper;
import android.text.TextUtils; import android.text.TextUtils;
import android.util.Log; import android.util.Log;
import org.chromium.base.BuildConfig;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
...@@ -31,6 +34,9 @@ public class ProxyChangeListener { ...@@ -31,6 +34,9 @@ public class ProxyChangeListener {
private static final String TAG = "ProxyChangeListener"; private static final String TAG = "ProxyChangeListener";
private static boolean sEnabled = true; private static boolean sEnabled = true;
private final Looper mLooper;
private final Handler mHandler;
private long mNativePtr; private long mNativePtr;
private ProxyReceiver mProxyReceiver; private ProxyReceiver mProxyReceiver;
private Delegate mDelegate; private Delegate mDelegate;
...@@ -55,7 +61,10 @@ public class ProxyChangeListener { ...@@ -55,7 +61,10 @@ public class ProxyChangeListener {
public void proxySettingsChanged(); public void proxySettingsChanged();
} }
private ProxyChangeListener() {} private ProxyChangeListener() {
mLooper = Looper.myLooper();
mHandler = new Handler(mLooper);
}
public static void setEnabled(boolean enabled) { public static void setEnabled(boolean enabled) {
sEnabled = enabled; sEnabled = enabled;
...@@ -77,6 +86,7 @@ public class ProxyChangeListener { ...@@ -77,6 +86,7 @@ public class ProxyChangeListener {
@CalledByNative @CalledByNative
public void start(long nativePtr) { public void start(long nativePtr) {
assertOnThread();
assert mNativePtr == 0; assert mNativePtr == 0;
mNativePtr = nativePtr; mNativePtr = nativePtr;
registerReceiver(); registerReceiver();
...@@ -84,15 +94,21 @@ public class ProxyChangeListener { ...@@ -84,15 +94,21 @@ public class ProxyChangeListener {
@CalledByNative @CalledByNative
public void stop() { public void stop() {
assertOnThread();
mNativePtr = 0; mNativePtr = 0;
unregisterReceiver(); unregisterReceiver();
} }
private class ProxyReceiver extends BroadcastReceiver { private class ProxyReceiver extends BroadcastReceiver {
@Override @Override
public void onReceive(Context context, Intent intent) { public void onReceive(Context context, final Intent intent) {
if (intent.getAction().equals(Proxy.PROXY_CHANGE_ACTION)) { if (intent.getAction().equals(Proxy.PROXY_CHANGE_ACTION)) {
proxySettingsChanged(extractNewProxy(intent)); runOnThread(new Runnable() {
@Override
public void run() {
proxySettingsChanged(ProxyReceiver.this, extractNewProxy(intent));
}
});
} }
} }
...@@ -173,8 +189,12 @@ public class ProxyChangeListener { ...@@ -173,8 +189,12 @@ public class ProxyChangeListener {
} }
} }
private void proxySettingsChanged(ProxyConfig cfg) { private void proxySettingsChanged(ProxyReceiver proxyReceiver, ProxyConfig cfg) {
if (!sEnabled) { if (!sEnabled
// Once execution begins on the correct thread, make sure unregisterReceiver()
// hasn't been called in the mean time. Ignore the changed signal if
// unregisterReceiver() was called.
|| proxyReceiver != mProxyReceiver) {
return; return;
} }
if (mDelegate != null) { if (mDelegate != null) {
...@@ -211,6 +231,24 @@ public class ProxyChangeListener { ...@@ -211,6 +231,24 @@ public class ProxyChangeListener {
mProxyReceiver = null; mProxyReceiver = null;
} }
private boolean onThread() {
return mLooper == Looper.myLooper();
}
private void assertOnThread() {
if (BuildConfig.DCHECK_IS_ON && !onThread()) {
throw new IllegalStateException("Must be called on ProxyChangeListener thread.");
}
}
private void runOnThread(Runnable r) {
if (onThread()) {
r.run();
} else {
mHandler.post(r);
}
}
/** /**
* See net/proxy/proxy_config_service_android.cc * See net/proxy/proxy_config_service_android.cc
*/ */
......
// Copyright 2017 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.os.Looper;
import org.chromium.base.annotations.CalledByNative;
/**
* Utility functions for testing features implemented in ProxyConfigServiceAndroid.
*/
public class AndroidProxyConfigServiceTestUtil {
/**
* Helper for tests that prepares the Looper on the current thread.
*/
@CalledByNative
private static void prepareLooper() {
Looper.prepare();
}
}
\ No newline at end of file
...@@ -592,6 +592,7 @@ public class NetworkChangeNotifierTest { ...@@ -592,6 +592,7 @@ public class NetworkChangeNotifierTest {
@MediumTest @MediumTest
@Feature({"Android-AppBase"}) @Feature({"Android-AppBase"})
public void testNetworkChangeNotifierJavaObservers() { public void testNetworkChangeNotifierJavaObservers() {
mReceiver.register();
// Initialize the NetworkChangeNotifier with a connection. // Initialize the NetworkChangeNotifier with a connection.
Intent connectivityIntent = new Intent(ConnectivityManager.CONNECTIVITY_ACTION); Intent connectivityIntent = new Intent(ConnectivityManager.CONNECTIVITY_ACTION);
mReceiver.onReceive(InstrumentationRegistry.getInstrumentation().getTargetContext(), mReceiver.onReceive(InstrumentationRegistry.getInstrumentation().getTargetContext(),
...@@ -657,6 +658,7 @@ public class NetworkChangeNotifierTest { ...@@ -657,6 +658,7 @@ public class NetworkChangeNotifierTest {
@MediumTest @MediumTest
@Feature({"Android-AppBase"}) @Feature({"Android-AppBase"})
public void testNetworkChangeNotifierMaxBandwidthNotifications() { public void testNetworkChangeNotifierMaxBandwidthNotifications() {
mReceiver.register();
// Initialize the NetworkChangeNotifier with a connection. // Initialize the NetworkChangeNotifier with a connection.
mConnectivityDelegate.setActiveNetworkExists(true); mConnectivityDelegate.setActiveNetworkExists(true);
mConnectivityDelegate.setNetworkType(ConnectivityManager.TYPE_WIFI); mConnectivityDelegate.setNetworkType(ConnectivityManager.TYPE_WIFI);
...@@ -953,6 +955,7 @@ public class NetworkChangeNotifierTest { ...@@ -953,6 +955,7 @@ public class NetworkChangeNotifierTest {
@MediumTest @MediumTest
@Feature({"Android-AppBase"}) @Feature({"Android-AppBase"})
public void testNetworkChangeNotifierIsOnline() { public void testNetworkChangeNotifierIsOnline() {
mReceiver.register();
Intent intent = new Intent(ConnectivityManager.CONNECTIVITY_ACTION); Intent intent = new Intent(ConnectivityManager.CONNECTIVITY_ACTION);
// For any connection type it should return true. // For any connection type it should return true.
for (int i = ConnectivityManager.TYPE_MOBILE; i < ConnectivityManager.TYPE_VPN; i++) { for (int i = ConnectivityManager.TYPE_MOBILE; i < ConnectivityManager.TYPE_VPN; i++) {
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "jni/AndroidProxyConfigServiceTestUtil_jni.h"
#include "net/proxy/proxy_config.h" #include "net/proxy/proxy_config.h"
#include "net/proxy/proxy_info.h" #include "net/proxy/proxy_info.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -45,6 +46,15 @@ class TestObserver : public ProxyConfigService::Observer { ...@@ -45,6 +46,15 @@ class TestObserver : public ProxyConfigService::Observer {
ProxyConfigService::ConfigAvailability availability_; ProxyConfigService::ConfigAvailability availability_;
}; };
// Helper class that simply prepares Java's Looper on construction.
class JavaLooperPreparer {
public:
JavaLooperPreparer() {
Java_AndroidProxyConfigServiceTestUtil_prepareLooper(
base::android::AttachCurrentThread());
}
};
} // namespace } // namespace
typedef std::map<std::string, std::string> StringMap; typedef std::map<std::string, std::string> StringMap;
...@@ -104,6 +114,10 @@ class ProxyConfigServiceAndroidTestBase : public testing::Test { ...@@ -104,6 +114,10 @@ class ProxyConfigServiceAndroidTestBase : public testing::Test {
StringMap configuration_; StringMap configuration_;
TestObserver observer_; TestObserver observer_;
base::MessageLoop* const message_loop_; base::MessageLoop* const message_loop_;
// |java_looper_preparer_| appears before |service_| so that Java's Looper is
// prepared before constructing |service_| as it creates a ProxyChangeListener
// which requires a Looper.
JavaLooperPreparer java_looper_preparer_;
ProxyConfigServiceAndroid service_; ProxyConfigServiceAndroid service_;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment