Reland "[conversions] Use base 10 encoding for impression and conversion data"

This is a reland of 107694e1

The change was reverted due to an uninitialized member on an Impression
object in the test. The reland defaults Impression.impression_data to
0 to avoid these types of errors.

Original change's description:
> [conversions] Use base 10 encoding for impression and conversion data
>
> What:
> No longer parse impressiondata as hex, or use hex when sending
> conversion reports.
>
> Why:
> This causes needless conversions for developers, as raised on
> https://github.com/WICG/conversion-measurement-api/issues/60
>
> Note that the conversions database stores imp and conversion data as
> strings currently because they are represented as hex. These can
> be converted to strings now, but this is left as a followup.
>
> Bug: 1163599
> Change-Id: I5b164c380a97cb272c6a0ee698df8b744feb58ae
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611958
> Reviewed-by: Charlie Harrison <csharrison@chromium.org>
> Reviewed-by: Nate Chapin <japhet@chromium.org>
> Commit-Queue: John Delaney <johnidel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#842561}

Bug: 1163599
Change-Id: I192ba4b9452c3ad97f9511ce093f36f9c697ba2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2625293Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: John Delaney <johnidel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843143}

content/browser/conversions/conversion_policy.cc

@@ -7,7 +7,7 @@
 #include "base/format_macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/rand_util.h"
-#include "base/strings/stringprintf.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 
 namespace content {
@@ -59,18 +59,14 @@ std::string ConversionPolicy::GetSanitizedConversionData(
   if (noise_provider_)
     conversion_data = noise_provider_->GetNoisedConversionData(conversion_data);
 
-  // Allow at most 3 bits of entropy in conversion data. base::StringPrintf() is
-  // used over base::HexEncode() because HexEncode() returns a hex string with
-  // little-endian ordering. Big-endian ordering is expected here because the
-  // API assumes big-endian when parsing attributes.
-  return base::StringPrintf("%" PRIx64,
-                            conversion_data % kMaxAllowedConversionValues);
+  // Allow at most 3 bits of entropy in conversion data.
+  return base::NumberToString(conversion_data % kMaxAllowedConversionValues);
 }
 
 std::string ConversionPolicy::GetSanitizedImpressionData(
     uint64_t impression_data) const {
   // Impression data is allowed the full 64 bits.
-  return base::StringPrintf("%" PRIx64, impression_data);
+  return base::NumberToString(impression_data);
 }
 
 base::Time ConversionPolicy::GetExpiryTimeForImpression(

content/browser/conversions/conversion_policy.h

@@ -49,7 +49,7 @@ class CONTENT_EXPORT ConversionPolicy {
       uint64_t conversion_data) const;
 
   // Gets the sanitized impression data for an impression. Returns the decoded
-  // number as a hexadecimal string.
+  // number as a base 10 string.
   virtual std::string GetSanitizedImpressionData(
       uint64_t impression_data) const;
 

content/browser/conversions/conversion_policy_unittest.cc

@@ -51,9 +51,9 @@ TEST_F(ConversionPolicyTest, HighEntropyConversionData_StrippedToLowerBits) {
 TEST_F(ConversionPolicyTest, SanitizeHighEntropyImpressionData_Unchanged) {
   uint64_t impression_data = 256LU;
 
-  // The policy should not alter the impression data, and return the hexadecimal
+  // The policy should not alter the impression data, and return the base 10
   // representation.
-  EXPECT_EQ("100",
+  EXPECT_EQ("256",
             ConversionPolicy().GetSanitizedImpressionData(impression_data));
 }
 

content/browser/conversions/conversion_storage_sql.cc

@@ -685,6 +685,9 @@ bool ConversionStorageSql::LazyInit(DbCreationPolicy creation_policy) {
 
 bool ConversionStorageSql::InitializeSchema() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // TODO(https://crbug.com/1163599): Convert impression data and conversion
+  // data fields to integers.
+  //
   // TODO(johnidel, csharrison): Many impressions will share a target origin and
   // a reporting origin, so it makes sense to make a "shared string" table for
   // these to save disk / memory. However, this complicates the schema a lot, so

content/browser/conversions/impression_declaration_browsertest.cc

@@ -245,12 +245,12 @@ IN_PROC_BROWSER_TEST_F(ImpressionDeclarationBrowserTest,
       web_contents(),
       https_server()->GetURL("b.test", "/page_with_impression_creator.html")));
 
-  // The provided data overflows an unsigned 64 bit int, and should be handled
+  // The provided data underflows an unsigned 64 bit int, and should be handled
   // properly.
   EXPECT_TRUE(ExecJs(web_contents(), R"(
     createImpressionTag("link",
                         "page_with_conversion_redirect.html",
-                        "FFFFFFFFFFFFFFFFFFFFFF" /* impression data */,
+                        "-1" /* impression data */,
                         "https://a.com" /* conversion_destination */);)"));
   EXPECT_TRUE(ExecJs(shell(), "simulateClick(\'link\');"));
 
@@ -476,7 +476,7 @@ IN_PROC_BROWSER_TEST_F(ImpressionDeclarationBrowserTest,
   content::UntrustworthyContextMenuParams params =
       context_menu_filter->get_params();
   EXPECT_TRUE(params.impression);
-  EXPECT_EQ(16UL, params.impression->impression_data);
+  EXPECT_EQ(10UL, params.impression->impression_data);
   EXPECT_EQ(url::Origin::Create(GURL("https://dest.com")),
             params.impression->conversion_destination);
 }

content/public/common/impression.h

@@ -40,7 +40,7 @@ struct CONTENT_EXPORT Impression {
 
   // Data that will be sent in conversion reports to identify this impression.
   // Declared by the impression tag.
-  uint64_t impression_data;
+  uint64_t impression_data = 0UL;
 
   // Optional expiry specifying the amount of time this impression can convert.
   // Declared by the impression tag.

third_party/blink/renderer/core/html/conversion_measurement_parsing.cc

@@ -72,7 +72,7 @@ base::Optional<WebImpression> GetImpression(
 
   bool impression_data_is_valid = false;
   uint64_t impression_data =
-      impression_data_string.HexToUInt64Strict(&impression_data_is_valid);
+      impression_data_string.ToUInt64Strict(&impression_data_is_valid);
 
   // Provide a default of 0 if the impression data was not valid.
   impression_data = impression_data_is_valid ? impression_data : 0UL;