[parkable_image] Allow ParkableImage to be parked while SegmentReader…

… is still alive. (cherry picked from commit eba1033) Bug: 1142834 Change-Id: Ie14a89d0470f2b452248289fef53e344707c02a9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2800477 Commit-Queue: Thiabaud Engelbrecht <thiabaud@google.com> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Benoit L <lizeb@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#871045} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2823812 Reviewed-by: Ben Mason <benmason@chromium.org> Reviewed-by: Thiabaud Engelbrecht <thiabaud@google.com> Owners-Override: Ben Mason <benmason@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Thiabaud Engelbrecht <thiabaud@google.com> Cr-Commit-Position: refs/branch-heads/4472@{#42} Cr-Branched-From: 3d60439-refs/heads/master@{#870763}
chromium · Apr 13, 2021 · ef16e55 · ef16e55
1 parent c246e34
commit ef16e55
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 14 deletions.
diff --git a/third_party/blink/renderer/platform/graphics/decoding_image_generator.cc b/third_party/blink/renderer/platform/graphics/decoding_image_generator.cc
@@ -37,6 +37,22 @@
 #include "third_party/skia/include/core/SkData.h"
 #include "third_party/skia/include/core/SkImageInfo.h"
 
+namespace {
+class ScopedSegmentReaderDataLocker {
+  STACK_ALLOCATED();
+
+ public:
+  explicit ScopedSegmentReaderDataLocker(blink::SegmentReader* segment_reader)
+      : segment_reader_(segment_reader) {
+    segment_reader_->LockData();
+  }
+  ~ScopedSegmentReaderDataLocker() { segment_reader_->UnlockData(); }
+
+ private:
+  blink::SegmentReader* const segment_reader_;
+};
+}  // namespace
+
 namespace blink {
 
 // static
@@ -178,6 +194,8 @@ bool DecodingImageGenerator::GetPixels(const SkImageInfo& dst_info,
   {
     TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"),
                  "Decode LazyPixelRef", "LazyPixelRef", lazy_pixel_ref);
+
+    ScopedSegmentReaderDataLocker lock_data(data_.get());
     decoded = frame_generator_->DecodeAndScale(
         data_.get(), all_data_received_, frame_index, decode_info, memory,
         adjusted_row_bytes, alpha_option, client_id);
@@ -226,6 +244,8 @@ bool DecodingImageGenerator::QueryYUVA(
   TRACE_EVENT0("blink", "DecodingImageGenerator::QueryYUVAInfo");
 
   DCHECK(all_data_received_);
+
+  ScopedSegmentReaderDataLocker lock_data(data_.get());
   return frame_generator_->GetYUVAInfo(data_.get(), supported_data_types,
                                        yuva_pixmap_info);
 }
@@ -261,6 +281,7 @@ bool DecodingImageGenerator::GetYUVAPlanes(const SkYUVAPixmaps& pixmaps,
     return false;
   }
 
+  ScopedSegmentReaderDataLocker lock_data(data_.get());
   return frame_generator_->DecodeToYUV(
       data_.get(), frame_index, pixmaps.plane(0).colorType(), plane_sizes,
       plane_addrs, plane_row_bytes);

diff --git a/third_party/blink/renderer/platform/graphics/parkable_image_manager.cc b/third_party/blink/renderer/platform/graphics/parkable_image_manager.cc
@@ -140,8 +140,6 @@ void ParkableImageManager::RecordStatisticsAfter5Minutes() const {
 }
 
 void ParkableImageManager::Remove(ParkableImage* image) {
-  DCHECK(IsMainThread());
-
   MutexLocker lock(lock_);
 
   // Image could be on disk or unparked. Remove it in either case.
@@ -162,8 +160,6 @@ void ParkableImageManager::MoveImage(ParkableImage* image,
 }
 
 bool ParkableImageManager::IsRegistered(ParkableImage* image) {
-  DCHECK(IsMainThread());
-
   MutexLocker lock(lock_);
 
   auto* map = image->is_on_disk() ? &on_disk_images_ : &unparked_images_;
@@ -190,8 +186,9 @@ void ParkableImageManager::ScheduleDelayedParkingTaskIfNeeded() {
   if (has_pending_parking_task_)
     return;
 
+  auto* thread = Thread::MainThread();
   scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-      Thread::Current()->GetTaskRunner();
+      thread->GetTaskRunner();
   task_runner->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(&ParkableImageManager::MaybeParkImages,

diff --git a/third_party/blink/renderer/platform/graphics/parkable_image_manager.h b/third_party/blink/renderer/platform/graphics/parkable_image_manager.h
@@ -115,7 +115,7 @@ class PLATFORM_EXPORT ParkableImageManager
   WTF::HashSet<ParkableImage*> on_disk_images_ GUARDED_BY(lock_);
 
   bool has_pending_parking_task_ GUARDED_BY(lock_) = false;
-  bool has_posted_accounting_task_ = false;
+  bool has_posted_accounting_task_ GUARDED_BY(lock_) = false;
 
   base::TimeDelta total_disk_read_time_ GUARDED_BY(lock_) = base::TimeDelta();
   base::TimeDelta total_disk_write_time_ GUARDED_BY(lock_) = base::TimeDelta();

diff --git a/third_party/blink/renderer/platform/graphics/parkable_image_test.cc b/third_party/blink/renderer/platform/graphics/parkable_image_test.cc
@@ -444,6 +444,7 @@ TEST_F(ParkableImageTest, ParkAndUnparkAborted) {
   EXPECT_TRUE(MaybePark(pi));
 
   auto snapshot = pi->MakeROSnapshot();
+  snapshot->LockData();
 
   // Run task to park image.
   RunPostedTasks();
@@ -471,6 +472,7 @@ TEST_F(ParkableImageTest, ParkAndUnparkAborted) {
   EXPECT_FALSE(MaybePark(pi));
 
   // kill the old snapshot.
+  snapshot->UnlockData();
   snapshot = nullptr;
 
   // Now that snapshot is gone, we can park.

diff --git a/third_party/blink/renderer/platform/image-decoders/segment_reader.cc b/third_party/blink/renderer/platform/image-decoders/segment_reader.cc
@@ -221,10 +221,12 @@ sk_sp<SkData> ROBufferSegmentReader::GetAsSkData() const {
 class ParkableImageSegmentReader : public SegmentReader {
  public:
   explicit ParkableImageSegmentReader(scoped_refptr<ParkableImage> image);
-  ~ParkableImageSegmentReader() override;
+  ~ParkableImageSegmentReader() override = default;
   size_t size() const override;
   size_t GetSomeData(const char*& data, size_t position) const override;
   sk_sp<SkData> GetAsSkData() const override;
+  void LockData() override;
+  void UnlockData() override;
 
  private:
   scoped_refptr<ParkableImage> parkable_image_;
@@ -242,12 +244,6 @@ ParkableImageSegmentReader::ParkableImageSegmentReader(
   MutexLocker lock(parkable_image_->lock_);
   parkable_image_->Unpark();
   DCHECK(parkable_image_->rw_buffer_);
-  parkable_image_->Lock();
-}
-
-ParkableImageSegmentReader::~ParkableImageSegmentReader() {
-  MutexLocker lock(parkable_image_->lock_);
-  parkable_image_->Unlock();
 }
 
 size_t ParkableImageSegmentReader::size() const {
@@ -260,7 +256,7 @@ size_t ParkableImageSegmentReader::GetSomeData(const char*& data,
     return 0;
 
   MutexLocker lock(parkable_image_->lock_);
-  parkable_image_->Unpark();
+  DCHECK(parkable_image_->is_locked());
 
   RWBuffer::ROIter iter(parkable_image_->rw_buffer_.get(), available_);
   size_t position_of_block = 0;
@@ -283,6 +279,19 @@ sk_sp<SkData> ParkableImageSegmentReader::GetAsSkData() const {
   return BufferCopyAsSkData(iter, available_);
 }
 
+void ParkableImageSegmentReader::LockData() {
+  MutexLocker lock(parkable_image_->lock_);
+  parkable_image_->Unpark();
+
+  parkable_image_->Lock();
+}
+
+void ParkableImageSegmentReader::UnlockData() {
+  MutexLocker lock(parkable_image_->lock_);
+
+  parkable_image_->Unlock();
+}
+
 // SegmentReader ---------------------------------------------------------------
 
 scoped_refptr<SegmentReader> SegmentReader::CreateFromSharedBuffer(

diff --git a/third_party/blink/renderer/platform/image-decoders/segment_reader.h b/third_party/blink/renderer/platform/image-decoders/segment_reader.h
@@ -52,6 +52,8 @@ class PLATFORM_EXPORT SegmentReader
   virtual size_t size() const = 0;
   virtual size_t GetSomeData(const char*& data, size_t position) const = 0;
   virtual sk_sp<SkData> GetAsSkData() const = 0;
+  virtual void LockData() {}
+  virtual void UnlockData() {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SegmentReader);

diff --git a/third_party/blink/renderer/platform/image-decoders/segment_reader_test.cc b/third_party/blink/renderer/platform/image-decoders/segment_reader_test.cc
@@ -84,6 +84,7 @@ TEST_F(ParkableImageSegmentReaderTest, GetSomeData) {
 
   auto segment_reader =
       SegmentReader::CreateFromParkableImage(std::move(parkable_image));
+  segment_reader->LockData();
 
   const char* segment;
   size_t position = 0;
@@ -93,6 +94,8 @@ TEST_F(ParkableImageSegmentReaderTest, GetSomeData) {
     position += length;
   }
   EXPECT_EQ(position, kDataSize);
+
+  segment_reader->UnlockData();
 }
 
 TEST_F(ParkableImageSegmentReaderTest, GetAsSkData) {
@@ -110,6 +113,7 @@ TEST_F(ParkableImageSegmentReaderTest, GetAsSkData) {
 
   auto segment_reader =
       SegmentReader::CreateFromParkableImage(std::move(parkable_image));
+  segment_reader->LockData();
   auto sk_data = segment_reader->GetAsSkData();
 
   const char* segment;
@@ -120,6 +124,8 @@ TEST_F(ParkableImageSegmentReaderTest, GetAsSkData) {
     position += length;
   }
   EXPECT_EQ(position, kDataSize);
+
+  segment_reader->UnlockData();
 }
 
 }  // namespace blink