Clear the serialization buffer before serialization.

Serialized operations include paddding in the returned
|serialized_size|, but this padding isn't initialized, so stale values
from the last serialization may be present in the padding region,
resulting in the same operation yielding different digests at different
times.

Clearing out the serialization buffer prior to serialization clears out
this padding.

Bug: 973801
Change-Id: I8b4a744c17b6d396bf86db549969532431c31327
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2245886
Commit-Queue: Caleb Raitto <caraitto@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792485}

third_party/blink/renderer/platform/graphics/identifiability_paint_op_digest.cc

@@ -4,6 +4,8 @@
 
 #include "third_party/blink/renderer/platform/graphics/identifiability_paint_op_digest.h"
 
+#include <cstring>
+
 #include "gpu/command_buffer/client/raster_interface.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_metrics.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_study_participation.h"
@@ -14,6 +16,14 @@
 
 namespace blink {
 
+namespace {
+
+// To minimize performance impact, don't exceed kMaxDigestOps during the
+// lifetime of this IdentifiabilityPaintOpDigest object.
+constexpr int kMaxDigestOps = 1 << 20;
+
+}  // namespace
+
 // Storage for serialized PaintOp state.
 Vector<char>& SerializationBuffer() {
   DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<Vector<char>>,
@@ -22,7 +32,12 @@ Vector<char>& SerializationBuffer() {
 }
 
 IdentifiabilityPaintOpDigest::IdentifiabilityPaintOpDigest(IntSize size)
-    : size_(size),
+    : IdentifiabilityPaintOpDigest(size, kMaxDigestOps) {}
+
+IdentifiabilityPaintOpDigest::IdentifiabilityPaintOpDigest(IntSize size,
+                                                           int max_digest_ops)
+    : max_digest_ops_(max_digest_ops),
+      size_(size),
       paint_cache_(cc::ClientPaintCache::kNoCachingBudget),
       nodraw_canvas_(size_.Width(), size_.Height()),
       serialize_options_(&image_provider_,
@@ -49,10 +64,7 @@ constexpr size_t IdentifiabilityPaintOpDigest::kInfiniteOps;
 void IdentifiabilityPaintOpDigest::MaybeUpdateDigest(
     const sk_sp<const cc::PaintRecord>& paint_record,
     const size_t num_ops_to_visit) {
-  // To minimize performance impact, don't exceed kMaxDigestOps during the
-  // lifetime of this IdentifiabilityPaintOpDigest object.
-  constexpr int kMaxDigestOps = 1 << 20;
-  if (!IsUserInIdentifiabilityStudy() || total_ops_digested_ > kMaxDigestOps)
+  if (!IsUserInIdentifiabilityStudy() || total_ops_digested_ >= max_digest_ops_)
     return;
 
   // Determine how many PaintOps we'll need to digest after the initial digests
@@ -85,6 +97,7 @@ void IdentifiabilityPaintOpDigest::MaybeUpdateDigest(
       continue;
     }
 
+    std::memset(SerializationBuffer().data(), 0, SerializationBuffer().size());
     size_t serialized_size;
     while ((serialized_size = op->Serialize(SerializationBuffer().data(),
                                             SerializationBuffer().size(),

third_party/blink/renderer/platform/graphics/identifiability_paint_op_digest.h

@@ -24,6 +24,8 @@ class PLATFORM_EXPORT IdentifiabilityPaintOpDigest {
  public:
   // Constructs based on the size of the CanvasResourceProvider.
   explicit IdentifiabilityPaintOpDigest(IntSize size);
+  // For-testing constructor that allows setting a custom |max_digest_ops_|.
+  IdentifiabilityPaintOpDigest(IntSize size, int max_digest_ops);
   ~IdentifiabilityPaintOpDigest();
 
   // When passed as |num_ops_to_visit| to MaybeUpdateDigest(), every
@@ -55,6 +57,10 @@ class PLATFORM_EXPORT IdentifiabilityPaintOpDigest {
     ScopedResult GetRasterContent(const cc::DrawImage& draw_image) override;
   };
 
+  // The maximum number of ops to digest during the lifetime of this
+  // IdentifiabilityPaintOpDigest object.
+  const int max_digest_ops_;
+
   // The current identifiability digest -- potentially updated every
   // MaybeUpdateDigest() call.
   uint64_t digest_ = 0;

third_party/blink/renderer/platform/graphics/identifiability_paint_op_digest_unittest.cc

@@ -46,6 +46,9 @@ class StudyParticipationRaii {
 // Arbitrary non-zero size.
 constexpr IntSize kSize(10, 10);
 
+constexpr float kScaleX = 1.0f, kScaleY = 1.0f;
+constexpr uint64_t kScaleDigest = UINT64_C(5461154373811575393);
+
 TEST(IdentifiabilityPaintOpDigestTest, Construct) {
   IdentifiabilityPaintOpDigest identifiability_paintop_digest(kSize);
 }
@@ -68,11 +71,10 @@ TEST(IdentifiabilityPaintOpDigestTest, SimpleDigest) {
   StudyParticipationRaii study_participation_raii;
   IdentifiabilityPaintOpDigest identifiability_paintop_digest(kSize);
   auto paint_record = sk_make_sp<cc::PaintRecord>();
-  paint_record->push<cc::ScaleOp>(1.0f, 1.0f);
+  paint_record->push<cc::ScaleOp>(kScaleX, kScaleY);
   identifiability_paintop_digest.MaybeUpdateDigest(paint_record,
                                                    /*num_ops_to_visit=*/1);
-  EXPECT_EQ(UINT64_C(5461154373811575393),
-            identifiability_paintop_digest.digest());
+  EXPECT_EQ(kScaleDigest, identifiability_paintop_digest.digest());
 }
 
 TEST(IdentifiabilityPaintOpDigestTest, DigestIsZeroIfNotInStudy) {
@@ -154,6 +156,140 @@ TEST(IdentifiabilityPaintOpDigestTest, PaintShaderStability) {
             identifiability_paintop_digest1.digest());
 }
 
+TEST(IdentifiabilityPaintOpDigestTest, BufferLeftoversDontAffectFutureDigests) {
+  StudyParticipationRaii study_participation_raii;
+  // Make a complex path to make a PaintOp with a large serialization.
+  SkPath path;
+  path.rLineTo(1.0f, 0.0f);
+  path.rLineTo(1.0f, 1.0f);
+  path.rLineTo(0.0f, 1.0f);
+  path.rLineTo(2.0f, 0.0f);
+  path.rLineTo(2.0f, 2.0f);
+  path.rLineTo(0.0f, 2.0f);
+  path.rLineTo(1.0f, 0.0f);
+  IdentifiabilityPaintOpDigest identifiability_paintop_digest1(kSize);
+  IdentifiabilityPaintOpDigest identifiability_paintop_digest2(kSize);
+  auto paint_record1 = sk_make_sp<cc::PaintRecord>();
+  auto paint_record2 = sk_make_sp<cc::PaintRecord>();
+  paint_record1->push<cc::DrawPathOp>(path, cc::PaintFlags());
+  paint_record2->push<cc::ScaleOp>(kScaleX, kScaleY);
+  identifiability_paintop_digest1.MaybeUpdateDigest(paint_record1,
+                                                    /*num_ops_to_visit=*/1);
+  identifiability_paintop_digest2.MaybeUpdateDigest(paint_record2,
+                                                    /*num_ops_to_visit=*/1);
+  EXPECT_EQ(UINT64_C(15080511244712756058),
+            identifiability_paintop_digest1.digest());
+  EXPECT_EQ(kScaleDigest, identifiability_paintop_digest2.digest());
+}
+
+TEST(IdentifiabilityPaintOpDigestTest,
+     BufferLeftoversDontAffectFutureDigests_SameCanvas) {
+  StudyParticipationRaii study_participation_raii;
+  // Make a complex path to make a PaintOp with a large serialization.
+  SkPath path;
+  path.rLineTo(1.0f, 0.0f);
+  path.rLineTo(1.0f, 1.0f);
+  path.rLineTo(0.0f, 1.0f);
+  path.rLineTo(2.0f, 0.0f);
+  path.rLineTo(2.0f, 2.0f);
+  path.rLineTo(0.0f, 2.0f);
+  path.rLineTo(1.0f, 0.0f);
+  IdentifiabilityPaintOpDigest identifiability_paintop_digest(kSize);
+  auto paint_record = sk_make_sp<cc::PaintRecord>();
+  paint_record->push<cc::DrawPathOp>(path, cc::PaintFlags());
+  paint_record->push<cc::ScaleOp>(kScaleX, kScaleY);
+  identifiability_paintop_digest.MaybeUpdateDigest(paint_record,
+                                                   /*num_ops_to_visit=*/2);
+  EXPECT_EQ(UINT64_C(11133276391926094139),
+            identifiability_paintop_digest.digest());
+}
+
+TEST(IdentifiabilityPaintOpDigestTest, IgnoresPrefixAndSuffix) {
+  StudyParticipationRaii study_participation_raii;
+  SkPath path1;
+  path1.rLineTo(1.0f, 0.0f);
+  SkPath path2;
+  path2.rLineTo(0.0f, 1.0f);
+  IdentifiabilityPaintOpDigest identifiability_paintop_digest(kSize);
+  auto paint_record = sk_make_sp<cc::PaintRecord>();
+  paint_record->push<cc::DrawPathOp>(path1, cc::PaintFlags());
+  paint_record->push<cc::ScaleOp>(kScaleX, kScaleY);
+  paint_record->push<cc::DrawPathOp>(path2, cc::PaintFlags());
+  identifiability_paintop_digest.SetPrefixSkipCount(1);
+  identifiability_paintop_digest.MaybeUpdateDigest(paint_record,
+                                                   /*num_ops_to_visit=*/2);
+  EXPECT_EQ(kScaleDigest, identifiability_paintop_digest.digest());
+}
+
+TEST(IdentifiabilityPaintOpDigestTest, IgnoresPrefixAndSuffix_MultipleOps) {
+  StudyParticipationRaii study_participation_raii;
+  SkPath path1;
+  path1.rLineTo(1.0f, 0.0f);
+  SkPath path2;
+  path2.rLineTo(0.0f, 1.0f);
+  SkPath path3;
+  path3.rLineTo(1.0f, 1.0f);
+  IdentifiabilityPaintOpDigest identifiability_paintop_digest(kSize);
+  auto paint_record = sk_make_sp<cc::PaintRecord>();
+  paint_record->push<cc::DrawPathOp>(path1, cc::PaintFlags());
+  paint_record->push<cc::ScaleOp>(kScaleX, kScaleY);
+  paint_record->push<cc::DrawPathOp>(path2, cc::PaintFlags());
+  paint_record->push<cc::DrawPathOp>(path3, cc::PaintFlags());
+  identifiability_paintop_digest.SetPrefixSkipCount(1);
+  identifiability_paintop_digest.MaybeUpdateDigest(paint_record,
+                                                   /*num_ops_to_visit=*/2);
+  EXPECT_EQ(UINT64_C(5461154373811575393),
+            identifiability_paintop_digest.digest());
+}
+
+TEST(IdentifiabilityPaintOpDigestTest, RecursesIntoDrawRecords) {
+  StudyParticipationRaii study_participation_raii;
+  IdentifiabilityPaintOpDigest identifiability_paintop_digest(kSize);
+  auto paint_record_inner = sk_make_sp<cc::PaintRecord>();
+  auto paint_record_outer = sk_make_sp<cc::PaintRecord>();
+  paint_record_inner->push<cc::ScaleOp>(kScaleX, kScaleY);
+  paint_record_outer->push<cc::DrawRecordOp>(paint_record_inner);
+  identifiability_paintop_digest.MaybeUpdateDigest(paint_record_outer,
+                                                   /*num_ops_to_visit=*/1);
+  EXPECT_EQ(kScaleDigest, identifiability_paintop_digest.digest());
+}
+
+TEST(IdentifiabilityPaintOpDigestTest, RecursesIntoDrawRecords_TwoLevels) {
+  StudyParticipationRaii study_participation_raii;
+  IdentifiabilityPaintOpDigest identifiability_paintop_digest(kSize);
+  auto paint_record_inner = sk_make_sp<cc::PaintRecord>();
+  auto paint_record_middle = sk_make_sp<cc::PaintRecord>();
+  auto paint_record_outer = sk_make_sp<cc::PaintRecord>();
+  paint_record_inner->push<cc::ScaleOp>(kScaleX, kScaleY);
+  paint_record_middle->push<cc::DrawRecordOp>(paint_record_inner);
+  paint_record_outer->push<cc::DrawRecordOp>(paint_record_middle);
+  identifiability_paintop_digest.MaybeUpdateDigest(paint_record_outer,
+                                                   /*num_ops_to_visit=*/1);
+  EXPECT_EQ(kScaleDigest, identifiability_paintop_digest.digest());
+}
+
+TEST(IdentifiabilityPaintOpDigestTest, StopsUpdatingDigestAfterThreshold) {
+  StudyParticipationRaii study_participation_raii;
+  constexpr int kMaxOperations = 5;
+  IdentifiabilityPaintOpDigest identifiability_paintop_digest(kSize,
+                                                              kMaxOperations);
+  uint64_t last_digest = UINT64_C(0);
+  for (int i = 0; i < kMaxOperations; i++) {
+    auto paint_record = sk_make_sp<cc::PaintRecord>();
+    paint_record->push<cc::ScaleOp>(kScaleX, kScaleY);
+    identifiability_paintop_digest.MaybeUpdateDigest(paint_record,
+                                                     /*num_ops_to_visit=*/1);
+    EXPECT_NE(last_digest, identifiability_paintop_digest.digest()) << i;
+    last_digest = identifiability_paintop_digest.digest();
+  }
+
+  auto paint_record = sk_make_sp<cc::PaintRecord>();
+  paint_record->push<cc::ScaleOp>(kScaleX, kScaleY);
+  identifiability_paintop_digest.MaybeUpdateDigest(paint_record,
+                                                   /*num_ops_to_visit=*/1);
+  EXPECT_EQ(last_digest, identifiability_paintop_digest.digest());
+}
+
 }  // namespace
 
 }  // namespace blink