[Cronet] Add callback wrapper classes to enforce API version checking.

components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java

 // Copyright 2016 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 android.support.annotation.IntDef;
 import android.support.annotation.VisibleForTesting;
 import android.util.Log;
 
@@ skipping 69 matching lines @@
 
     // Private fields are simply storage of configuration for the resulting CronetEngine.
     // See setters below for verbose descriptions.
     private final Context mApplicationContext;
     private final List<QuicHint> mQuicHints = new LinkedList<>();
     private final List<Pkp> mPkps = new LinkedList<>();
     private boolean mPublicKeyPinningBypassForLocalTrustAnchorsEnabled;
     private String mUserAgent;
     private String mStoragePath;
     private boolean mLegacyModeEnabled;
-    private CronetEngine.Builder.LibraryLoader mLibraryLoader;
+    private VersionSafeCallbacks.LibraryLoader mLibraryLoader;

[kapishnikov, 2016/11/18 16:32:57]

Can we keep the old CronetEngine.Builder.LibraryLoader type here? I think
CronetEngineBuilderImpl should only use the interface methods of
CronetEngine.Builder.LibraryLoader without being aware that it is actually the
wrapper.

[pauljensen, 2016/11/18 19:18:07]

I think that defeats the purpose of this CL.  Users of libraryLoader() would
then be calling directly through the API class...which would fail if the API
class was too old and was missing new members.

[kapishnikov, 2016/11/18 19:39:15]

You are right. We should not reference the API classes directly. It slipped my
mind.

     private String mLibraryName;
     private boolean mQuicEnabled;
     private boolean mHttp2Enabled;
     private boolean mSdchEnabled;
     private String mDataReductionProxyKey;
     private String mDataReductionProxyPrimaryProxy;
     private String mDataReductionProxyFallbackProxy;
     private String mDataReductionProxySecureProxyCheckUrl;
     private boolean mDisableCache;
     private int mHttpCacheMode;
@@ skipping 66 matching lines @@
         mLibraryName = libName;
         return this;
     }
 
     String libraryName() {
         return mLibraryName;
     }
 
     @Override
     public CronetEngineBuilderImpl setLibraryLoader(CronetEngine.Builder.LibraryLoader loader) {
-        mLibraryLoader = loader;
+        mLibraryLoader = new VersionSafeCallbacks.LibraryLoader(loader);

[kapishnikov, 2016/11/18 16:32:57]

I think it would be better if we don't create the wrapper directly but instead
use some factory that creates the wrapper. Thus, the specifics of how the
wrapper is created would be outside of this class and can be changed in the
future without changing this class. It can also help with testing. E.g., we can
inject (or directly instantiate) something like VersionSafeCallbacksFactory that
has a method createLibraryLoader().

This line would be changed to:
mLibraryLoader = mVersionSafeCallbacksFactory.createLibraryLoader(loader);

         return this;
     }
 
-    CronetEngine.Builder.LibraryLoader libraryLoader() {
+    VersionSafeCallbacks.LibraryLoader libraryLoader() {

[kapishnikov, 2016/11/18 16:32:57]

Same here. Keep CronetEngine.Builder.LibraryLoader

[pauljensen, 2016/11/18 19:18:07]

I think that defeats the purpose of this CL.  Users of libraryLoader() would
then be calling directly through the API class...which would fail if the API
class was too old and was missing new members.

[kapishnikov, 2016/11/18 19:39:15]

Acknowledged.

         return mLibraryLoader;
     }
 
     @Override
     public CronetEngineBuilderImpl enableQuic(boolean value) {
         mQuicEnabled = value;
         return this;
     }
 
     boolean quicEnabled() {
@@ skipping 315 matching lines @@
         } catch (ClassNotFoundException e) {
             // Leave as null.
             Log.i(TAG, "Class loader " + loader + " cannot find Cronet engine implementation: "
                             + name + ". Will try to find an alternative implementation.");
         } catch (Exception e) {
             throw new IllegalStateException("Cannot instantiate: " + name, e);
         }
         return cronetEngine;
     }
 }