[Cronet] Move HistogramManager to CronetEngine method

BUG=531538

Review URL: https://codereview.chromium.org/1431703002

Cr-Commit-Position: refs/heads/master@{#358309}

12 files changed, 66 insertions, 169 deletions

diff --git a/components/cronet/android/api/src/org/chromium/net/CronetEngine.java b/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
index 29fd3f01..e6f3161 100644
--- a/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
+++ b/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
@@ -486,6 +486,30 @@ public abstract class CronetEngine {
     public abstract void stopNetLog();
 
     /**
+     * Returns differences in metrics collected by Cronet since the last call to
+     * {@link #getGlobalMetricsDeltas}.
+     * <p>
+     * Cronet collects these metrics globally. This means deltas returned by
+     * {@code getGlobalMetricsDeltas()} will include measurements of requests
+     * processed by other {@link CronetEngine} instances. Since this function
+     * returns differences in metrics collected since the last call, and these
+     * metrics are collected globally, a call to any {@code CronetEngine}
+     * instance's {@code getGlobalMetricsDeltas()} method will affect the deltas
+     * returned by any other {@code CronetEngine} instance's
+     * {@code getGlobalMetricsDeltas()}.
+     * <p>
+     * Cronet starts collecting these metrics after the first call to
+     * {@code getGlobalMetricsDeltras()}, so the first call returns no
+     * useful data as no metrics have yet been collected.
+     *
+     * @return differences in metrics collected by Cronet, since the last call
+     *         to {@code getGlobalMetricsDeltas()}, serialized as a
+     *         <a href=https://developers.google.com/protocol-buffers>protobuf
+     *         </a>.
+     */
+    public abstract byte[] getGlobalMetricsDeltas();
+
+    /**
      * Enables the network quality estimator, which collects and reports
      * measurements of round trip time (RTT) and downstream throughput at
      * various layers of the network stack. After enabling the estimator,
diff --git a/components/cronet/android/api/src/org/chromium/net/HistogramManager.java b/components/cronet/android/api/src/org/chromium/net/HistogramManager.java
deleted file mode 100644
index 63bc673..0000000
--- a/components/cronet/android/api/src/org/chromium/net/HistogramManager.java
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright 2014 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;
-
-import java.lang.reflect.Constructor;
-
-/**
- * Controls user metrics analysis (UMA) histograms.
- */
-public abstract class HistogramManager {
-    private static final String CRONET_HISTOGRAM_MANAGER =
-            "org.chromium.net.CronetHistogramManager";
-
-    // Don't expose public constructor. Use createHistogramManager() instead.
-    HistogramManager() {}
-
-    /**
-     * Get histogram deltas serialized as
-     * <a href=https://developers.google.com/protocol-buffers>protobuf</a>.
-     */
-    public abstract byte[] getHistogramDeltas();
-
-    /**
-     * Creates Histogram Manager if native library is loaded, returns {@code null} if not.
-     */
-    public static HistogramManager createHistogramManager() {
-        HistogramManager histogramManager = null;
-        try {
-            Class<? extends HistogramManager> histogramManagerClass =
-                    HistogramManager.class.getClassLoader()
-                            .loadClass(CRONET_HISTOGRAM_MANAGER)
-                            .asSubclass(HistogramManager.class);
-            Constructor<? extends HistogramManager> constructor =
-                    histogramManagerClass.getConstructor();
-            histogramManager = constructor.newInstance();
-        } catch (ClassNotFoundException e) {
-            // Leave as null.
-        } catch (Exception e) {
-            throw new IllegalStateException(
-                    "Cannot instantiate: " + CRONET_HISTOGRAM_MANAGER,
-                    e);
-        }
-        return histogramManager;
-    }
-}
diff --git a/components/cronet/android/cronet_histogram_manager.cc b/components/cronet/android/cronet_histogram_manager.cc
deleted file mode 100644
index ca7c3f0..0000000
--- a/components/cronet/android/cronet_histogram_manager.cc
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright 2014 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.
-
-#include "components/cronet/android/cronet_histogram_manager.h"
-
-#include <string>
-#include <vector>
-
-#include "base/android/jni_array.h"
-#include "base/metrics/statistics_recorder.h"
-#include "components/cronet/histogram_manager.h"
-
-#include "jni/CronetHistogramManager_jni.h"
-
-namespace cronet {
-
-// Explicitly register static JNI functions.
-bool CronetHistogramManagerRegisterJni(JNIEnv* env) {
-  return RegisterNativesImpl(env);
-}
-
-static void EnsureInitialized(JNIEnv* env,
-                              const JavaParamRef<jobject>& jcaller) {
-  base::StatisticsRecorder::Initialize();
-}
-
-static ScopedJavaLocalRef<jbyteArray> GetHistogramDeltas(
-    JNIEnv* env,
-    const JavaParamRef<jobject>& jcaller) {
-  std::vector<uint8> data;
-  if (!HistogramManager::GetInstance()->GetDeltas(&data))
-    return ScopedJavaLocalRef<jbyteArray>();
-  return base::android::ToJavaByteArray(env, &data[0], data.size());
-}
-
-}  // namespace cronet
diff --git a/components/cronet/android/cronet_histogram_manager.h b/components/cronet/android/cronet_histogram_manager.h
deleted file mode 100644
index 1d298e1..0000000
--- a/components/cronet/android/cronet_histogram_manager.h
+++ /dev/null
@@ -1,16 +0,0 @@
-// 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.
-
-#ifndef COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_
-#define COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_
-
-#include <jni.h>
-
-namespace cronet {
-
-bool CronetHistogramManagerRegisterJni(JNIEnv* env);
-
-}  // namespace cronet
-
-#endif  // COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_
diff --git a/components/cronet/android/cronet_library_loader.cc b/components/cronet/android/cronet_library_loader.cc
index d0021a8..fec9360 100644
--- a/components/cronet/android/cronet_library_loader.cc
+++ b/components/cronet/android/cronet_library_loader.cc
@@ -17,7 +17,6 @@
 #include "base/message_loop/message_loop.h"
 #include "components/cronet/android/chromium_url_request.h"
 #include "components/cronet/android/chromium_url_request_context.h"
-#include "components/cronet/android/cronet_histogram_manager.h"
 #include "components/cronet/android/cronet_upload_data_stream_adapter.h"
 #include "components/cronet/android/cronet_url_request_adapter.h"
 #include "components/cronet/android/cronet_url_request_context_adapter.h"
@@ -41,7 +40,6 @@ const base::android::RegistrationMethod kCronetRegisteredMethods[] = {
     {"BaseAndroid", base::android::RegisterJni},
     {"ChromiumUrlRequest", ChromiumUrlRequestRegisterJni},
     {"ChromiumUrlRequestContext", ChromiumUrlRequestContextRegisterJni},
-    {"CronetHistogramManager", CronetHistogramManagerRegisterJni},
     {"CronetLibraryLoader", RegisterNativesImpl},
     {"CronetUploadDataStreamAdapter", CronetUploadDataStreamAdapterRegisterJni},
     {"CronetUrlRequestAdapter", CronetUrlRequestAdapterRegisterJni},
diff --git a/components/cronet/android/cronet_url_request_context_adapter.cc b/components/cronet/android/cronet_url_request_context_adapter.cc
index 37a18ce..67dad4e 100644
--- a/components/cronet/android/cronet_url_request_context_adapter.cc
+++ b/components/cronet/android/cronet_url_request_context_adapter.cc
@@ -7,6 +7,7 @@
 #include <map>
 
 #include "base/android/jni_android.h"
+#include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/bind.h"
 #include "base/files/file_util.h"
@@ -14,6 +15,7 @@
 #include "base/logging.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/statistics_recorder.h"
 #include "base/prefs/pref_filter.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
@@ -21,6 +23,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "components/cronet/histogram_manager.h"
 #include "components/cronet/url_request_context_config.h"
 #include "jni/CronetUrlRequestContext_jni.h"
 #include "net/base/external_estimate_provider.h"
@@ -538,4 +541,14 @@ static jint SetMinLogLevel(JNIEnv* env,
   return old_log_level;
 }
 
+static ScopedJavaLocalRef<jbyteArray> GetHistogramDeltas(
+    JNIEnv* env,
+    const JavaParamRef<jclass>& jcaller) {
+  base::StatisticsRecorder::Initialize();
+  std::vector<uint8> data;
+  if (!HistogramManager::GetInstance()->GetDeltas(&data))
+    return ScopedJavaLocalRef<jbyteArray>();
+  return base::android::ToJavaByteArray(env, &data[0], data.size());
+}
+
 }  // namespace cronet
diff --git a/components/cronet/android/java/src/org/chromium/net/CronetHistogramManager.java b/components/cronet/android/java/src/org/chromium/net/CronetHistogramManager.java
deleted file mode 100644
index 702f98e..0000000
--- a/components/cronet/android/java/src/org/chromium/net/CronetHistogramManager.java
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright 2014 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;
-
-import org.chromium.base.annotations.JNINamespace;
-
-/**
- * The implementation of {@link HistogramManager}. Controls UMA histograms.
- */
-@JNINamespace("cronet")
-final class CronetHistogramManager extends HistogramManager {
-    public CronetHistogramManager() {
-        nativeEnsureInitialized();
-    }
-
-    @Override
-    public byte[] getHistogramDeltas() {
-        return nativeGetHistogramDeltas();
-    }
-
-    private native byte[] nativeGetHistogramDeltas();
-
-    private native void nativeEnsureInitialized();
-}
diff --git a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
index 6a315f7..227bd41 100644
--- a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
+++ b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
@@ -179,6 +179,13 @@ class CronetUrlRequestContext extends CronetEngine {
         }
     }
 
+    // This method is intentionally non-static to ensure Cronet native library
+    // is loaded by class constructor.
+    @Override
+    public byte[] getGlobalMetricsDeltas() {
+        return nativeGetHistogramDeltas();
+    }
+
     @Override
     public void enableNetworkQualityEstimator(Executor executor) {
         enableNetworkQualityEstimatorForTesting(false, false, executor);
@@ -397,6 +404,8 @@ class CronetUrlRequestContext extends CronetEngine {
 
     private static native int nativeSetMinLogLevel(int loggingLevel);
 
+    private static native byte[] nativeGetHistogramDeltas();
+
     @NativeClassQualifiedName("CronetURLRequestContextAdapter")
     private native void nativeDestroy(long nativePtr);
 
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
index 9b023ac..1132c06 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
@@ -18,6 +18,7 @@ import org.chromium.net.TestUrlRequestCallback.ResponseStep;
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
+import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.NoSuchElementException;
 import java.util.concurrent.Executor;
@@ -773,4 +774,21 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         secondEngine.shutdown();
         thirdEngine.shutdown();
     }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testGetGlobalMetricsDeltas() throws Exception {
+        mTestFramework = startCronetTestFramework();
+
+        byte delta1[] = mTestFramework.mCronetEngine.getGlobalMetricsDeltas();
+
+        TestUrlRequestCallback callback = new TestUrlRequestCallback();
+        UrlRequest.Builder builder = new UrlRequest.Builder(
+                TEST_URL, callback, callback.getExecutor(), mTestFramework.mCronetEngine);
+        builder.build().start();
+        callback.blockForDone();
+        byte delta2[] = mTestFramework.mCronetEngine.getGlobalMetricsDeltas();
+        assertTrue(delta2.length != 0);
+        assertFalse(Arrays.equals(delta1, delta2));
+    }
 }
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/HistogramManagerTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/HistogramManagerTest.java
deleted file mode 100644
index 2dd5306..0000000
--- a/components/cronet/android/test/javatests/src/org/chromium/net/HistogramManagerTest.java
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright 2014 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;
-
-import android.test.suitebuilder.annotation.SmallTest;
-
-import org.chromium.base.test.util.Feature;
-
-import java.util.Arrays;
-
-/**
- * Test HistogramManager.
- */
-public class HistogramManagerTest extends CronetTestBase {
-    // URLs used for tests.
-    private static final String TEST_URL = "http://127.0.0.1:8000";
-
-    CronetTestFramework mTestFramework;
-
-    @SmallTest
-    @Feature({"Cronet"})
-    public void testHistogramManager() throws Exception {
-        mTestFramework = startCronetTestFramework();
-        byte delta1[] = mTestFramework.mHistogramManager.getHistogramDeltas();
-
-        TestUrlRequestCallback callback = new TestUrlRequestCallback();
-        UrlRequest.Builder builder = new UrlRequest.Builder(
-                TEST_URL, callback, callback.getExecutor(), mTestFramework.mCronetEngine);
-        builder.build().start();
-        callback.blockForDone();
-        byte delta2[] = mTestFramework.mHistogramManager.getHistogramDeltas();
-        assertTrue(delta2.length != 0);
-        assertFalse(Arrays.equals(delta1, delta2));
-    }
-}
diff --git a/components/cronet/android/test/src/org/chromium/net/CronetTestFramework.java b/components/cronet/android/test/src/org/chromium/net/CronetTestFramework.java
index 530c1dd..fbee5f9 100644
--- a/components/cronet/android/test/src/org/chromium/net/CronetTestFramework.java
+++ b/components/cronet/android/test/src/org/chromium/net/CronetTestFramework.java
@@ -66,7 +66,6 @@ public class CronetTestFramework {
     public URLStreamHandlerFactory mStreamHandlerFactory;
     public CronetEngine mCronetEngine;
     @SuppressWarnings("deprecation") HttpUrlRequestFactory mRequestFactory;
-    @SuppressFBWarnings("URF_UNREAD_FIELD") HistogramManager mHistogramManager;
 
     private final String[] mCommandLine;
     private final Context mContext;
@@ -129,7 +128,8 @@ public class CronetTestFramework {
             mStreamHandlerFactory = mCronetEngine.createURLStreamHandlerFactory();
         }
 
-        mHistogramManager = HistogramManager.createHistogramManager();
+        // Start collecting metrics.
+        mCronetEngine.getGlobalMetricsDeltas();
 
         if (LIBRARY_INIT_CRONET_ONLY.equals(initString)) {
             return;
diff --git a/components/cronet/cronet_static.gypi b/components/cronet/cronet_static.gypi
index f2dce0d..cf62a0a 100644
--- a/components/cronet/cronet_static.gypi
+++ b/components/cronet/cronet_static.gypi
@@ -21,8 +21,6 @@
     'android/chromium_url_request_context.h',
     'android/chromium_url_request_error_list.h',
     'android/chromium_url_request_priority_list.h',
-    'android/cronet_histogram_manager.cc',
-    'android/cronet_histogram_manager.h',
     'android/cronet_in_memory_pref_store.cc',
     'android/cronet_in_memory_pref_store.h',
     'android/cronet_library_loader.cc',