Upstream changes for NetworkQualityProvider

chrome/android/java/src/org/chromium/chrome/browser/net/qualityprovider/NetworkQualityProviderHelper.java

Index: chrome/android/java/src/org/chromium/chrome/browser/net/qualityprovider/NetworkQualityProviderHelper.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/net/qualityprovider/NetworkQualityProviderHelper.java b/chrome/android/java/src/org/chromium/chrome/browser/net/qualityprovider/NetworkQualityProviderHelper.java
new file mode 100644
index 0000000000000000000000000000000000000000..ba41612c71218b77ff8c18df7ca4cd4b5b36670f
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/net/qualityprovider/NetworkQualityProviderHelper.java
@@ -0,0 +1,98 @@
+// Copyright 2015 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.chrome.browser.net.qualityprovider;
+
+import android.content.Context;
+
+import org.chromium.base.CalledByNative;
+import org.chromium.chrome.browser.ChromeApplication;
+import org.chromium.chrome.browser.util.NonThreadSafe;
+
+/**
+ * This class is responsible for helping the native code communicate with the APIs provided by
+ * the operating system through NetworkQualityProvider.
+ */
+public class NetworkQualityProviderHelper {

[nyquist, 2015/07/22 14:47:00]

This class can not be instantiated externally, and all methods are private. So
make class final?

[tbansal1, 2015/07/22 21:11:30]

Done.

+    private static Context sContext = null;
+    private static NonThreadSafe sThreadCheck = null;
+
+    private NetworkQualityProviderHelper() {}
+
+    @CalledByNative
+    private static NetworkQualityProviderHelper create(Context context) {

[nyquist, 2015/07/22 14:47:00]

Why is this class here instead of just using the NetworkQualityProvider? Could
you maybe move this method to the NetworkQualityProvider instead?
You could pass in the context as an argument still, and use that context to
create the instance?

[tbansal1, 2015/07/22 21:11:30]

OK, seems like my understanding is incorrect. I thought this extra helper class
is needed because it will automatically either call the 
NetworkQualityProvider.java or the overriding class 
from the downstream code (please see the other CL for Chrome on Android)
by appropriately creating the right provider when 
createNetworkQualityProvider() is called.

My understanding is that only if there was no need 
to override NetworkQualityProvider in the downstream code,
then this class was redundant and could be removed.

Is that not correct? If not, then who calls
createNetworkQualityProvider()?

[nyquist, 2015/07/22 21:42:44]

I don't know what your plan was for who should own this. Is there a natural
owner for the instance?

You could of course keep it as a singleton, and lazily create it with a
getInstance() style method (and use getFoo...() in the context) whenever you
need it.

I do believe normal polymorphism works regardless of JNI, so that should be
fine. I don't think you need this helper class.

+        assert sThreadCheck == null;
+        assert sContext == null;
+        assert context != null;
+        sThreadCheck = new NonThreadSafe();
+        sContext = context;
+        return new NetworkQualityProviderHelper();
+    }
+
+    @CalledByNative
+    private static boolean isEstimateAvailable() {
+        assert sThreadCheck.calledOnValidThread();
+        if (sContext == null) return false;
+
+        final NetworkQualityProvider networkQualityProvider =

[nyquist, 2015/07/22 14:47:00]

Why not store this reference in the constructor instead?

[tbansal1, 2015/07/22 21:11:29]

Good idea, Done.

+                ((ChromeApplication) sContext).getNetworkQualityProvider();
+        return networkQualityProvider.isEstimateAvailable();
+    }
+
+    @CalledByNative
+    private static int getRTTMilliseconds() {
+        assert sThreadCheck.calledOnValidThread();
+        if (sContext == null) return NetworkQualityProvider.NO_VALUE;
+
+        final NetworkQualityProvider networkQualityProvider =

[nyquist, 2015/07/22 14:47:00]

Nit: Unnecessary final throughout this class.

[tbansal1, 2015/07/22 21:11:30]

Done.

+                ((ChromeApplication) sContext).getNetworkQualityProvider();
+        return networkQualityProvider.getRTTMilliseconds();
+    }
+
+    @CalledByNative
+    private static int getDownstreamThroughputKbps() {
+        assert sThreadCheck.calledOnValidThread();
+        if (sContext == null) return NetworkQualityProvider.NO_VALUE;
+
+        final NetworkQualityProvider networkQualityProvider =
+                ((ChromeApplication) sContext).getNetworkQualityProvider();
+        return convertLongToInt(networkQualityProvider.getDownstreamThroughputKbps());
+    }
+
+    @CalledByNative
+    private static int getUpstreamThroughputKbps() {
+        assert sThreadCheck.calledOnValidThread();
+        if (sContext == null) return NetworkQualityProvider.NO_VALUE;
+
+        final NetworkQualityProvider networkQualityProvider =
+                ((ChromeApplication) sContext).getNetworkQualityProvider();
+        return convertLongToInt(networkQualityProvider.getUpstreamThroughputKbps());
+    }
+
+    @CalledByNative
+    private static int getTimeSinceLastUpdateSeconds() {
+        assert sThreadCheck.calledOnValidThread();
+        if (sContext == null) return NetworkQualityProvider.NO_VALUE;
+
+        final NetworkQualityProvider networkQualityProvider =
+                ((ChromeApplication) sContext).getNetworkQualityProvider();
+        return convertLongToInt(networkQualityProvider.getTimeSinceLastUpdateSeconds());
+    }
+
+    @CalledByNative
+    private static int getNoValue() {
+        assert sThreadCheck.calledOnValidThread();
+        return NetworkQualityProvider.NO_VALUE;
+    }
+
+    private static int convertLongToInt(long value) {
+        if (value > Integer.MAX_VALUE) {
+            return Integer.MAX_VALUE;
+        }
+        if (value < Integer.MIN_VALUE) {
+            return Integer.MIN_VALUE;
+        }
+        return (int) value;
+    }
+}