[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/+/2611958Reviewed-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}

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);
 }

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;