[AW] Cache packages allowlist lookup result in pref store

Bug: 1216202
Test: bin/run_android_webview_unittests -f "*AwMetricsServiceClientTest*"
Change-Id: Ibef135c57bbc5c0965316279539b3023e41ecc98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2988026
Reviewed-by: Michael Bai <michaelbai@chromium.org>
Commit-Queue: Hazem Ashmawy <hazems@chromium.org>
Cr-Commit-Position: refs/heads/master@{#899585}

android_webview/browser/aw_feature_list_creator.cc

@@ -89,6 +89,11 @@ const char* const kPersistentPrefsAllowlist[] = {
     // determine if the seed is expired.
     variations::prefs::kVariationsLastFetchTime,
     variations::prefs::kVariationsSeedDate,
+
+    // Cache the expiry date of the list of apps whose package names are allowed
+    // to be recorded in UMA. This will have a valid value (not
+    // base::Time::Min()) only if the app is in the allowlist.
+    prefs::kMetricsShouldRecordAppPackageNameExpiryDate,
 };
 
 void HandleReadError(PersistentPrefStore::PrefReadError error) {}
@@ -120,7 +125,7 @@ AwFeatureListCreator::~AwFeatureListCreator() {}
 std::unique_ptr<PrefService> AwFeatureListCreator::CreatePrefService() {
   auto pref_registry = base::MakeRefCounted<user_prefs::PrefRegistrySyncable>();
 
-  AwMetricsServiceClient::RegisterPrefs(pref_registry.get());
+  AwMetricsServiceClient::RegisterMetricsPrefs(pref_registry.get());
   variations::VariationsService::RegisterPrefs(pref_registry.get());
 
   embedder_support::OriginTrialPrefs::RegisterPrefs(pref_registry.get());

android_webview/browser/component_updater/loader_policies/aw_apps_package_names_allowlist_component_loader_policy.cc

@@ -38,40 +38,45 @@ namespace {
 
 constexpr int kBitsPerByte = 8;
 
-// Creates a bloomfilter after loading its data from file in `allowlist_fd`
-// and lookup the given `package_name` in it.
-bool IsLoggingPackageNameAllowed(base::ScopedFD allowlist_fd,
-                                 int num_hash,
-                                 int num_bits,
-                                 const std::string& package_name) {
+// It returns an empty (null) absl::optional<base::Time> if loading of the
+// allowlist file fails. Otherwise it returns:
+// - `expiry_date` if the given `package_name` is in the allowlist.
+// - `base::Time::Min()` if the given `package_name` isn't in the allowlist.
+absl::optional<base::Time> GetExpiryTimeIfPackageNameLoggable(
+    base::ScopedFD allowlist_fd,
+    int num_hash,
+    int num_bits,
+    const std::string& package_name,
+    const base::Time& expiry_date) {
   // Transfer the ownership of the file from `allowlist_fd` to `file_stream`.
   base::ScopedFILE file_stream(fdopen(allowlist_fd.release(), "r"));
   if (!file_stream.get())
-    return false;
+    return absl::optional<base::Time>();
 
   // TODO(https://crbug.com/1219496): use mmap instead of reading the whole
   // file.
   std::string bloom_filter_data;
   if (!base::ReadStreamToString(file_stream.get(), &bloom_filter_data) ||
       bloom_filter_data.empty()) {
-    return false;
+    return absl::optional<base::Time>();
   }
 
   // Make sure the bloomfilter binary data is of the correct length.
   if (bloom_filter_data.size() !=
       size_t((num_bits + kBitsPerByte - 1) / kBitsPerByte)) {
-    return false;
+    return absl::optional<base::Time>();
   }
 
   return optimization_guide::BloomFilter(num_hash, num_bits, bloom_filter_data)
-      .Contains(package_name);
+                 .Contains(package_name)
+             ? expiry_date
+             : base::Time::Min();
 }
 
-void SetShouldRecordPackageName(bool package_present_in_allowlist) {
+void SetShouldRecordPackageName(absl::optional<base::Time> expiry_date) {
   auto* metrics_service_client = AwMetricsServiceClient::GetInstance();
   DCHECK(metrics_service_client);
-  metrics_service_client->SetShouldRecordPackageName(
-      package_present_in_allowlist);
+  metrics_service_client->SetShouldRecordPackageName(expiry_date);
 }
 
 }  // namespace
@@ -79,7 +84,7 @@ void SetShouldRecordPackageName(bool package_present_in_allowlist) {
 AwAppsPackageNamesAllowlistComponentLoaderPolicy::
     AwAppsPackageNamesAllowlistComponentLoaderPolicy(
         std::string app_package_name,
-        base::OnceCallback<void(bool)> lookup_callback)
+        AllowListLookupCallback lookup_callback)
     : app_package_name_(std::move(app_package_name)),
       lookup_callback_(std::move(lookup_callback)) {
   DCHECK(!app_package_name_.empty());
@@ -111,25 +116,27 @@ void AwAppsPackageNamesAllowlistComponentLoaderPolicy::ComponentLoaded(
     const base::Version& version,
     base::flat_map<std::string, base::ScopedFD>& fd_map,
     std::unique_ptr<base::DictionaryValue> manifest) {
+  // TODO(https://crbug.com/1216202): store the allowlist version in the local
+  // cache, don't lookup the allowlist if it's the same version.
+
   // Have to use double because base::DictionaryValue doesn't support int64
   // values.
   absl::optional<double> expiry_date_ms =
       manifest->FindDoublePath(kExpiryDateKey);
-
+  absl::optional<int> num_hash = manifest->FindIntPath(kBloomFilterNumHashKey);
+  absl::optional<int> num_bits = manifest->FindIntPath(kBloomFilterNumBitsKey);
   // Being conservative and consider the allowlist expired when a valid expiry
   // date is absent.
-  if (!expiry_date_ms.has_value() ||
-      base::Time::UnixEpoch() +
-              base::TimeDelta::FromMillisecondsD(expiry_date_ms.value()) <=
-          base::Time::Now()) {
+  if (!expiry_date_ms.has_value() || !num_hash.has_value() ||
+      !num_bits.has_value() || num_hash.value() <= 0 || num_bits.value() <= 0) {
     ComponentLoadFailed();
     return;
   }
 
-  absl::optional<int> num_hash = manifest->FindIntPath(kBloomFilterNumHashKey);
-  absl::optional<int> num_bits = manifest->FindIntPath(kBloomFilterNumBitsKey);
-  if (!num_hash.has_value() || !num_bits.has_value() || num_hash.value() <= 0 ||
-      num_bits.value() <= 0) {
+  base::Time expiry_date =
+      base::Time::UnixEpoch() +
+      base::TimeDelta::FromMillisecondsD(expiry_date_ms.value_or(0.0));
+  if (expiry_date <= base::Time::Now()) {
     ComponentLoadFailed();
     return;
   }
@@ -142,15 +149,17 @@ void AwAppsPackageNamesAllowlistComponentLoaderPolicy::ComponentLoaded(
 
   base::ThreadPool::PostTaskAndReplyWithResult(
       FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
-      base::BindOnce(&IsLoggingPackageNameAllowed,
+      base::BindOnce(&GetExpiryTimeIfPackageNameLoggable,
                      std::move(allowlist_iterator->second), num_hash.value(),
-                     num_bits.value(), std::move(app_package_name_)),
+                     num_bits.value(), std::move(app_package_name_),
+                     expiry_date),
       std::move(lookup_callback_));
 }
 
 void AwAppsPackageNamesAllowlistComponentLoaderPolicy::ComponentLoadFailed() {
   DCHECK(lookup_callback_);
-  std::move(lookup_callback_).Run(/* lookup_result= */ false);
+  std::move(lookup_callback_)
+      .Run(/* expiry_date= */ absl::optional<base::Time>());
 }
 
 void AwAppsPackageNamesAllowlistComponentLoaderPolicy::GetHash(

android_webview/browser/component_updater/loader_policies/aw_apps_package_names_allowlist_component_loader_policy.h

@@ -15,6 +15,7 @@
 #include "base/containers/flat_map.h"
 #include "base/files/scoped_file.h"
 #include "components/component_updater/android/component_loader_policy.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace base {
 class DictionaryValue;
@@ -30,6 +31,16 @@ constexpr char kBloomFilterNumHashKey[] = "bloomfilter_num_hash";
 constexpr char kBloomFilterNumBitsKey[] = "bloomfilter_num_bits";
 constexpr char kExpiryDateKey[] = "expiry_date";
 
+// A callback that accepts an `absl::optional<base::Time>` expiry_date:
+// - If the allowlist loading fails, it will be called with a null value.
+// - If the package name isn't in the allowlist, it will be called with an
+//   always expired date `base::Time::Min()`.
+// - If the package name is in the allowlist, it will be called with the
+//   expiry_date after which the app package name shouldn't be recorded in UMA
+//   metrics.
+using AllowListLookupCallback =
+    base::OnceCallback<void(absl::optional<base::Time>)>;
+
 // Defines a loader responsible for receiving the allowlist for apps package
 // names that can be included in UMA records and lookup the embedding app's name
 // in that list.
@@ -41,7 +52,7 @@ class AwAppsPackageNamesAllowlistComponentLoaderPolicy
   //                   `app_package_name` in the packages names allowlist.
   AwAppsPackageNamesAllowlistComponentLoaderPolicy(
       std::string app_package_name,
-      base::OnceCallback<void(bool)> lookup_callback);
+      AllowListLookupCallback lookup_callback);
   ~AwAppsPackageNamesAllowlistComponentLoaderPolicy() override;
 
   AwAppsPackageNamesAllowlistComponentLoaderPolicy(
@@ -59,7 +70,7 @@ class AwAppsPackageNamesAllowlistComponentLoaderPolicy
 
  private:
   std::string app_package_name_;
-  base::OnceCallback<void(bool)> lookup_callback_;
+  AllowListLookupCallback lookup_callback_;
 };
 
 void LoadPackageNamesAllowlistComponent(

android_webview/browser/component_updater/loader_policies/aw_apps_package_names_allowlist_component_loader_policy_unittest.cc

@@ -33,23 +33,17 @@ constexpr int kNumHash = 11;
 constexpr int kNumBitsPerEntry = 16;
 const std::string kTestAllowlist[] = {"com.example.test", "my.fake.app",
                                       "yet.another.app"};
-double OneDayFromNowMs() {
-  return (base::Time::Now() + base::TimeDelta::FromDays(1) -
-          base::Time::UnixEpoch())
-      .InMillisecondsF();
-}
-
-double OneDayAgoMs() {
-  return (base::Time::Now() - base::TimeDelta::FromDays(1) -
-          base::Time::UnixEpoch())
-      .InMillisecondsF();
+double MillisFromUnixEpoch(const base::Time& time) {
+  return (time - base::Time::UnixEpoch()).InMillisecondsF();
 }
 
 std::unique_ptr<base::Value> BuildTestManifest() {
   auto manifest = std::make_unique<base::Value>(base::Value::Type::DICTIONARY);
   manifest->SetKey(kBloomFilterNumHashKey, base::Value(kNumHash));
   manifest->SetKey(kBloomFilterNumBitsKey, base::Value(3 * kNumBitsPerEntry));
-  manifest->SetKey(kExpiryDateKey, base::Value(OneDayFromNowMs()));
+  manifest->SetKey(kExpiryDateKey,
+                   base::Value(MillisFromUnixEpoch(
+                       base::Time::Now() + base::TimeDelta::FromDays(1))));
 
   return manifest;
 }
@@ -86,9 +80,9 @@ class AwAppsPackageNamesAllowlistComponentLoaderPolicyTest
     return base::ScopedFD(allowlist_fd);
   }
 
-  void LookupConfirmationCallback(bool lookup_result) {
+  void LookupConfirmationCallback(absl::optional<base::Time> expiry_date) {
     EXPECT_TRUE(checker_.CalledOnValidSequence());
-    lookup_result_ = lookup_result;
+    allowlist_expiry_date_ = expiry_date;
     lookup_run_loop_.Quit();
   }
 
@@ -98,7 +92,7 @@ class AwAppsPackageNamesAllowlistComponentLoaderPolicyTest
   base::SequenceCheckerImpl checker_;
   base::RunLoop lookup_run_loop_;
 
-  bool lookup_result_;
+  absl::optional<base::Time> allowlist_expiry_date_;
 
  private:
   base::FilePath allowlist_path_;
@@ -109,6 +103,10 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
   WritePackageNamesAllowListToFile();
   base::flat_map<std::string, base::ScopedFD> fd_map;
   fd_map[kAllowlistBloomFilterFileName] = OpenAndGetAllowlistFd();
+  std::unique_ptr<base::Value> manifest = BuildTestManifest();
+  base::Time one_day_from_now =
+      base::Time::Now() + base::TimeDelta::FromDays(1);
+  manifest->SetDoubleKey(kExpiryDateKey, MillisFromUnixEpoch(one_day_from_now));
 
   auto policy =
       std::make_unique<AwAppsPackageNamesAllowlistComponentLoaderPolicy>(
@@ -118,10 +116,11 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
                          base::Unretained(this)));
 
   policy->ComponentLoaded(base::Version(), fd_map,
-                          base::DictionaryValue::From(BuildTestManifest()));
+                          base::DictionaryValue::From(std::move(manifest)));
 
   lookup_run_loop_.Run();
-  EXPECT_TRUE(lookup_result_);
+  EXPECT_TRUE(allowlist_expiry_date_.has_value());
+  EXPECT_EQ(allowlist_expiry_date_.value(), one_day_from_now);
 }
 
 TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
@@ -141,7 +140,8 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
                           base::DictionaryValue::From(BuildTestManifest()));
 
   lookup_run_loop_.Run();
-  EXPECT_FALSE(lookup_result_);
+  EXPECT_TRUE(allowlist_expiry_date_.has_value());
+  EXPECT_TRUE(allowlist_expiry_date_.value().is_min());
 }
 
 TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
@@ -160,7 +160,7 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
                           base::DictionaryValue::From(BuildTestManifest()));
 
   lookup_run_loop_.Run();
-  EXPECT_FALSE(lookup_result_);
+  EXPECT_FALSE(allowlist_expiry_date_.has_value());
 }
 
 TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
@@ -180,7 +180,7 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
                           std::make_unique<base::DictionaryValue>());
 
   lookup_run_loop_.Run();
-  EXPECT_FALSE(lookup_result_);
+  EXPECT_FALSE(allowlist_expiry_date_.has_value());
 }
 
 TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
@@ -200,7 +200,7 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
                           base::DictionaryValue::From(BuildTestManifest()));
 
   lookup_run_loop_.Run();
-  EXPECT_FALSE(lookup_result_);
+  EXPECT_FALSE(allowlist_expiry_date_.has_value());
 }
 
 TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
@@ -220,7 +220,7 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
                           base::DictionaryValue::From(BuildTestManifest()));
 
   lookup_run_loop_.Run();
-  EXPECT_FALSE(lookup_result_);
+  EXPECT_FALSE(allowlist_expiry_date_.has_value());
 }
 
 TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
@@ -229,7 +229,9 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
   base::flat_map<std::string, base::ScopedFD> fd_map;
   fd_map[kAllowlistBloomFilterFileName] = OpenAndGetAllowlistFd();
   std::unique_ptr<base::Value> manifest = BuildTestManifest();
-  manifest->SetKey(kExpiryDateKey, base::Value(OneDayAgoMs()));
+  manifest->SetKey(kExpiryDateKey,
+                   base::Value(MillisFromUnixEpoch(
+                       base::Time::Now() - base::TimeDelta::FromDays(1))));
 
   auto policy =
       std::make_unique<AwAppsPackageNamesAllowlistComponentLoaderPolicy>(
@@ -242,7 +244,7 @@ TEST_F(AwAppsPackageNamesAllowlistComponentLoaderPolicyTest,
                           base::DictionaryValue::From(std::move(manifest)));
 
   lookup_run_loop_.Run();
-  EXPECT_FALSE(lookup_result_);
+  EXPECT_FALSE(allowlist_expiry_date_.has_value());
 }
 
 }  // namespace android_webview

android_webview/browser/metrics/aw_metrics_service_client.cc

@@ -16,6 +16,7 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/persistent_histogram_allocator.h"
 #include "base/no_destructor.h"
+#include "base/time/time.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "components/prefs/pref_service.h"
@@ -23,6 +24,11 @@
 
 namespace android_webview {
 
+namespace prefs {
+const char kMetricsShouldRecordAppPackageNameExpiryDate[] =
+    "aw_metrics_app_package_name_allowlist.expiry_date";
+}  // namespace prefs
+
 namespace {
 
 // IMPORTANT: DO NOT CHANGE sample rates without first ensuring the Chrome
@@ -90,18 +96,29 @@ int AwMetricsServiceClient::GetSampleRatePerMille() const {
 }
 
 bool AwMetricsServiceClient::ShouldRecordPackageName() {
-  if (base::FeatureList::IsEnabled(
+  if (!base::FeatureList::IsEnabled(
           android_webview::features::kWebViewAppsPackageNamesAllowlist)) {
-    return should_record_package_name_;
+    // Revert to the default implementation of using a random sample to decide
+    // whether to record the app package name or not.
+    return ::metrics::AndroidMetricsServiceClient::ShouldRecordPackageName();
   }
-  // Revert to the default implementation of using a random sample to decide
-  // whether to record the app package name or not.
-  return ::metrics::AndroidMetricsServiceClient::ShouldRecordPackageName();
+
+  PrefService* local_state = pref_service();
+  DCHECK(local_state);
+  return local_state->GetTime(
+             prefs::kMetricsShouldRecordAppPackageNameExpiryDate) >=
+         base::Time::Now();
 }
 
 void AwMetricsServiceClient::SetShouldRecordPackageName(
-    bool should_record_package_name) {
-  should_record_package_name_ = should_record_package_name;
+    absl::optional<base::Time> expiry_date) {
+  if (!expiry_date.has_value())
+    return;
+
+  PrefService* local_state = pref_service();
+  DCHECK(local_state);
+  local_state->SetTime(prefs::kMetricsShouldRecordAppPackageNameExpiryDate,
+                       expiry_date.value());
 }
 
 void AwMetricsServiceClient::OnMetricsStart() {
@@ -151,6 +168,14 @@ void AwMetricsServiceClient::RegisterAdditionalMetricsProviders(
   delegate_->RegisterAdditionalMetricsProviders(service);
 }
 
+// static
+void AwMetricsServiceClient::RegisterMetricsPrefs(
+    PrefRegistrySimple* registry) {
+  RegisterPrefs(registry);
+  registry->RegisterTimePref(
+      prefs::kMetricsShouldRecordAppPackageNameExpiryDate, base::Time::Min());
+}
+
 // static
 void JNI_AwMetricsServiceClient_SetHaveMetricsConsent(JNIEnv* env,
                                                       jboolean user_consent,

android_webview/browser/metrics/aw_metrics_service_client.h

@@ -13,6 +13,7 @@
 #include "base/metrics/field_trial.h"
 #include "base/no_destructor.h"
 #include "base/sequence_checker.h"
+#include "base/time/time.h"
 #include "components/embedder_support/android/metrics/android_metrics_service_client.h"
 #include "components/metrics/enabled_state_provider.h"
 #include "components/metrics/metrics_log_uploader.h"
@@ -22,6 +23,10 @@
 
 namespace android_webview {
 
+namespace prefs {
+extern const char kMetricsShouldRecordAppPackageNameExpiryDate[];
+}  // namespace prefs
+
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused.
 // TODO(https://crbug.com/1012025): remove this when the kInstallDate pref has
@@ -127,6 +132,8 @@ class AwMetricsServiceClient : public ::metrics::AndroidMetricsServiceClient,
   static void SetInstance(
       std::unique_ptr<AwMetricsServiceClient> aw_metrics_service_client);
 
+  static void RegisterMetricsPrefs(PrefRegistrySimple* registry);
+
   AwMetricsServiceClient(std::unique_ptr<Delegate> delegate);
   ~AwMetricsServiceClient() override;
 
@@ -154,15 +161,24 @@ class AwMetricsServiceClient : public ::metrics::AndroidMetricsServiceClient,
   // `::metrics::AndroidMetricsServiceClient::ShouldRecordPackageName` is used.
   bool ShouldRecordPackageName() override;
 
-  // Sets whether the embedding app's package name is allowed to be recorded in
-  // UMA logs or not. This is determened by looking up the app package name in a
+  // Sets that the embedding app's package name is allowed to be recorded in
+  // UMA logs. This is determened by looking up the app package name in a
   // dynamically downloaded allowlist of apps see
   // `AwAppsPackageNamesAllowlistComponentLoaderPolicy`.
-  void SetShouldRecordPackageName(bool should_record_package_name);
+  //
+  // `expiry_date` the date after which the app package name shouldn't be
+  //               recoreded in UMA because the allowlist that contained this
+  //               app has expired. If it has a null value, then it will be
+  //               ignored and the cached date will be used if any.
+  void SetShouldRecordPackageName(absl::optional<base::Time> expiry_date);
+
+ protected:
+  // Restrict usage of the inherited AndroidMetricsServiceClient::RegisterPrefs,
+  // RegisterMetricsPrefs should be used instead.
+  using AndroidMetricsServiceClient::RegisterPrefs;
 
  private:
   bool app_in_foreground_ = false;
-  bool should_record_package_name_ = false;
   std::unique_ptr<Delegate> delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(AwMetricsServiceClient);

android_webview/browser/metrics/aw_metrics_service_client_unittest.cc

@@ -0,0 +1,103 @@
+// Copyright 2021 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 "android_webview/browser/metrics/aw_metrics_service_client.h"
+
+#include <memory>
+
+#include "android_webview/common/aw_features.h"
+#include "base/metrics/user_metrics.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/test_simple_task_runner.h"
+#include "base/time/time.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace android_webview {
+
+namespace {
+
+class AwMetricsServiceClientTestDelegate
+    : public AwMetricsServiceClient::Delegate {
+  void RegisterAdditionalMetricsProviders(
+      metrics::MetricsService* service) override {}
+  void AddWebViewAppStateObserver(WebViewAppStateObserver* observer) override {}
+  bool HasAwContentsEverCreated() const override { return false; }
+};
+
+class AwMetricsServiceClientTest : public testing::Test {
+  AwMetricsServiceClientTest& operator=(const AwMetricsServiceClientTest&) =
+      delete;
+  AwMetricsServiceClientTest(AwMetricsServiceClientTest&&) = delete;
+  AwMetricsServiceClientTest& operator=(AwMetricsServiceClientTest&&) = delete;
+
+ protected:
+  AwMetricsServiceClientTest()
+      : task_runner_(new base::TestSimpleTaskRunner),
+        prefs_(std::make_unique<TestingPrefServiceSimple>()),
+        client_(std::make_unique<AwMetricsServiceClient>(
+            std::make_unique<AwMetricsServiceClientTestDelegate>())) {
+    // Required by MetricsService.
+    base::SetRecordActionTaskRunner(task_runner_);
+    AwMetricsServiceClient::RegisterMetricsPrefs(prefs_->registry());
+    client_->Initialize(prefs_.get());
+  }
+
+  AwMetricsServiceClient* GetClient() { return client_.get(); }
+
+ private:
+  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+  std::unique_ptr<TestingPrefServiceSimple> prefs_;
+  std::unique_ptr<AwMetricsServiceClient> client_;
+};
+
+}  // namespace
+
+TEST_F(AwMetricsServiceClientTest, TestShouldRecordPackageName_CacheNotSet) {
+  base::test::ScopedFeatureList scoped_list;
+  scoped_list.InitAndEnableFeature(
+      android_webview::features::kWebViewAppsPackageNamesAllowlist);
+
+  EXPECT_FALSE(GetClient()->ShouldRecordPackageName());
+}
+
+TEST_F(AwMetricsServiceClientTest,
+       TestShouldRecordPackageName_TestExpiredResult) {
+  base::test::ScopedFeatureList scoped_list;
+  scoped_list.InitAndEnableFeature(
+      android_webview::features::kWebViewAppsPackageNamesAllowlist);
+
+  auto* client = GetClient();
+  auto one_day_ago = base::Time::Now() - base::TimeDelta::FromDays(1);
+  client->SetShouldRecordPackageName(/* expiry_date= */ one_day_ago);
+  EXPECT_FALSE(client->ShouldRecordPackageName());
+}
+
+TEST_F(AwMetricsServiceClientTest,
+       TestShouldRecordPackageName_TestValidResult) {
+  base::test::ScopedFeatureList scoped_list;
+  scoped_list.InitAndEnableFeature(
+      android_webview::features::kWebViewAppsPackageNamesAllowlist);
+
+  auto* client = GetClient();
+  auto one_day_from_now = base::Time::Now() + base::TimeDelta::FromDays(1);
+  client->SetShouldRecordPackageName(/* expiry_date= */ one_day_from_now);
+  EXPECT_TRUE(client->ShouldRecordPackageName());
+}
+
+TEST_F(AwMetricsServiceClientTest,
+       TestShouldRecordPackageName_TestInvalidResult) {
+  base::test::ScopedFeatureList scoped_list;
+  scoped_list.InitAndEnableFeature(
+      android_webview::features::kWebViewAppsPackageNamesAllowlist);
+
+  auto* client = GetClient();
+  auto one_day_from_now = base::Time::Now() + base::TimeDelta::FromDays(1);
+  client->SetShouldRecordPackageName(/* expiry_date= */ one_day_from_now);
+  client->SetShouldRecordPackageName(
+      /* expiry_date= */ absl::optional<base::Time>());
+  EXPECT_TRUE(client->ShouldRecordPackageName());
+}
+
+}  // namespace android_webview

android_webview/test/BUILD.gn

@@ -525,6 +525,7 @@ test("android_webview_unittests") {
     "../browser/gfx/test/rendering_test.cc",
     "../browser/gfx/test/rendering_test.h",
     "../browser/lifecycle/aw_contents_lifecycle_notifier_unittest.cc",
+    "../browser/metrics/aw_metrics_service_client_unittest.cc",
     "../browser/metrics/aw_stability_metrics_provider_unittest.cc",
     "../browser/metrics/visibility_metrics_logger_unittest.cc",
     "../browser/permission/media_access_permission_request_unittest.cc",