diff --git a/components/cronet/android/BUILD.gn b/components/cronet/android/BUILD.gn index b3b539182c3ba..0cd47bdd8f66d 100644 --- a/components/cronet/android/BUILD.gn +++ b/components/cronet/android/BUILD.gn @@ -349,6 +349,7 @@ android_library("cronet_impl_native_java") { "java/src/org/chromium/net/impl/CronetUrlRequest.java", "java/src/org/chromium/net/impl/CronetUrlRequestContext.java", "java/src/org/chromium/net/impl/NativeCronetEngineBuilderImpl.java", + "java/src/org/chromium/net/impl/NativeCronetEngineBuilderWithLibraryLoaderImpl.java", "java/src/org/chromium/net/impl/NativeCronetProvider.java", "java/src/org/chromium/net/urlconnection/CronetBufferedOutputStream.java", "java/src/org/chromium/net/urlconnection/CronetChunkedOutputStream.java", diff --git a/components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java b/components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java index e69573134ef16..f564937275e69 100644 --- a/components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java +++ b/components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java @@ -83,7 +83,6 @@ public static class Pkp { private boolean mPublicKeyPinningBypassForLocalTrustAnchorsEnabled; private String mUserAgent; private String mStoragePath; - private VersionSafeCallbacks.LibraryLoader mLibraryLoader; private boolean mQuicEnabled; private boolean mHttp2Enabled; private boolean mSdchEnabled; @@ -142,12 +141,20 @@ String storagePath() { @Override public CronetEngineBuilderImpl setLibraryLoader(CronetEngine.Builder.LibraryLoader loader) { - mLibraryLoader = new VersionSafeCallbacks.LibraryLoader(loader); + // |CronetEngineBuilderImpl| is an abstract class that is used by concrete builder + // implementations, including the Java Cronet engine builder; therefore, the implementation + // of this method should be "no-op". Subclasses that care about the library loader + // should override this method. return this; } + /** + * Default implementation of the method that returns {@code null}. + * + * @return {@code null}. + */ VersionSafeCallbacks.LibraryLoader libraryLoader() { - return mLibraryLoader; + return null; } @Override @@ -402,7 +409,7 @@ public CronetEngineBuilderImpl setThreadPriority(int priority) { } /** - * @returns thread priority provided by user, or {@code defaultThreadPriority} if none provided. + * @return thread priority provided by user, or {@code defaultThreadPriority} if none provided. */ int threadPriority(int defaultThreadPriority) { return mThreadPriority == INVALID_THREAD_PRIORITY ? defaultThreadPriority : mThreadPriority; diff --git a/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetEngineBuilderWithLibraryLoaderImpl.java b/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetEngineBuilderWithLibraryLoaderImpl.java new file mode 100644 index 0000000000000..8bc0371067729 --- /dev/null +++ b/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetEngineBuilderWithLibraryLoaderImpl.java @@ -0,0 +1,39 @@ +// 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.impl; + +import android.content.Context; + +import org.chromium.net.CronetEngine.Builder.LibraryLoader; +import org.chromium.net.ICronetEngineBuilder; + +/** + * An extension of {@link NativeCronetEngineBuilderImpl} that implements + * {@link ICronetEngineBuilder#setLibraryLoader}. + */ +public class NativeCronetEngineBuilderWithLibraryLoaderImpl extends NativeCronetEngineBuilderImpl { + private VersionSafeCallbacks.LibraryLoader mLibraryLoader; + + /** + * Constructs a builder for Native Cronet Engine. + * Default config enables SPDY, disables QUIC, SDCH and HTTP cache. + * + * @param context Android {@link Context} for engine to use. + */ + public NativeCronetEngineBuilderWithLibraryLoaderImpl(Context context) { + super(context); + } + + @Override + public CronetEngineBuilderImpl setLibraryLoader(LibraryLoader loader) { + mLibraryLoader = new VersionSafeCallbacks.LibraryLoader(loader); + return this; + } + + @Override + VersionSafeCallbacks.LibraryLoader libraryLoader() { + return mLibraryLoader; + } +} diff --git a/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetProvider.java b/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetProvider.java index e659d2bbdd8b8..3ff896776e154 100644 --- a/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetProvider.java +++ b/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetProvider.java @@ -29,7 +29,7 @@ public NativeCronetProvider(Context context) { @Override public CronetEngine.Builder createBuilder() { - ICronetEngineBuilder impl = new NativeCronetEngineBuilderImpl(mContext); + ICronetEngineBuilder impl = new NativeCronetEngineBuilderWithLibraryLoaderImpl(mContext); return new ExperimentalCronetEngine.Builder(impl); } diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java index 1c1304dccac34..6bdc638848641 100644 --- a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java +++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java @@ -40,8 +40,8 @@ import org.chromium.net.CronetTestRule.RequiresMinApi; import org.chromium.net.TestUrlRequestCallback.ResponseStep; import org.chromium.net.impl.CronetEngineBuilderImpl; -import org.chromium.net.impl.CronetLibraryLoader; import org.chromium.net.impl.CronetUrlRequestContext; +import org.chromium.net.impl.NativeCronetEngineBuilderImpl; import org.chromium.net.test.EmbeddedTestServer; import java.io.BufferedReader; @@ -1288,21 +1288,32 @@ boolean wasCalled() { @SmallTest @Feature({"Cronet"}) @OnlyRunNativeCronet - public void testSkipLibraryLoading() throws Exception { + public void testSetLibraryLoaderIsEnforcedByDefaultEmbeddedProvider() throws Exception { CronetEngine.Builder builder = new CronetEngine.Builder(getContext()); TestBadLibraryLoader loader = new TestBadLibraryLoader(); builder.setLibraryLoader(loader); try { - // ensureInitialized() calls native code to check the version right after library load - // and will error with the message below if library loading was skipped - CronetLibraryLoader.ensureInitialized(getContext().getApplicationContext(), - (CronetEngineBuilderImpl) builder.mBuilderDelegate); + builder.build(); fail("Native library should not be loaded"); } catch (UnsatisfiedLinkError e) { assertTrue(loader.wasCalled()); } } + @Test + @SmallTest + @Feature({"Cronet"}) + @OnlyRunNativeCronet + public void testSetLibraryLoaderIsIgnoredInNativeCronetEngineBuilderImpl() throws Exception { + CronetEngine.Builder builder = + new CronetEngine.Builder(new NativeCronetEngineBuilderImpl(getContext())); + TestBadLibraryLoader loader = new TestBadLibraryLoader(); + builder.setLibraryLoader(loader); + CronetEngine engine = builder.build(); + assertNotNull(engine); + assertFalse(loader.wasCalled()); + } + // Creates a CronetEngine on another thread and then one on the main thread. This shouldn't // crash. @Test