[Merge M62] CronetEngine.Builder.setLibraryLoader not safe

Do not enforce ICronetEngineBuilder.setLibraryLoader() in Java and
Native builder implementations by default. Instead introduce a new
NativeCronetEngineBuilderWithLibraryLoaderImpl class that enforces
the library loader.

BUG=766248

(cherry picked from commit a9edc7a)

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ie78383084cd68b31f4a87463763891bd30368d21
Reviewed-on: https://chromium-review.googlesource.com/671416
Reviewed-by: Paul Jensen <[email protected]>
Commit-Queue: Paul Jensen <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#502749}
Reviewed-on: https://chromium-review.googlesource.com/676168
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#363}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}

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",

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;

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;
+    }
+}

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);
     }
 

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