Commit 6c10657e authored by pauljensen's avatar pauljensen Committed by Commit bot

[Cronet] Make sure to only pass the application Context to...

[Cronet] Make sure to only pass the application Context to InitApplicationContext to avoid DCHECK failures.

An application may have multiple Contexts but only one application Context.  InitApplicationContext DCHECKs if passed more than one Context, so make sure to always pass it only the application Context.
Also, clean up other uses of Contexts in Cronet to only use the application Context when necessary.  Unnecessarily using the application Context was hiding the bug this CL fixes.  I'm renaming variables to make it clear where an application context is needed versus where any Context is okay.  It's good practice to use the Context explicitly passed in as the wrapping it provides may add additional functionality on top of wrapped Contexts and the application Context.
Also, add a test that Cronet can be simultaneously instantiated upon a Context, a wrapped version of the Context and the application Context to verify Cronet doesn't DCHECK.

BUG=453845

Review URL: https://codereview.chromium.org/981013002

Cr-Commit-Position: refs/heads/master@{#319622}
parent bfc7eb20
...@@ -63,7 +63,7 @@ bool ChromiumUrlRequestContextRegisterJni(JNIEnv* env) { ...@@ -63,7 +63,7 @@ bool ChromiumUrlRequestContextRegisterJni(JNIEnv* env) {
// Sets global user-agent to be used for all subsequent requests. // Sets global user-agent to be used for all subsequent requests.
static jlong CreateRequestContextAdapter(JNIEnv* env, static jlong CreateRequestContextAdapter(JNIEnv* env,
jobject jcaller, jobject jcaller,
jobject jcontext, jobject japp_context,
jstring juser_agent, jstring juser_agent,
jint jlog_level, jint jlog_level,
jstring jconfig) { jstring jconfig) {
...@@ -86,8 +86,9 @@ static jlong CreateRequestContextAdapter(JNIEnv* env, ...@@ -86,8 +86,9 @@ static jlong CreateRequestContextAdapter(JNIEnv* env,
} }
// Set application context. // Set application context.
base::android::ScopedJavaLocalRef<jobject> scoped_context(env, jcontext); base::android::ScopedJavaLocalRef<jobject> scoped_app_context(env,
base::android::InitApplicationContext(env, scoped_context); japp_context);
base::android::InitApplicationContext(env, scoped_app_context);
// TODO(mef): MinLogLevel is global, shared by all URLRequestContexts. // TODO(mef): MinLogLevel is global, shared by all URLRequestContexts.
// Revisit this if each URLRequestContext would need an individual log level. // Revisit this if each URLRequestContext would need an individual log level.
......
...@@ -87,10 +87,11 @@ void CronetOnUnLoad(JavaVM* jvm, void* reserved) { ...@@ -87,10 +87,11 @@ void CronetOnUnLoad(JavaVM* jvm, void* reserved) {
} }
} }
void CronetInitOnMainThread(JNIEnv* env, jclass jcaller, jobject jcontext) { void CronetInitOnMainThread(JNIEnv* env, jclass jcaller, jobject japp_context) {
// Set application context. // Set application context.
base::android::ScopedJavaLocalRef<jobject> scoped_context(env, jcontext); base::android::ScopedJavaLocalRef<jobject> scoped_app_context(env,
base::android::InitApplicationContext(env, scoped_context); japp_context);
base::android::InitApplicationContext(env, scoped_app_context);
#if !defined(USE_ICU_ALTERNATIVES_ON_ANDROID) #if !defined(USE_ICU_ALTERNATIVES_ON_ANDROID)
base::i18n::InitializeICU(); base::i18n::InitializeICU();
......
...@@ -34,7 +34,7 @@ public class ChromiumUrlRequestContext { ...@@ -34,7 +34,7 @@ public class ChromiumUrlRequestContext {
protected ChromiumUrlRequestContext(final Context context, String userAgent, protected ChromiumUrlRequestContext(final Context context, String userAgent,
String config) { String config) {
mChromiumUrlRequestContextAdapter = nativeCreateRequestContextAdapter( mChromiumUrlRequestContextAdapter = nativeCreateRequestContextAdapter(
context, userAgent, getLoggingLevel(), config); context.getApplicationContext(), userAgent, getLoggingLevel(), config);
if (mChromiumUrlRequestContextAdapter == 0) { if (mChromiumUrlRequestContextAdapter == 0) {
throw new NullPointerException("Context Adapter creation failed"); throw new NullPointerException("Context Adapter creation failed");
} }
...@@ -134,8 +134,8 @@ public class ChromiumUrlRequestContext { ...@@ -134,8 +134,8 @@ public class ChromiumUrlRequestContext {
// Returns an instance ChromiumUrlRequestContextAdapter to be stored in // Returns an instance ChromiumUrlRequestContextAdapter to be stored in
// mChromiumUrlRequestContextAdapter. // mChromiumUrlRequestContextAdapter.
private native long nativeCreateRequestContextAdapter(Context context, private native long nativeCreateRequestContextAdapter(
String userAgent, int loggingLevel, String config); Context appContext, String userAgent, int loggingLevel, String config);
private native void nativeReleaseRequestContextAdapter( private native void nativeReleaseRequestContextAdapter(
long chromiumUrlRequestContextAdapter); long chromiumUrlRequestContextAdapter);
......
...@@ -25,8 +25,7 @@ public class ChromiumUrlRequestFactory extends HttpUrlRequestFactory { ...@@ -25,8 +25,7 @@ public class ChromiumUrlRequestFactory extends HttpUrlRequestFactory {
if (isEnabled()) { if (isEnabled()) {
CronetLibraryLoader.ensureInitialized(context, config); CronetLibraryLoader.ensureInitialized(context, config);
mRequestContext = new ChromiumUrlRequestContext( mRequestContext = new ChromiumUrlRequestContext(
context.getApplicationContext(), UserAgent.from(context), context, UserAgent.from(context), config.toString());
config.toString());
} }
} }
......
...@@ -62,9 +62,9 @@ class CronetLibraryLoader { ...@@ -62,9 +62,9 @@ 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(context); nativeCronetInitOnMainThread(context.getApplicationContext());
} }
// Native methods are implemented in cronet_loader.cc. // Native methods are implemented in cronet_loader.cc.
private static native void nativeCronetInitOnMainThread(Context context); private static native void nativeCronetInitOnMainThread(Context appContext);
} }
...@@ -44,7 +44,7 @@ public class CronetUrlRequestContext extends UrlRequestContext { ...@@ -44,7 +44,7 @@ public class CronetUrlRequestContext extends UrlRequestContext {
CronetLibraryLoader.ensureInitialized(context, config); CronetLibraryLoader.ensureInitialized(context, config);
nativeSetMinLogLevel(getLoggingLevel()); nativeSetMinLogLevel(getLoggingLevel());
mUrlRequestContextAdapter = nativeCreateRequestContextAdapter( mUrlRequestContextAdapter = nativeCreateRequestContextAdapter(
context, config.toString()); context.getApplicationContext(), config.toString());
if (mUrlRequestContextAdapter == 0) { if (mUrlRequestContextAdapter == 0) {
throw new NullPointerException("Context Adapter creation failed."); throw new NullPointerException("Context Adapter creation failed.");
} }
...@@ -195,8 +195,7 @@ public class CronetUrlRequestContext extends UrlRequestContext { ...@@ -195,8 +195,7 @@ public class CronetUrlRequestContext extends UrlRequestContext {
} }
// Native methods are implemented in cronet_url_request_context.cc. // Native methods are implemented in cronet_url_request_context.cc.
private static native long nativeCreateRequestContextAdapter( private static native long nativeCreateRequestContextAdapter(Context appContext, String config);
Context context, String config);
private static native int nativeSetMinLogLevel(int loggingLevel); private static native int nativeSetMinLogLevel(int loggingLevel);
......
...@@ -20,7 +20,7 @@ class HttpUrlConnectionUrlRequestFactory extends HttpUrlRequestFactory { ...@@ -20,7 +20,7 @@ class HttpUrlConnectionUrlRequestFactory extends HttpUrlRequestFactory {
public HttpUrlConnectionUrlRequestFactory( public HttpUrlConnectionUrlRequestFactory(
Context context, UrlRequestContextConfig config) { Context context, UrlRequestContextConfig config) {
mContext = context.getApplicationContext(); mContext = context;
} }
@Override @Override
......
...@@ -102,8 +102,7 @@ public class CronetSampleActivity extends Activity { ...@@ -102,8 +102,7 @@ public class CronetSampleActivity extends Activity {
} }
} }
mRequestFactory = HttpUrlRequestFactory.createFactory( mRequestFactory = HttpUrlRequestFactory.createFactory(this, config);
getApplicationContext(), config);
String appUrl = getUrlFromIntent(getIntent()); String appUrl = getUrlFromIntent(getIntent());
if (appUrl == null) { if (appUrl == null) {
...@@ -192,8 +191,7 @@ public class CronetSampleActivity extends Activity { ...@@ -192,8 +191,7 @@ public class CronetSampleActivity extends Activity {
public void startNetLog() { public void startNetLog() {
mRequestFactory.startNetLogToFile( mRequestFactory.startNetLogToFile(
Environment.getExternalStorageDirectory().getPath() + Environment.getExternalStorageDirectory().getPath() + "/cronet_sample_netlog.json");
"/cronet_sample_netlog.json");
} }
public void stopNetLog() { public void stopNetLog() {
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
package org.chromium.net; package org.chromium.net;
import android.content.ContextWrapper;
import android.test.suitebuilder.annotation.SmallTest; import android.test.suitebuilder.annotation.SmallTest;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
...@@ -128,4 +129,24 @@ public class ContextInitTest extends CronetTestBase { ...@@ -128,4 +129,24 @@ public class ContextInitTest extends CronetTestBase {
request.start(); request.start();
return listener; return listener;
} }
@SmallTest
@Feature({"Cronet"})
public void testInitDifferentContexts() throws Exception {
// Test that concurrently instantiating ChromiumUrlRequestContext's upon
// various different versions of the same Android Context does not cause
// crashes like crbug.com/453845
final CronetTestActivity activity = launchCronetTestApp();
HttpUrlRequestFactory firstFactory =
HttpUrlRequestFactory.createFactory(activity, activity.getContextConfig());
HttpUrlRequestFactory secondFactory = HttpUrlRequestFactory.createFactory(
activity.getApplicationContext(), activity.getContextConfig());
HttpUrlRequestFactory thirdFactory = HttpUrlRequestFactory.createFactory(
new ContextWrapper(activity), activity.getContextConfig());
// Meager attempt to extend lifetimes to ensure they're concurrently
// alive.
firstFactory.getName();
secondFactory.getName();
thirdFactory.getName();
}
} }
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
package org.chromium.net; package org.chromium.net;
import android.content.ContextWrapper;
import android.os.ConditionVariable; import android.os.ConditionVariable;
import android.os.Handler; import android.os.Handler;
import android.os.Looper; import android.os.Looper;
...@@ -518,8 +519,7 @@ public class CronetUrlRequestContextTest extends CronetTestBase { ...@@ -518,8 +519,7 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
// Shutdown original context and create another that uses the same cache. // Shutdown original context and create another that uses the same cache.
mActivity.mUrlRequestContext.shutdown(); mActivity.mUrlRequestContext.shutdown();
mActivity.mUrlRequestContext = mActivity.mUrlRequestContext.createContext( mActivity.mUrlRequestContext = mActivity.mUrlRequestContext.createContext(
getInstrumentation().getTargetContext().getApplicationContext(), getInstrumentation().getTargetContext(), config);
config);
checkRequestCaching(url, true); checkRequestCaching(url, true);
} }
...@@ -594,4 +594,22 @@ public class CronetUrlRequestContextTest extends CronetTestBase { ...@@ -594,4 +594,22 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
assertEquals(200, thread1.mListener.mResponseInfo.getHttpStatusCode()); assertEquals(200, thread1.mListener.mResponseInfo.getHttpStatusCode());
assertEquals(404, thread2.mListener.mResponseInfo.getHttpStatusCode()); assertEquals(404, thread2.mListener.mResponseInfo.getHttpStatusCode());
} }
@SmallTest
@Feature({"Cronet"})
public void testInitDifferentContexts() throws Exception {
// Test that concurrently instantiating Cronet context's upon various
// different versions of the same Android Context does not cause crashes
// like crbug.com/453845
mActivity = launchCronetTestApp();
CronetUrlRequestContext firstContext =
new CronetUrlRequestContext(mActivity, mActivity.getContextConfig());
CronetUrlRequestContext secondContext = new CronetUrlRequestContext(
mActivity.getApplicationContext(), mActivity.getContextConfig());
CronetUrlRequestContext thirdContext = new CronetUrlRequestContext(
new ContextWrapper(mActivity), mActivity.getContextConfig());
firstContext.shutdown();
secondContext.shutdown();
thirdContext.shutdown();
}
} }
...@@ -75,8 +75,7 @@ public class CronetTestActivity extends Activity { ...@@ -75,8 +75,7 @@ public class CronetTestActivity extends Activity {
} }
mUrlRequestContext = initRequestContext(); mUrlRequestContext = initRequestContext();
mStreamHandlerFactory = new CronetURLStreamHandlerFactory( mStreamHandlerFactory = new CronetURLStreamHandlerFactory(this, getContextConfig());
getApplicationContext(), getContextConfig());
mHistogramManager = HistogramManager.createHistogramManager(); mHistogramManager = HistogramManager.createHistogramManager();
if (LIBRARY_INIT_CRONET_ONLY.equals(initString)) { if (LIBRARY_INIT_CRONET_ONLY.equals(initString)) {
...@@ -117,12 +116,12 @@ public class CronetTestActivity extends Activity { ...@@ -117,12 +116,12 @@ public class CronetTestActivity extends Activity {
// Helper function to initialize request context. Also used in testing. // Helper function to initialize request context. Also used in testing.
public UrlRequestContext initRequestContext() { public UrlRequestContext initRequestContext() {
return UrlRequestContext.createContext(getApplicationContext(), getContextConfig()); return UrlRequestContext.createContext(this, getContextConfig());
} }
// Helper function to initialize request factory. Also used in testing. // Helper function to initialize request factory. Also used in testing.
public HttpUrlRequestFactory initRequestFactory() { public HttpUrlRequestFactory initRequestFactory() {
return HttpUrlRequestFactory.createFactory(getApplicationContext(), getContextConfig()); return HttpUrlRequestFactory.createFactory(this, getContextConfig());
} }
private static String getUrlFromIntent(Intent intent) { private static String getUrlFromIntent(Intent intent) {
......
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