diff --git a/android_webview/browser/metrics/aw_metrics_service_client.cc b/android_webview/browser/metrics/aw_metrics_service_client.cc index 63a8a07f6e2cea..d16ef6a02e3f2a 100644 --- a/android_webview/browser/metrics/aw_metrics_service_client.cc +++ b/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& listener) { + AwMetricsServiceClient::GetInstance() + ->SetOnFinalMetricsCollectedListenerForTesting(base::BindRepeating( + base::android::RunRunnableAndroid, + base::android::ScopedJavaGlobalRef(listener))); +} + } // namespace android_webview diff --git a/android_webview/java/src/org/chromium/android_webview/metrics/AwMetricsServiceClient.java b/android_webview/java/src/org/chromium/android_webview/metrics/AwMetricsServiceClient.java index 0f49c219764233..9c6cbb4d645d16 100644 --- a/android_webview/java/src/org/chromium/android_webview/metrics/AwMetricsServiceClient.java +++ b/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); } } diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java index d730abd1f0c315..c26d4218e55c4c 100644 --- a/android_webview/javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java +++ b/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(); + } + } } diff --git a/components/embedder_support/android/metrics/android_metrics_service_client.cc b/components/embedder_support/android/metrics/android_metrics_service_client.cc index c4508daef9ac02..b59d1ce2883f9b 100644 --- a/components/embedder_support/android/metrics/android_metrics_service_client.cc +++ b/components/embedder_support/android/metrics/android_metrics_service_client.cc @@ -190,6 +190,20 @@ std::unique_ptr 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())); } diff --git a/components/embedder_support/android/metrics/android_metrics_service_client.h b/components/embedder_support/android/metrics/android_metrics_service_client.h index d0edeeaea09c38..7d6a9f6fb63cd5 100644 --- a/components/embedder_support/android/metrics/android_metrics_service_client.h +++ b/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