Notify MetricsService of app's state

Notify MetricsService of WebView apps' foreground and background state

Bug: 1042048
Change-Id: I320d9baea677d30c464f9eaf987ab8c0f082f714
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2053126
Commit-Queue: Tao Bai <michaelbai@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741645}

android_webview/browser/metrics/BUILD.gn

@@ -11,6 +11,7 @@ source_set("metrics") {
   ]
   deps = [
     "//android_webview:browser_jni_headers",
+    "//android_webview/browser/lifecycle",
     "//android_webview/common",
     "//base",
     "//components/embedder_support/android/metrics",

android_webview/browser/metrics/DEPS

 include_rules = [
   "-android_webview/browser",
+  "+android_webview/browser/lifecycle",
   "+android_webview/browser/metrics",
   "+third_party/metrics_proto",
 ]

android_webview/browser/metrics/aw_metrics_service_client.cc

@@ -7,6 +7,7 @@
 #include <jni.h>
 #include <cstdint>
 
+#include "android_webview/browser/lifecycle/aw_contents_lifecycle_notifier.h"
 #include "android_webview/browser/metrics/aw_stability_metrics_provider.h"
 #include "android_webview/browser_jni_headers/AwMetricsServiceClient_jni.h"
 #include "android_webview/common/aw_features.h"
@@ -131,6 +132,7 @@ void AwMetricsServiceClient::InitInternal() {
 }
 
 void AwMetricsServiceClient::OnMetricsStart() {
+  AwContentsLifecycleNotifier::GetInstance().AddObserver(this);
   SetReportingEnabledDateIfNotSet(pref_service());
 }
 
@@ -142,6 +144,36 @@ bool AwMetricsServiceClient::ShouldWakeMetricsService() {
   return base::FeatureList::IsEnabled(features::kWebViewWakeMetricsService);
 }
 
+void AwMetricsServiceClient::OnAppStateChanged(
+    WebViewAppStateObserver::State state) {
+  // To match MetricsService's expectation,
+  // - does nothing if no WebView has ever been created.
+  // - starts notifying MetricsService once a WebView is created and the app
+  //   is foreground.
+  // - consolidates the other states other than kForeground into background.
+  // - avoids the duplicated notification.
+  if (state == WebViewAppStateObserver::State::kDestroyed &&
+      !AwContentsLifecycleNotifier::GetInstance()
+           .has_aw_contents_ever_created()) {
+    return;
+  }
+
+  bool foreground = state == WebViewAppStateObserver::State::kForeground;
+
+  if (foreground == app_in_foreground_)
+    return;
+
+  app_in_foreground_ = foreground;
+  if (app_in_foreground_) {
+    GetMetricsService()->OnAppEnterForeground();
+  } else {
+    // TODO(https://crbug.com/1052392): Turn on the background recording.
+    // Not recording in background, this matches Chrome's behavior.
+    GetMetricsService()->OnAppEnterBackground(
+        /* keep_recording_in_background = false */);
+  }
+}
+
 void AwMetricsServiceClient::RegisterAdditionalMetricsProviders(
     metrics::MetricsService* service) {
   service->RegisterMetricsProvider(

android_webview/browser/metrics/aw_metrics_service_client.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "android_webview/browser/lifecycle/webview_app_state_observer.h"
 #include "base/macros.h"
 #include "base/metrics/field_trial.h"
 #include "base/no_destructor.h"
@@ -88,7 +89,8 @@ enum class BackfillInstallDate {
 // the sample, it then calls MetricsService::Start(). If consent was not
 // granted, MaybeStartMetrics() instead clears the client ID, if any.
 
-class AwMetricsServiceClient : public ::metrics::AndroidMetricsServiceClient {
+class AwMetricsServiceClient : public ::metrics::AndroidMetricsServiceClient,
+                               public WebViewAppStateObserver {
   friend class base::NoDestructor<AwMetricsServiceClient>;
 
  public:
@@ -100,6 +102,9 @@ class AwMetricsServiceClient : public ::metrics::AndroidMetricsServiceClient {
   // metrics::MetricsServiceClient
   int32_t GetProduct() override;
 
+  // WebViewAppStateObserver
+  void OnAppStateChanged(WebViewAppStateObserver::State state) override;
+
   // metrics::AndroidMetricsServiceClient:
   void InitInternal() override;
   void OnMetricsStart() override;
@@ -112,6 +117,8 @@ class AwMetricsServiceClient : public ::metrics::AndroidMetricsServiceClient {
   std::string GetAppPackageNameInternal() override;
 
  private:
+  bool app_in_foreground_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(AwMetricsServiceClient);
 };
 

android_webview/browser/metrics/aw_metrics_service_client_unittest.cc

@@ -11,13 +11,13 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
-#include "base/test/task_environment.h"
 #include "base/test/test_simple_task_runner.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "components/metrics/metrics_switches.h"
 #include "components/prefs/testing_pref_service.h"
-#include "content/public/browser/notification_service.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_content_client_initializer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace android_webview {
@@ -86,9 +86,7 @@ std::unique_ptr<TestClient> CreateAndInitTestClient(PrefService* prefs) {
 
 class AwMetricsServiceClientTest : public testing::Test {
  public:
-  AwMetricsServiceClientTest()
-      : task_runner_(new base::TestSimpleTaskRunner),
-        notification_service_(content::NotificationService::Create()) {
+  AwMetricsServiceClientTest() : task_runner_(new base::TestSimpleTaskRunner) {
     // Required by MetricsService.
     base::SetRecordActionTaskRunner(task_runner_);
   }
@@ -96,15 +94,23 @@ class AwMetricsServiceClientTest : public testing::Test {
  protected:
   ~AwMetricsServiceClientTest() override {}
 
+  void SetUp() override {
+    test_content_client_initializer_ =
+        std::make_unique<content::TestContentClientInitializer>();
+  }
+
+  void TearDown() override {
+    base::RunLoop().RunUntilIdle();
+    test_content_client_initializer_.reset();
+  }
+
+  content::BrowserTaskEnvironment task_environment_;
+  std::unique_ptr<content::TestContentClientInitializer>
+      test_content_client_initializer_;
+
  private:
-  base::test::TaskEnvironment task_environment_;
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
 
-  // AwMetricsServiceClient::RegisterForNotifications() requires the
-  // NotificationService to be up and running. Initialize it here, throw away
-  // the value because we don't need it directly.
-  std::unique_ptr<content::NotificationService> notification_service_;
-
   DISALLOW_COPY_AND_ASSIGN(AwMetricsServiceClientTest);
 };