AW UMA: test for renderer histograms

No change to production logic. This adds a test to verify we collect
renderer process histograms and merge them with browser process
histograms (at which point we can be confident they'll be included in
the next UMA log).

Fixed: 1112018
Test: run_webview_instrumentation_test_apk -f AwMetricsIntegrationTest.*
Change-Id: I2a4cd8328061fea6e67f9f4fb43f1fdd07669525
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2473807
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Auto-Submit: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818037}

android_webview/browser/metrics/aw_metrics_service_client.cc

@@ -9,6 +9,7 @@
 
 #include "android_webview/browser/metrics/aw_stability_metrics_provider.h"
 #include "android_webview/browser_jni_headers/AwMetricsServiceClient_jni.h"
+#include "base/android/callback_android.h"
 #include "base/android/jni_android.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/persistent_histogram_allocator.h"
@@ -161,4 +162,14 @@ void JNI_AwMetricsServiceClient_SetUploadIntervalForTesting(
       base::TimeDelta::FromMilliseconds(upload_interval_ms));
 }
 
+// static
+void JNI_AwMetricsServiceClient_SetOnFinalMetricsCollectedListenerForTesting(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& listener) {
+  AwMetricsServiceClient::GetInstance()
+      ->SetOnFinalMetricsCollectedListenerForTesting(base::BindRepeating(
+          base::android::RunRunnableAndroid,
+          base::android::ScopedJavaGlobalRef<jobject>(listener)));
+}
+
 }  // namespace android_webview

android_webview/java/src/org/chromium/android_webview/metrics/AwMetricsServiceClient.java

@@ -72,10 +72,19 @@ public static void setUploadIntervalForTesting(long uploadIntervalMs) {
         AwMetricsServiceClientJni.get().setUploadIntervalForTesting(uploadIntervalMs);
     }
 
+    /**
+     * Sets a callback to run each time after final metrics have been collected.
+     */
+    @VisibleForTesting
+    public static void setOnFinalMetricsCollectedListenerForTesting(Runnable listener) {
+        AwMetricsServiceClientJni.get().setOnFinalMetricsCollectedListenerForTesting(listener);
+    }
+
     @NativeMethods
     interface Natives {
         void setHaveMetricsConsent(boolean userConsent, boolean appConsent);
         void setFastStartupForTesting(boolean fastStartupForTesting);
         void setUploadIntervalForTesting(long uploadIntervalMs);
+        void setOnFinalMetricsCollectedListenerForTesting(Runnable listener);
     }
 }

android_webview/javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java

@@ -4,6 +4,10 @@
 
 package org.chromium.android_webview.test;
 
+import static org.chromium.android_webview.test.OnlyRunIn.ProcessMode.MULTI_PROCESS;
+
+import android.support.test.InstrumentationRegistry;
+
 import androidx.test.filters.MediumTest;
 
 import org.junit.Assert;
@@ -19,12 +23,14 @@
 import org.chromium.base.Callback;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.metrics.RecordHistogram;
+import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.components.metrics.ChromeUserMetricsExtensionProtos.ChromeUserMetricsExtension;
 import org.chromium.components.metrics.MetricsSwitches;
 import org.chromium.components.metrics.SystemProfileProtos.SystemProfileProto;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
+import org.chromium.net.test.EmbeddedTestServer;
 
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -305,4 +311,56 @@ public void testPageLoadsEnableMultipleUploads() throws Throwable {
         // onPageStarted & onPageFinished, in which case onPageFinished would *also* wake up the
         // metrics service, and we might potentially have a third metrics log in the queue.
     }
+
+    @Test
+    @MediumTest
+    @Feature({"AndroidWebView"})
+    @OnlyRunIn(MULTI_PROCESS) // This test is specific to the OOP-renderer
+    public void testRendererHistograms() throws Throwable {
+        EmbeddedTestServer embeddedTestServer = EmbeddedTestServer.createAndStartServer(
+                InstrumentationRegistry.getInstrumentation().getContext());
+        try {
+            // Discard initial log since the renderer process hasn't been created yet.
+            mPlatformServiceBridge.waitForNextMetricsLog();
+
+            final CallbackHelper helper = new CallbackHelper();
+            int finalMetricsCollectedCount = helper.getCallCount();
+
+            // Load a page and wait for final metrics collection.
+            TestThreadUtils.runOnUiThreadBlocking(() -> {
+                AwMetricsServiceClient.setOnFinalMetricsCollectedListenerForTesting(
+                        () -> { helper.notifyCalled(); });
+            });
+
+            // Load a page to ensure the renderer process is created.
+            mRule.loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
+                    embeddedTestServer.getURL("/simple_page.html"));
+            helper.waitForCallback(finalMetricsCollectedCount, 1);
+
+            // At this point we know one of two things must be true:
+            //
+            // 1. The renderer process completed startup (logging the expected histogram) before
+            //    subprocess histograms were collected. In this case, we know the desired histogram
+            //    has been copied into the browser process.
+            // 2. Subprocess histograms were collected before the renderer process completed
+            //    startup. While we don't know if our histogram was copied over, we do know the
+            //    page load has finished and this woke up the metrics service, so MetricsService
+            //    will collect subprocess metrics again.
+            //
+            // Load a page and wait for another final log collection. We know this log collection
+            // must be triggered by either the second page load start (scenario 1) or the first page
+            // load finish (scenario 2), either of which ensures the renderer startup histogram must
+            // have been copied into the browser process.
+
+            mRule.loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
+                    embeddedTestServer.getURL("/simple_page.html"));
+            helper.waitForCallback(finalMetricsCollectedCount, 2);
+
+            Assert.assertEquals(1,
+                    RecordHistogram.getHistogramTotalCountForTesting(
+                            "Android.SeccompStatus.RendererSandbox"));
+        } finally {
+            embeddedTestServer.stopAndDestroyServer();
+        }
+    }
 }

components/embedder_support/android/metrics/android_metrics_service_client.cc

@@ -190,6 +190,20 @@ std::unique_ptr<metrics::FileMetricsProvider> CreateFileMetricsProvider(
   return file_metrics_provider;
 }
 
+base::OnceClosure CreateChainedClosure(base::OnceClosure cb1,
+                                       base::OnceClosure cb2) {
+  return base::BindOnce(
+      [](base::OnceClosure cb1, base::OnceClosure cb2) {
+        if (cb1) {
+          std::move(cb1).Run();
+        }
+        if (cb2) {
+          std::move(cb2).Run();
+        }
+      },
+      std::move(cb1), std::move(cb2));
+}
+
 }  // namespace
 
 // Needs to be kept in sync with the writer in
@@ -442,8 +456,11 @@ void AndroidMetricsServiceClient::CollectFinalMetricsForLog(
   // Set up the callback task to call after we receive histograms from all
   // child processes. |timeout| specifies how long to wait before absolutely
   // calling us back on the task.
-  content::FetchHistogramsAsynchronously(base::ThreadTaskRunnerHandle::Get(),
-                                         std::move(done_callback), timeout);
+  content::FetchHistogramsAsynchronously(
+      base::ThreadTaskRunnerHandle::Get(),
+      CreateChainedClosure(std::move(done_callback),
+                           on_final_metrics_collected_listener_),
+      timeout);
 
   if (collect_final_metrics_for_log_closure_)
     std::move(collect_final_metrics_for_log_closure_).Run();
@@ -519,6 +536,11 @@ void AndroidMetricsServiceClient::SetCollectFinalMetricsForLogClosureForTesting(
   collect_final_metrics_for_log_closure_ = std::move(closure);
 }
 
+void AndroidMetricsServiceClient::SetOnFinalMetricsCollectedListenerForTesting(
+    base::RepeatingClosure listener) {
+  on_final_metrics_collected_listener_ = std::move(listener);
+}
+
 int AndroidMetricsServiceClient::GetSampleBucketValue() {
   return UintToPerMille(base::PersistentHash(metrics_service_->GetClientId()));
 }

components/embedder_support/android/metrics/android_metrics_service_client.h

@@ -154,9 +154,14 @@ class AndroidMetricsServiceClient : public MetricsServiceClient,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // Runs |closure| when CollectFinalMetricsForLog() is called.
+  // Runs |closure| when CollectFinalMetricsForLog() is called, when we begin
+  // collecting final metrics.
   void SetCollectFinalMetricsForLogClosureForTesting(base::OnceClosure closure);
 
+  // Runs |listener| after all final metrics have been collected.
+  void SetOnFinalMetricsCollectedListenerForTesting(
+      base::RepeatingClosure listener);
+
   metrics::MetricsStateManager* metrics_state_manager() const {
     return metrics_state_manager_.get();
   }
@@ -256,6 +261,7 @@ class AndroidMetricsServiceClient : public MetricsServiceClient,
   base::TimeDelta overridden_upload_interval_;
 
   base::OnceClosure collect_final_metrics_for_log_closure_;
+  base::RepeatingClosure on_final_metrics_collected_listener_;
 
   // MetricsServiceClient may be created before the UI thread is promoted to
   // BrowserThread::UI. Use |sequence_checker_| to enforce that the