Incorporate review feedback points

ReactAndroid/src/main/java/com/facebook/react/modules/network/DefaultOkHttpProvider.java

@@ -12,6 +12,7 @@
 import android.os.Build;
 
 import com.facebook.common.logging.FLog;
+import com.facebook.react.common.ReactConstants;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -34,12 +35,12 @@ public OkHttpClient get() {
     return enableTls12OnPreLollipop(client).build();
   }
 
-  /*
-    On Android 4.1-4.4 (API level 16 to 19) TLS 1.1 and 1.2 are
-    available but not enabled by default. The following method
-    enables it.
+  /**
+   * On Android 4.1-4.4 (API level 16 to 19) TLS 1.1 and 1.2 are
+   * available but not enabled by default. The following method
+   * enables it.
    */
-  public static OkHttpClient.Builder enableTls12OnPreLollipop(OkHttpClient.Builder client) {
+  private static OkHttpClient.Builder enableTls12OnPreLollipop(OkHttpClient.Builder client) {
     if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN && Build.VERSION.SDK_INT <= Build.VERSION_CODES.KITKAT) {
       try {
         client.sslSocketFactory(new TLSSocketFactory());
@@ -54,11 +55,10 @@ public static OkHttpClient.Builder enableTls12OnPreLollipop(OkHttpClient.Builder
         specs.add(ConnectionSpec.CLEARTEXT);
 
         client.connectionSpecs(specs);
-      } catch (Exception exc) {
-        FLog.e("OkHttpClientProvider", "Error while enabling TLS 1.2", exc);
+      } catch (Exception e) {
+        FLog.e(ReactConstants.TAG, "Error while enabling TLS 1.2", e);
       }
     }
-
     return client;
   }
 }

ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java

@@ -76,11 +76,11 @@ public final class NetworkingModule extends ReactContextBaseJavaModule {
   /* package */ NetworkingModule(
       ReactApplicationContext reactContext,
       @Nullable String defaultUserAgent,
-      OkHttpClientProvider okHttpClientProvider,
+      OkHttpClientProvider httpClientProvider,
       @Nullable List<NetworkInterceptorCreator> networkInterceptorCreators) {
     super(reactContext);
 
-    mClient = createOkHttpClient(okHttpClientProvider, networkInterceptorCreators);
+    mClient = createOkHttpClient(httpClientProvider, networkInterceptorCreators);
     mCookieHandler = new ForwardingCookieHandler(reactContext);
     mCookieJarContainer = (CookieJarContainer) mClient.cookieJar();
     mShuttingDown = false;
@@ -89,9 +89,9 @@ public final class NetworkingModule extends ReactContextBaseJavaModule {
   }
 
   private OkHttpClient createOkHttpClient(
-    OkHttpClientProvider okHttpClientProvider,
+    @Nullable OkHttpClientProvider httpClientProvider,
     @Nullable List<NetworkInterceptorCreator> networkInterceptorCreators) {
-    OkHttpClient client = okHttpClientProvider == null ? new DefaultOkHttpProvider().get() : okHttpClientProvider.get();
+    OkHttpClient client = httpClientProvider == null ? new DefaultOkHttpProvider().get() : httpClientProvider.get();
     if (networkInterceptorCreators != null) {
       OkHttpClient.Builder clientBuilder = client.newBuilder();
       for (NetworkInterceptorCreator networkInterceptorCreator : networkInterceptorCreators) {
@@ -106,13 +106,13 @@ private OkHttpClient createOkHttpClient(
    * @param context the ReactContext of the application
    * @param defaultUserAgent the User-Agent header that will be set for all requests where the
    * caller does not provide one explicitly
-   * @param okHttpClientProvider the {@link OkHttpClient} provider to be used for networking
+   * @param httpClientProvider  {@link OkHttpClientProvider} to provider {@link OkHttpClient}
    */
   /* package */ NetworkingModule(
     ReactApplicationContext context,
     @Nullable String defaultUserAgent,
-    OkHttpClientProvider okHttpClientProvider) {
-    this(context, defaultUserAgent, okHttpClientProvider, null);
+    @Nullable OkHttpClientProvider httpClientProvider) {
+    this(context, defaultUserAgent, httpClientProvider, null);
   }
 
   /**
@@ -124,10 +124,10 @@ public NetworkingModule(final ReactApplicationContext context) {
 
   /**
    * @param context the ReactContext of the application
-   * @param okHttpClientProvider to provide OkHttpInstance with different settings
+   * @param httpClientProvider  {@link OkHttpClientProvider} to provide {@link OkHttpClient}
    */
-  public NetworkingModule(final ReactApplicationContext context, OkHttpClientProvider okHttpClientProvider) {
-    this(context, null, okHttpClientProvider, null);
+  public NetworkingModule(final ReactApplicationContext context, @Nullable OkHttpClientProvider httpClientProvider) {
+    this(context, null, httpClientProvider, null);
   }
 
   /**

ReactAndroid/src/main/java/com/facebook/react/modules/network/OkHttpClientProvider.java

@@ -12,7 +12,7 @@
 import okhttp3.OkHttpClient;
 
 /**
- *  Interface that provides the OkHttpClient instance, that will be used for all networking requests.
+ *  Interface to provide OkHttpClient instance that will be used for all networking requests.
  */
 public interface OkHttpClientProvider {
   OkHttpClient get();

ReactAndroid/src/main/java/com/facebook/react/shell/MainPackageConfig.java

@@ -13,45 +13,46 @@
 import com.facebook.react.modules.network.DefaultOkHttpProvider;
 import com.facebook.react.modules.network.OkHttpClientProvider;
 
+import javax.annotation.Nullable;
+
 /**
  * Configuration for {@link MainReactPackage}
  */
 public class MainPackageConfig {
 
-
-  private final OkHttpClientProvider mOkHttpClientProvider;
+  private final @Nullable OkHttpClientProvider mHttpClientProvider;
 
   private final ImagePipelineConfig mFrescoConfig;
 
   private MainPackageConfig(Builder builder) {
     mFrescoConfig = builder.mFrescoConfig;
-    mOkHttpClientProvider = builder.mOkHttpClientProvider;
+    mHttpClientProvider = builder.mHttpClientProvider;
   }
 
   public ImagePipelineConfig getFrescoConfig() {
     return mFrescoConfig;
   }
 
   public OkHttpClientProvider getOkHttpClientProvider() {
-    return mOkHttpClientProvider != null ? mOkHttpClientProvider : new DefaultOkHttpProvider();
+    return mHttpClientProvider != null ? mHttpClientProvider : new DefaultOkHttpProvider();
   }
 
   public static class Builder {
 
     private ImagePipelineConfig mFrescoConfig;
-    private OkHttpClientProvider mOkHttpClientProvider;
+    private OkHttpClientProvider mHttpClientProvider;
 
     public Builder setFrescoConfig(ImagePipelineConfig frescoConfig) {
       mFrescoConfig = frescoConfig;
       return this;
     }
 
     /**
-     * Ability to inject configured OkHttpClient provider to react-native networking subsystem.
-     * This will be very useful, especially in brownfield applications.
+     * Option to inject configured OkHttpClient to react-native networking subsystem.
+     * This will be used only by the fetch networking calls (Fresco library, for eg, will not use this client).
      */
     public Builder setOkHttpClientProvider(OkHttpClientProvider okHttpClientProvider) {
-      this.mOkHttpClientProvider = okHttpClientProvider;
+      this.mHttpClientProvider = okHttpClientProvider;
       return this;
     }
 

ReactAndroid/src/main/java/com/facebook/react/shell/MainReactPackage.java

@@ -11,6 +11,7 @@
 
 import android.content.SharedPreferences;
 import android.preference.PreferenceManager;
+
 import com.facebook.react.LazyReactPackage;
 import com.facebook.react.animated.NativeAnimatedModule;
 import com.facebook.react.bridge.JavaScriptModule;
@@ -75,12 +76,13 @@
 import com.facebook.react.views.viewpager.ReactViewPagerManager;
 import com.facebook.react.views.webview.ReactWebViewManager;
 
-import javax.inject.Provider;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import javax.inject.Provider;
+
 /**
  * Package defining basic modules and view managers.
  */
@@ -194,7 +196,9 @@ public NativeModule get() {
       new ModuleSpec(NetworkingModule.class, new Provider<NativeModule>() {
         @Override
         public NativeModule get() {
-          return mConfig == null ? new NetworkingModule(context) : new NetworkingModule(context, mConfig.getOkHttpClientProvider());
+          return mConfig == null ?
+            new NetworkingModule(context) :
+            new NetworkingModule(context, mConfig.getOkHttpClientProvider());
         }
       }),
       new ModuleSpec(NetInfoModule.class, new Provider<NativeModule>() {

ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java

@@ -78,7 +78,6 @@ public class NetworkingModuleTest {
   @Test
   public void testGetWithoutHeaders() throws Exception {
     OkHttpClient httpClient = mock(OkHttpClient.class);
-
     when(httpClient.newCall(any(Request.class))).thenAnswer(new Answer<Object>() {
       @Override
       public Object answer(InvocationOnMock invocation) throws Throwable {