Add shim library for //third_party/distributed_point_functions

This CL adds a shim library that exposes exactly the functionality that
//content/browser/aggregation_service needs. Its header file does *not*
include dpf/distributed_point_function.h, which would cause a future
name collision, as described in the linked bug.

Bug: 1499970, 1501035
Low-Coverage-Reason: COVERAGE_UNDERREPORTED https://crbug.com/1501035
Change-Id: Ia7adabcb22e490f0d08559fd8f6042af04095713
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5008533
Reviewed-by: Alex Turner <alexmt@chromium.org>
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Auto-Submit: Dan McArdle <dmcardle@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1222494}

BUILD.gn

@@ -106,6 +106,7 @@ group("gn_all") {
       "//third_party/angle/src/tests:angle_end2end_tests",
       "//third_party/angle/src/tests:angle_unittests",
       "//third_party/angle/src/tests:angle_white_box_tests",
+      "//third_party/distributed_point_functions:distributed_point_functions_unittests",
       "//third_party/flatbuffers:flatbuffers_unittests",
       "//third_party/liburlpattern:liburlpattern_unittests",
       "//tools/binary_size:binary_size_trybot_py",

content/browser/aggregation_service/DEPS

@@ -1,4 +1,5 @@
 include_rules = [
   "+components/aggregation_service",
-  "+third_party/distributed_point_functions",
+  "+third_party/distributed_point_functions/shim",
+  "+third_party/distributed_point_functions/dpf/distributed_point_function.pb.h",
 ]

content/browser/aggregation_service/aggregatable_report.cc

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include <limits>
+#include <optional>
 #include <ostream>
 #include <string>
 #include <type_traits>
@@ -43,7 +44,8 @@
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/blink/public/mojom/private_aggregation/aggregatable_report.mojom.h"
 #include "third_party/boringssl/src/include/openssl/hpke.h"
-#include "third_party/distributed_point_functions/code/dpf/distributed_point_function.h"
+#include "third_party/distributed_point_functions/dpf/distributed_point_function.pb.h"
+#include "third_party/distributed_point_functions/shim/distributed_point_function_shim.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
@@ -51,8 +53,6 @@ namespace content {
 
 namespace {
 
-using DistributedPointFunction =
-    distributed_point_functions::DistributedPointFunction;
 using DpfKey = distributed_point_functions::DpfKey;
 using DpfParameters = distributed_point_functions::DpfParameters;
 
@@ -112,31 +112,22 @@ std::vector<DpfKey> GenerateDpfKeys(
             blink::mojom::AggregationServiceMode::kExperimentalPoplar);
   DCHECK_EQ(contents.contributions.size(), 1u);
 
-  // absl::StatusOr is not allowed in the codebase, but this minimal usage is
-  // necessary to interact with //third_party/distributed_point_functions/.
-  absl::StatusOr<std::unique_ptr<DistributedPointFunction>> status_or_dpf =
-      DistributedPointFunction::CreateIncremental(ConstructDpfParameters());
-  if (!status_or_dpf.ok()) {
-    return {};
-  }
-  std::unique_ptr<DistributedPointFunction> dpf =
-      std::move(status_or_dpf).value();
-
-  // We want the same beta, no matter which prefix length is used.
-  absl::StatusOr<std::pair<DpfKey, DpfKey>> status_or_dpf_keys =
-      dpf->GenerateKeysIncremental(
+  std::optional<std::pair<DpfKey, DpfKey>> maybe_dpf_keys =
+      distributed_point_functions::GenerateKeysIncremental(
+          ConstructDpfParameters(),
           /*alpha=*/contents.contributions[0].bucket,
-          /*beta=*/std::vector<absl::uint128>(
-              AggregatableReport::kBucketDomainBitLength,
-              contents.contributions[0].value));
-  if (!status_or_dpf_keys.ok()) {
+          // We want the same beta, no matter which prefix length is used.
+          /*beta=*/
+          std::vector<absl::uint128>(AggregatableReport::kBucketDomainBitLength,
+                                     contents.contributions[0].value));
+
+  if (!maybe_dpf_keys.has_value()) {
     return {};
   }
 
   std::vector<DpfKey> dpf_keys;
-  dpf_keys.push_back(std::move(status_or_dpf_keys->first));
-  dpf_keys.push_back(std::move(status_or_dpf_keys->second));
-
+  dpf_keys.push_back(std::move(maybe_dpf_keys->first));
+  dpf_keys.push_back(std::move(maybe_dpf_keys->second));
   return dpf_keys;
 }
 

third_party/distributed_point_functions/BUILD.gn

@@ -3,14 +3,9 @@
 # found in the LICENSE file.
 
 import("//testing/libfuzzer/fuzzer_test.gni")
+import("//testing/test.gni")
 import("//third_party/protobuf/proto_library.gni")
 
-proto_library("proto") {
-  sources = [ "code/dpf/distributed_point_function.proto" ]
-  proto_out_dir = "third_party/distributed_point_functions/dpf"
-  cc_generator_options = "lite"
-}
-
 if (!defined(dpf_abseil_cpp_dir)) {
   dpf_abseil_cpp_dir = "//third_party/abseil-cpp"
 }
@@ -18,7 +13,54 @@ if (!defined(dpf_highway_cpp_dir)) {
   dpf_highway_cpp_dir = "//third_party/highway"
 }
 
-config("distributed_point_functions_includes") {
+# This is Chromium's interface with the third-party distributed_point_functions
+# library. It prevents macros like absl/log/check.h's CHECK from leaking into
+# Chromium code via header includes. Chromium already defines CHECK in
+# base/check.h, so direct includes can lead to macro redefinition errors. For
+# more context, see <https://crbug.com/1499970>.
+static_library("distributed_point_functions") {
+  sources = [
+    "shim/distributed_point_function_shim.cc",
+    "shim/distributed_point_function_shim.h",
+  ]
+  deps = [ ":internal" ]
+  public_deps = [ ":proto" ]
+  configs += [ ":includes" ]
+}
+
+proto_library("proto") {
+  sources = [ "code/dpf/distributed_point_function.proto" ]
+  proto_out_dir = "third_party/distributed_point_functions/dpf"
+  cc_generator_options = "lite"
+}
+
+test("distributed_point_functions_unittests") {
+  sources = [ "shim/distributed_point_function_shim_unittest.cc" ]
+  deps = [
+    ":distributed_point_functions",
+    "$dpf_abseil_cpp_dir:absl",
+    "//testing/gtest",
+    "//testing/gtest:gtest_main",
+    "//third_party/protobuf:protobuf_lite",
+  ]
+}
+
+fuzzer_test("dpf_fuzzer") {
+  sources = [ "fuzz/dpf_fuzzer.cc" ]
+  deps = [ ":internal" ]
+
+  # Do not apply Chromium code rules to this third-party code.
+  suppressed_configs = [ "//build/config/compiler:chromium_code" ]
+  additional_configs = [ "//build/config/compiler:no_chromium_code" ]
+
+  additional_configs += [ ":includes" ]
+}
+
+# The targets below this line are internal-only; they should only be visible to
+# targets in this directory.
+visibility = [ ":*" ]
+
+config("includes") {
   include_dirs = [
     ".",
     "code",
@@ -26,7 +68,7 @@ config("distributed_point_functions_includes") {
   ]
 }
 
-source_set("distributed_point_functions") {
+source_set("internal") {
   sources = [
     "code/dpf/aes_128_fixed_key_hash.cc",
     "code/dpf/aes_128_fixed_key_hash.h",
@@ -61,14 +103,5 @@ source_set("distributed_point_functions") {
   configs -= [ "//build/config/compiler:chromium_code" ]
   configs += [ "//build/config/compiler:no_chromium_code" ]
 
-  public_configs = [ ":distributed_point_functions_includes" ]
-}
-
-fuzzer_test("dpf_fuzzer") {
-  sources = [ "fuzz/dpf_fuzzer.cc" ]
-  deps = [ ":distributed_point_functions" ]
-
-  # Do not apply Chromium code rules to this third-party code.
-  suppressed_configs = [ "//build/config/compiler:chromium_code" ]
-  additional_configs = [ "//build/config/compiler:no_chromium_code" ]
+  configs += [ ":includes" ]
 }

third_party/distributed_point_functions/DEPS

@@ -6,6 +6,7 @@ include_rules = [
   "+gmock",
   "+google/protobuf",
   "+gtest",
+  "+testing",
   "+hwy",
   "+openssl",
 ]

third_party/distributed_point_functions/shim/distributed_point_function_shim.cc

@@ -0,0 +1,40 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <memory>
+#include <optional>
+#include <utility>
+#include <vector>
+
+#include "third_party/abseil-cpp/absl/numeric/int128.h"
+#include "third_party/abseil-cpp/absl/status/status.h"
+#include "third_party/abseil-cpp/absl/status/statusor.h"
+#include "third_party/distributed_point_functions/dpf/distributed_point_function.pb.h"
+#include "third_party/distributed_point_functions/shim/distributed_point_function_shim.h"
+
+// NOTE: To avoid name-colliding macros named "CHECK", this header must not be
+// transitively included into a compilation unit that also uses base/check.h.
+// See <https://crbug.com/1499970>.
+#include "third_party/distributed_point_functions/code/dpf/distributed_point_function.h"
+
+namespace distributed_point_functions {
+std::optional<std::pair<DpfKey, DpfKey>> GenerateKeysIncremental(
+    std::vector<DpfParameters> parameters,
+    absl::uint128 alpha,
+    std::vector<absl::uint128> beta) {
+  // absl::StatusOr is not allowed in the codebase, but this minimal usage is
+  // necessary to interact with //third_party/distributed_point_functions/.
+  absl::StatusOr<std::unique_ptr<DistributedPointFunction>> status_or_dpf =
+      DistributedPointFunction::CreateIncremental(std::move(parameters));
+  if (!status_or_dpf.ok() || !status_or_dpf.value()) {
+    return std::nullopt;
+  }
+  absl::StatusOr<std::pair<DpfKey, DpfKey>> status_or_dpf_keys =
+      status_or_dpf.value()->GenerateKeysIncremental(alpha, std::move(beta));
+  if (!status_or_dpf_keys.ok()) {
+    return std::nullopt;
+  }
+  return std::move(status_or_dpf_keys.value());
+}
+}  // namespace distributed_point_functions

third_party/distributed_point_functions/shim/distributed_point_function_shim.h

@@ -0,0 +1,30 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_BROWSER_AGGREGATION_SERVICE_DISTRIBUTED_POINT_FUNCTION_SHIM_H_
+#define CONTENT_BROWSER_AGGREGATION_SERVICE_DISTRIBUTED_POINT_FUNCTION_SHIM_H_
+
+#include <optional>
+#include <utility>
+#include <vector>
+
+#include "third_party/abseil-cpp/absl/numeric/int128.h"
+#include "third_party/distributed_point_functions/dpf/distributed_point_function.pb.h"
+
+// This header is the interface between Chromium code and the third-party
+// distributed_point_functions library. For more info, see the comment in
+// ../BUILD.gn above the //third_party/distributed_point_functions target.
+
+namespace distributed_point_functions {
+
+// Generates a pair of keys for a DPF that evaluates to `beta` when given
+// `alpha`. On failure, returns std::nullopt.
+std::optional<std::pair<DpfKey, DpfKey>> GenerateKeysIncremental(
+    std::vector<DpfParameters> parameters,
+    absl::uint128 alpha,
+    std::vector<absl::uint128> beta);
+
+}  // namespace distributed_point_functions
+
+#endif  // CONTENT_BROWSER_AGGREGATION_SERVICE_DISTRIBUTED_POINT_FUNCTION_SHIM_H_

third_party/distributed_point_functions/shim/distributed_point_function_shim_unittest.cc

@@ -0,0 +1,52 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <stddef.h>
+
+#include <optional>
+#include <utility>
+#include <vector>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/abseil-cpp/absl/numeric/int128.h"
+#include "third_party/distributed_point_functions/dpf/distributed_point_function.pb.h"
+#include "third_party/distributed_point_functions/shim/distributed_point_function_shim.h"
+
+namespace distributed_point_functions {
+
+// The shim's GenerateKeysIncremental() can return a value besides std::nullopt.
+TEST(DistributedPointFunctionShimTest, GenerateKeysIncrementalConstructsKeys) {
+  constexpr size_t kBitLength = 32;
+  std::vector<DpfParameters> parameters(kBitLength);
+  for (size_t i = 0; i < parameters.size(); ++i) {
+    parameters[i].set_log_domain_size(i + 1);
+    parameters[i].mutable_value_type()->mutable_integer()->set_bitsize(
+        parameters.size());
+  }
+  std::optional<std::pair<DpfKey, DpfKey>> maybe_dpf_keys =
+      GenerateKeysIncremental(
+          std::move(parameters),
+          /*alpha=*/absl::uint128{1},
+          /*beta=*/std::vector<absl::uint128>(kBitLength, absl::uint128{1}));
+  EXPECT_TRUE(maybe_dpf_keys.has_value());
+}
+
+// When DistributedPointFunction::CreateIncremental() fails, the shim's
+// GenerateKeysIncremental() should return std::nullopt.
+TEST(DistributedPointFunctionShimTest, GenerateKeysIncrementalEmptyParameters) {
+  EXPECT_FALSE(GenerateKeysIncremental(/*parameters=*/{},
+                                       /*alpha=*/absl::uint128{}, /*beta=*/{}));
+}
+
+// When the length of beta does not match the number of parameters, the internal
+// call to DistributedPointFunction::GenerateKeysIncremental() will fail, and
+// the shim's GenerateKeysIncremental() should return std::nullopt.
+TEST(DistributedPointFunctionShimTest, GenerateKeysIncrementalBetaWrongSize) {
+  std::vector<DpfParameters> parameters(3);
+  EXPECT_FALSE(
+      GenerateKeysIncremental(/*parameters=*/std::vector<DpfParameters>(3),
+                              /*alpha=*/absl::uint128{}, /*beta=*/{1, 2, 3}));
+}
+
+}  // namespace distributed_point_functions