Commit 164469e8 authored by Nate Fischer's avatar Nate Fischer Committed by Commit Bot

AW UMA: support kForceEnableMetricsReporting debugging flag

This adds support for the kForceEnableMetricsReporting, but should not
change behavior unless this flag is specified.

When the flag is specified, override sampling, user consent, and app
consent to always upload metrics logs.

This does not aim to support the flag for other embedders, only WebView.

Bug: 994418
Test: build/android/adb_system_webview_command_line --force-enable-metrics-reporting
Test: out/Default/bin/system_webview_shell_apk launch https://example.com/
Test: Verify AwMetricsLogUploader#uploadLog is consistently called
Test: run_android_webview_unittests -f AwMetricsServiceClientTest.TestCanForceEnableMetrics
Change-Id: I2a8804ad26a1e7d55cb0e9fdf9ef0b58f4bbfc3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1797025Reviewed-by: default avatarChangwan Ryu <changwan@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695692}
parent 2c5c7340
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include "android_webview/browser/aw_feature_list.h" #include "android_webview/browser/aw_feature_list.h"
#include "android_webview/browser/aw_metrics_log_uploader.h" #include "android_webview/browser/aw_metrics_log_uploader.h"
#include "android_webview/browser_jni_headers/AwMetricsServiceClient_jni.h" #include "android_webview/browser_jni_headers/AwMetricsServiceClient_jni.h"
#include "android_webview/common/aw_switches.h"
#include "base/android/jni_android.h" #include "base/android/jni_android.h"
#include "base/android/jni_array.h" #include "base/android/jni_array.h"
#include "base/android/jni_string.h" #include "base/android/jni_string.h"
...@@ -184,7 +183,8 @@ bool AwMetricsServiceClient::IsConsentGiven() const { ...@@ -184,7 +183,8 @@ bool AwMetricsServiceClient::IsConsentGiven() const {
bool AwMetricsServiceClient::IsReportingEnabled() const { bool AwMetricsServiceClient::IsReportingEnabled() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return EnabledStateProvider::IsReportingEnabled() && is_in_sample_; return IsMetricsReportingForceEnabled() ||
(EnabledStateProvider::IsReportingEnabled() && is_in_sample_);
} }
metrics::MetricsService* AwMetricsServiceClient::GetMetricsService() { metrics::MetricsService* AwMetricsServiceClient::GetMetricsService() {
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "android_webview/browser/aw_metrics_service_client.h" #include "android_webview/browser/aw_metrics_service_client.h"
#include "base/command_line.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/run_loop.h" #include "base/run_loop.h"
...@@ -11,6 +12,7 @@ ...@@ -11,6 +12,7 @@
#include "base/test/test_simple_task_runner.h" #include "base/test/test_simple_task_runner.h"
#include "components/metrics/metrics_pref_names.h" #include "components/metrics/metrics_pref_names.h"
#include "components/metrics/metrics_service.h" #include "components/metrics/metrics_service.h"
#include "components/metrics/metrics_switches.h"
#include "components/prefs/testing_pref_service.h" #include "components/prefs/testing_pref_service.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -23,7 +25,7 @@ const char kTestClientId[] = "01234567-89ab-40cd-80ef-0123456789ab"; ...@@ -23,7 +25,7 @@ const char kTestClientId[] = "01234567-89ab-40cd-80ef-0123456789ab";
class TestClient : public AwMetricsServiceClient { class TestClient : public AwMetricsServiceClient {
public: public:
TestClient() {} TestClient() : in_sample_(true) {}
~TestClient() override {} ~TestClient() override {}
bool IsRecordingActive() { bool IsRecordingActive() {
...@@ -32,11 +34,13 @@ class TestClient : public AwMetricsServiceClient { ...@@ -32,11 +34,13 @@ class TestClient : public AwMetricsServiceClient {
return service->recording_active(); return service->recording_active();
return false; return false;
} }
void SetInSample(bool value) { in_sample_ = value; }
protected: protected:
bool IsInSample() override { return true; } bool IsInSample() override { return true; }
private: private:
bool in_sample_;
DISALLOW_COPY_AND_ASSIGN(TestClient); DISALLOW_COPY_AND_ASSIGN(TestClient);
}; };
...@@ -125,4 +129,22 @@ TEST_F(AwMetricsServiceClientTest, TestSetConsentFalseClearsClientId) { ...@@ -125,4 +129,22 @@ TEST_F(AwMetricsServiceClientTest, TestSetConsentFalseClearsClientId) {
ASSERT_FALSE(prefs->HasPrefPath(metrics::prefs::kMetricsClientID)); ASSERT_FALSE(prefs->HasPrefPath(metrics::prefs::kMetricsClientID));
} }
TEST_F(AwMetricsServiceClientTest, TestCanForceEnableMetrics) {
auto prefs = CreateTestPrefs();
auto client = CreateAndInitTestClient(prefs.get());
base::CommandLine::ForCurrentProcess()->AppendSwitch(
metrics::switches::kForceEnableMetricsReporting);
// Flip everything to false; flag should have higher precedence.
client->SetHaveMetricsConsent(false);
client->SetInSample(false);
ASSERT_TRUE(client->IsReportingEnabled());
// Flip things to true just to double-check everything still works.
client->SetHaveMetricsConsent(true);
client->SetInSample(true);
ASSERT_TRUE(client->IsReportingEnabled());
}
} // namespace android_webview } // namespace android_webview
...@@ -69,7 +69,7 @@ void MetricsServiceClient::UpdateRunningServices() { ...@@ -69,7 +69,7 @@ void MetricsServiceClient::UpdateRunningServices() {
update_running_services_.Run(); update_running_services_.Run();
} }
bool MetricsServiceClient::IsMetricsReportingForceEnabled() { bool MetricsServiceClient::IsMetricsReportingForceEnabled() const {
return base::CommandLine::ForCurrentProcess()->HasSwitch( return base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kForceEnableMetricsReporting); switches::kForceEnableMetricsReporting);
} }
......
...@@ -146,7 +146,7 @@ class MetricsServiceClient { ...@@ -146,7 +146,7 @@ class MetricsServiceClient {
void UpdateRunningServices(); void UpdateRunningServices();
// Checks if the user has forced metrics collection on via the override flag. // Checks if the user has forced metrics collection on via the override flag.
bool IsMetricsReportingForceEnabled(); bool IsMetricsReportingForceEnabled() const;
private: private:
base::Closure update_running_services_; base::Closure update_running_services_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment