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

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

Index: components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.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 4fe66aa9419661f3ce20267c1faa8750bdf1628c..c53f5e137d42dea651a565e8641b4187ddadf17d 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
@@ -87,7 +87,7 @@ public class CronetEngineBuilderImpl extends ICronetEngineBuilder {
     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;
@@ -174,11 +174,11 @@ public class CronetEngineBuilderImpl extends ICronetEngineBuilder {
 
     @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;
     }