Backport to M116: [Privacy Sandbox] Add more granular metrics for att…

…estation failures In metrics, distinguish between the cases where: * the attestations map hasn't yet been loaded * the desired site isn't attested (cherry picked from commit 3ea840d) Change-Id: I99f1c5318bf709a7c348540974e1809858c821d0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4615871 Reviewed-by: Shivani Sharma <shivanisha@chromium.org> Commit-Queue: Garrett Tanzer <gtanzer@chromium.org> Reviewed-by: Theodore Olsauskas-Warren <sauski@google.com> Cr-Original-Commit-Position: refs/heads/main@{#1162439} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4650269 Cr-Commit-Position: refs/branch-heads/5845@{#150} Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
chromium · Jun 27, 2023 · 92711e2 · 92711e2
1 parent 2957333
commit 92711e2
Show file tree

Hide file tree

Showing 14 changed files with 220 additions and 134 deletions.
diff --git a/chrome/browser/privacy_sandbox/privacy_sandbox_service.h b/chrome/browser/privacy_sandbox/privacy_sandbox_service.h
@@ -13,6 +13,7 @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/prefs/pref_change_registrar.h"
+#include "components/privacy_sandbox/canonical_topic.h"
 #include "components/privacy_sandbox/privacy_sandbox_prefs.h"
 #include "components/privacy_sandbox/privacy_sandbox_settings.h"
 #include "components/profile_metrics/browser_profile_type.h"

diff --git a/chrome/browser/privacy_sandbox/privacy_sandbox_settings_browsertest.cc b/chrome/browser/privacy_sandbox/privacy_sandbox_settings_browsertest.cc
@@ -236,14 +236,15 @@ IN_PROC_BROWSER_TEST_F(PrivacySandboxSettingsEventReportingBrowserTest,
   ASSERT_TRUE(https_server_.Start());
 
   privacy_sandbox::PrivacySandboxAttestations::GetInstance()
-      ->SetAttestationsForTesting({
-          {net::SchemefulSite(https_server_.GetOrigin("a.test")),
-           {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
-                kProtectedAudience}},
-          {net::SchemefulSite(https_server_.GetOrigin("d.test")),
-           {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
-                kProtectedAudience}},
-      });
+      ->SetAttestationsForTesting(
+          privacy_sandbox::PrivacySandboxAttestationsMap{
+              {net::SchemefulSite(https_server_.GetOrigin("a.test")),
+               {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
+                    kProtectedAudience}},
+              {net::SchemefulSite(https_server_.GetOrigin("d.test")),
+               {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
+                    kProtectedAudience}},
+          });
 
   content::RenderFrameHost* fenced_frame_node = LoadAndNavigateFencedFrame();
   ASSERT_NE(fenced_frame_node, nullptr);
@@ -282,11 +283,12 @@ IN_PROC_BROWSER_TEST_F(PrivacySandboxSettingsEventReportingBrowserTest,
   ASSERT_TRUE(https_server_.Start());
 
   privacy_sandbox::PrivacySandboxAttestations::GetInstance()
-      ->SetAttestationsForTesting({
-          {net::SchemefulSite(https_server_.GetOrigin("a.test")),
-           {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
-                kProtectedAudience}},
-      });
+      ->SetAttestationsForTesting(
+          privacy_sandbox::PrivacySandboxAttestationsMap{
+              {net::SchemefulSite(https_server_.GetOrigin("a.test")),
+               {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
+                    kProtectedAudience}},
+          });
 
   content::RenderFrameHost* fenced_frame_node = LoadAndNavigateFencedFrame();
   ASSERT_NE(fenced_frame_node, nullptr);
@@ -328,14 +330,15 @@ IN_PROC_BROWSER_TEST_F(PrivacySandboxSettingsEventReportingBrowserTest,
   ASSERT_TRUE(https_server_.Start());
 
   privacy_sandbox::PrivacySandboxAttestations::GetInstance()
-      ->SetAttestationsForTesting({
-          {net::SchemefulSite(https_server_.GetOrigin("a.test")),
-           {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
-                kProtectedAudience}},
-          {net::SchemefulSite(https_server_.GetOrigin("d.test")),
-           {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
-                kProtectedAudience}},
-      });
+      ->SetAttestationsForTesting(
+          privacy_sandbox::PrivacySandboxAttestationsMap{
+              {net::SchemefulSite(https_server_.GetOrigin("a.test")),
+               {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
+                    kProtectedAudience}},
+              {net::SchemefulSite(https_server_.GetOrigin("d.test")),
+               {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
+                    kProtectedAudience}},
+          });
 
   content::RenderFrameHost* fenced_frame_node = LoadAndNavigateFencedFrame();
   ASSERT_NE(fenced_frame_node, nullptr);
@@ -370,14 +373,15 @@ IN_PROC_BROWSER_TEST_F(PrivacySandboxSettingsEventReportingBrowserTest,
   ASSERT_TRUE(https_server_.Start());
 
   privacy_sandbox::PrivacySandboxAttestations::GetInstance()
-      ->SetAttestationsForTesting({
-          {net::SchemefulSite(https_server_.GetOrigin("a.test")),
-           {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
-                kProtectedAudience}},
-          {net::SchemefulSite(https_server_.GetOrigin("d.test")),
-           {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
-                kSharedStorage}},
-      });
+      ->SetAttestationsForTesting(
+          privacy_sandbox::PrivacySandboxAttestationsMap{
+              {net::SchemefulSite(https_server_.GetOrigin("a.test")),
+               {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
+                    kProtectedAudience}},
+              {net::SchemefulSite(https_server_.GetOrigin("d.test")),
+               {privacy_sandbox::PrivacySandboxAttestationsGatedAPI::
+                    kSharedStorage}},
+          });
 
   content::RenderFrameHost* fenced_frame_node = LoadAndNavigateFencedFrame();
   ASSERT_NE(fenced_frame_node, nullptr);

diff --git a/components/privacy_sandbox/BUILD.gn b/components/privacy_sandbox/BUILD.gn
@@ -29,13 +29,30 @@ component("features") {
   public_deps = [ "//base" ]
 }
 
+source_set("privacy_sandbox_settings_headers") {
+  sources = [
+    "privacy_sandbox_settings.h",
+    "privacy_sandbox_settings_impl.h",
+  ]
+
+  deps = [
+    "//components/browsing_topics/common:common",
+    "//components/keyed_service/core:core",
+    "//components/pref_registry:pref_registry",
+    "//components/prefs",
+    "//content/public/browser",
+    "//third_party/blink/public/common",
+    "//url:url",
+  ]
+
+  public_deps = [ "//base" ]
+}
+
 source_set("privacy_sandbox") {
   sources = [
     "canonical_topic.cc",
     "canonical_topic.h",
-    "privacy_sandbox_settings.h",
     "privacy_sandbox_settings_impl.cc",
-    "privacy_sandbox_settings_impl.h",
   ]
 
   deps = [
@@ -57,7 +74,7 @@ source_set("privacy_sandbox") {
 
   public_deps = [
     ":features",
-    "//base",
+    ":privacy_sandbox_settings_headers",
   ]
 }
 

diff --git a/components/privacy_sandbox/privacy_sandbox_attestations/BUILD.gn b/components/privacy_sandbox/privacy_sandbox_attestations/BUILD.gn
@@ -12,6 +12,7 @@ source_set("privacy_sandbox_attestations") {
 
   deps = [
     "//components/privacy_sandbox:features",
+    "//components/privacy_sandbox:privacy_sandbox_settings_headers",
     "//components/privacy_sandbox/privacy_sandbox_attestations/proto:proto",
   ]
 

diff --git a/components/privacy_sandbox/privacy_sandbox_attestations/privacy_sandbox_attestations.cc b/components/privacy_sandbox/privacy_sandbox_attestations/privacy_sandbox_attestations.cc
@@ -90,44 +90,38 @@ PrivacySandboxAttestations::CreateForTesting() {
 
 PrivacySandboxAttestations::~PrivacySandboxAttestations() = default;
 
-PrivacySandboxAttestations::PrivacySandboxAttestations(
-    PrivacySandboxAttestations&&) = default;
-
-PrivacySandboxAttestations& PrivacySandboxAttestations::operator=(
-    PrivacySandboxAttestations&&) = default;
-
-bool PrivacySandboxAttestations::IsSiteAttested(
+PrivacySandboxSettingsImpl::Status PrivacySandboxAttestations::IsSiteAttested(
     const net::SchemefulSite& site,
     PrivacySandboxAttestationsGatedAPI invoking_api) const {
   // If attestations aren't enabled, pass the check trivially.
   if (!base::FeatureList::IsEnabled(
           privacy_sandbox::kEnforcePrivacySandboxAttestations)) {
-    return true;
+    return PrivacySandboxSettingsImpl::Status::kAllowed;
   }
 
   // Pass the check if the site is in the list of devtools overrides.
   if (IsOverridden(site)) {
-    return true;
+    return PrivacySandboxSettingsImpl::Status::kAllowed;
   }
 
   // When the attesations map is not present, the behavior is default-deny.
   if (!attestations_map_.has_value()) {
-    return false;
+    return PrivacySandboxSettingsImpl::Status::kAttestationsNotLoaded;
   }
 
   // If `site` isn't enrolled at all, fail the check.
   auto it = attestations_map_->find(site);
   if (it == attestations_map_->end()) {
-    return false;
+    return PrivacySandboxSettingsImpl::Status::kAttestationFailed;
   }
 
   // If `site` is attested for `invoking_api`, pass the check.
   if (it->second.Has(invoking_api)) {
-    return true;
+    return PrivacySandboxSettingsImpl::Status::kAllowed;
   }
 
   // Otherwise, fail.
-  return false;
+  return PrivacySandboxSettingsImpl::Status::kAttestationFailed;
 }
 
 void PrivacySandboxAttestations::AddOverride(const net::SchemefulSite& site) {
@@ -140,7 +134,7 @@ bool PrivacySandboxAttestations::IsOverridden(
 }
 
 void PrivacySandboxAttestations::SetAttestationsForTesting(
-    PrivacySandboxAttestationsMap attestations_map) {
+    absl::optional<PrivacySandboxAttestationsMap> attestations_map) {
   attestations_map_ = std::move(attestations_map);
 }
 

diff --git a/components/privacy_sandbox/privacy_sandbox_attestations/privacy_sandbox_attestations.h b/components/privacy_sandbox/privacy_sandbox_attestations/privacy_sandbox_attestations.h
@@ -8,6 +8,8 @@
 #include <memory>
 #include <vector>
 
+#include "components/privacy_sandbox/privacy_sandbox_settings_impl.h"
+
 #include "base/containers/enum_set.h"
 #include "base/containers/flat_map.h"
 #include "base/no_destructor.h"
@@ -16,21 +18,6 @@
 
 namespace privacy_sandbox {
 
-// When a new enum value is added:
-// 1. Update kMaxValue to match it.
-// 2. Update `PrivacySandboxAttestationsGatedAPIProto` in
-//    `privacy_sandbox_attestations.proto`.
-// 3. Update `AllowAPI` in `privacy_sandbox_attestations_parser.cc`.
-enum class PrivacySandboxAttestationsGatedAPI {
-  kTopics,
-  kProtectedAudience,
-  kPrivateAggregation,
-  kAttributionReporting,
-  kSharedStorage,
-
-  kMaxValue = kSharedStorage,
-};
-
 using PrivacySandboxAttestationsGatedAPISet =
     base::EnumSet<PrivacySandboxAttestationsGatedAPI,
                   PrivacySandboxAttestationsGatedAPI::kTopics,
@@ -75,8 +62,9 @@ class PrivacySandboxAttestations {
   // Returns whether `site` is enrolled and attested for `invoking_api`.
   // (If the `kEnforcePrivacySandboxAttestations` flag is disabled, returns
   // true unconditionally.)
-  bool IsSiteAttested(const net::SchemefulSite& site,
-                      PrivacySandboxAttestationsGatedAPI invoking_api) const;
+  PrivacySandboxSettingsImpl::Status IsSiteAttested(
+      const net::SchemefulSite& site,
+      PrivacySandboxAttestationsGatedAPI invoking_api) const;
 
   // Override the site to be attested for all the Privacy Sandbox APIs, even if
   // it is not officially enrolled. This allows developers to test Privacy
@@ -89,7 +77,7 @@ class PrivacySandboxAttestations {
   // calling this to make sure the attestations map is set to the testing
   // instance.
   void SetAttestationsForTesting(
-      PrivacySandboxAttestationsMap attestations_map);
+      absl::optional<PrivacySandboxAttestationsMap> attestations_map);
 
  private:
   // TODO(xiaochenzh): This class should also hold the version of the

diff --git a/...nts/privacy_sandbox/privacy_sandbox_attestations/privacy_sandbox_attestations_unittest.cc b/...nts/privacy_sandbox/privacy_sandbox_attestations/privacy_sandbox_attestations_unittest.cc
@@ -22,6 +22,9 @@ class PrivacySandboxAttestationsTestBase : public testing::Test {
   PrivacySandboxAttestationsTestBase()
       : scoped_attestations_(PrivacySandboxAttestations::CreateForTesting()) {}
 
+ protected:
+  using Status = PrivacySandboxSettingsImpl::Status;
+
  private:
   ScopedPrivacySandboxAttestations scoped_attestations_;
 };
@@ -41,8 +44,10 @@ TEST_F(PrivacySandboxAttestationsTestBase,
       privacy_sandbox::kEnforcePrivacySandboxAttestations));
   net::SchemefulSite site(GURL("https://example.com"));
 
-  EXPECT_TRUE(PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
-      site, PrivacySandboxAttestationsGatedAPI::kTopics));
+  Status attestation_status =
+      PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
+          site, PrivacySandboxAttestationsGatedAPI::kTopics);
+  EXPECT_EQ(attestation_status, Status::kAllowed);
 }
 
 class PrivacySandboxAttestationsFeatureEnabledTest
@@ -61,14 +66,18 @@ TEST_F(PrivacySandboxAttestationsFeatureEnabledTest,
        DefaultDenyIfAttestationsMapNotPresent) {
   net::SchemefulSite site(GURL("https://example.com"));
 
-  EXPECT_FALSE(PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
-      site, PrivacySandboxAttestationsGatedAPI::kTopics));
+  Status attestation_status =
+      PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
+          site, PrivacySandboxAttestationsGatedAPI::kTopics);
+  EXPECT_EQ(attestation_status, Status::kAttestationsNotLoaded);
 }
 
 TEST_F(PrivacySandboxAttestationsFeatureEnabledTest, AttestedIfOverridden) {
   net::SchemefulSite site(GURL("https://example.com"));
-  ASSERT_FALSE(PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
-      site, PrivacySandboxAttestationsGatedAPI::kTopics));
+  Status attestation_status =
+      PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
+          site, PrivacySandboxAttestationsGatedAPI::kTopics);
+  ASSERT_NE(attestation_status, Status::kAllowed);
 
   PrivacySandboxAttestations::GetInstance()->AddOverride(site);
   EXPECT_TRUE(PrivacySandboxAttestations::GetInstance()->IsOverridden(site));
@@ -77,27 +86,36 @@ TEST_F(PrivacySandboxAttestationsFeatureEnabledTest, AttestedIfOverridden) {
 TEST_F(PrivacySandboxAttestationsFeatureEnabledTest,
        EnrolledWithoutAttestations) {
   net::SchemefulSite site(GURL("https://example.com"));
-  ASSERT_FALSE(PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
-      site, PrivacySandboxAttestationsGatedAPI::kTopics));
+  Status attestation_status =
+      PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
+          site, PrivacySandboxAttestationsGatedAPI::kTopics);
+  ASSERT_NE(attestation_status, Status::kAllowed);
 
   PrivacySandboxAttestations::GetInstance()->SetAttestationsForTesting(
-      {{site, {}}});
+      PrivacySandboxAttestationsMap{{site, {}}});
 
-  EXPECT_FALSE(PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
-      site, PrivacySandboxAttestationsGatedAPI::kTopics));
+  Status new_attestation_status =
+      PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
+          site, PrivacySandboxAttestationsGatedAPI::kTopics);
+  EXPECT_NE(new_attestation_status, Status::kAllowed);
 }
 
 TEST_F(PrivacySandboxAttestationsFeatureEnabledTest, EnrolledAndAttested) {
   net::SchemefulSite site(GURL("https://example.com"));
-  ASSERT_FALSE(PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
-      site, PrivacySandboxAttestationsGatedAPI::kTopics));
+  Status attestation_status =
+      PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
+          site, PrivacySandboxAttestationsGatedAPI::kTopics);
+  ASSERT_NE(attestation_status, Status::kAllowed);
 
   PrivacySandboxAttestations::GetInstance()->SetAttestationsForTesting(
-      {{site, PrivacySandboxAttestationsGatedAPISet{
-                  PrivacySandboxAttestationsGatedAPI::kTopics}}});
-
-  EXPECT_TRUE(PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
-      site, PrivacySandboxAttestationsGatedAPI::kTopics));
+      PrivacySandboxAttestationsMap{
+          {site, PrivacySandboxAttestationsGatedAPISet{
+                     PrivacySandboxAttestationsGatedAPI::kTopics}}});
+
+  Status new_attestation_status =
+      PrivacySandboxAttestations::GetInstance()->IsSiteAttested(
+          site, PrivacySandboxAttestationsGatedAPI::kTopics);
+  EXPECT_EQ(new_attestation_status, Status::kAllowed);
 }
 
 }  // namespace privacy_sandbox
diff --git a/components/privacy_sandbox/privacy_sandbox_settings.h b/components/privacy_sandbox/privacy_sandbox_settings.h
@@ -6,8 +6,8 @@
 #define COMPONENTS_PRIVACY_SANDBOX_PRIVACY_SANDBOX_SETTINGS_H_
 
 #include "components/keyed_service/core/keyed_service.h"
-#include "components/privacy_sandbox/canonical_topic.h"
-#include "components/privacy_sandbox/privacy_sandbox_attestations/privacy_sandbox_attestations.h"
+
+#include "base/time/time.h"
 
 class GURL;
 
@@ -17,6 +17,23 @@ class Origin;
 
 namespace privacy_sandbox {
 
+class CanonicalTopic;
+
+// When a new enum value is added:
+// 1. Update kMaxValue to match it.
+// 2. Update `PrivacySandboxAttestationsGatedAPIProto` in
+//    `privacy_sandbox_attestations.proto`.
+// 3. Update `AllowAPI` in `privacy_sandbox_attestations_parser.cc`.
+enum class PrivacySandboxAttestationsGatedAPI {
+  kTopics,
+  kProtectedAudience,
+  kPrivateAggregation,
+  kAttributionReporting,
+  kSharedStorage,
+
+  kMaxValue = kSharedStorage,
+};
+
 // A service which acts as a intermediary between Privacy Sandbox APIs and the
 // preferences and content settings which define when they are allowed to be
 // accessed. Privacy Sandbox APIs, regardless of where they live (renderer,