diff --git a/lib/ui/painting/image_decoder.cc b/lib/ui/painting/image_decoder.cc index e679e4d296bb9..3d497f9aea82c 100644 --- a/lib/ui/painting/image_decoder.cc +++ b/lib/ui/painting/image_decoder.cc @@ -75,7 +75,7 @@ static sk_sp ResizeRasterImage(sk_sp image, } static sk_sp ImageFromDecompressedData( - fml::RefPtr descriptor, + ImageDescriptor* descriptor, uint32_t target_width, uint32_t target_height, const fml::tracing::TraceFlow& flow) { @@ -98,7 +98,7 @@ static sk_sp ImageFromDecompressedData( SkISize::Make(target_width, target_height), flow); } -sk_sp ImageFromCompressedData(fml::RefPtr descriptor, +sk_sp ImageFromCompressedData(ImageDescriptor* descriptor, uint32_t target_width, uint32_t target_height, const fml::tracing::TraceFlow& flow) { @@ -216,38 +216,55 @@ static SkiaGPUObject UploadRasterImage( return result; } -void ImageDecoder::Decode(fml::RefPtr descriptor, +void ImageDecoder::Decode(fml::RefPtr descriptor_ref_ptr, uint32_t target_width, uint32_t target_height, const ImageResult& callback) { TRACE_EVENT0("flutter", __FUNCTION__); fml::tracing::TraceFlow flow(__FUNCTION__); + // ImageDescriptors have Dart peers that must be collected on the UI thread. + // However, closures in MakeCopyable below capture the descriptor. The + // captures of copyable closures may be collected on any of the thread + // participating in task execution. + // + // To avoid this issue, we resort to manually reference counting the + // descriptor. Since all task flows invoke the `result` callback, the raw + // descriptor is retained in the beginning and released in the `result` + // callback. + // + // `ImageDecoder::Decode` itself is invoked on the UI thread, so the + // collection of the smart pointer from which we obtained the raw descriptor + // is fine in this scope. + auto raw_descriptor = descriptor_ref_ptr.get(); + raw_descriptor->AddRef(); + FML_DCHECK(callback); FML_DCHECK(runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); // Always service the callback (and cleanup the descriptor) on the UI thread. - auto result = [callback, descriptor, ui_runner = runners_.GetUITaskRunner()]( - SkiaGPUObject image, - fml::tracing::TraceFlow flow) { - ui_runner->PostTask( - fml::MakeCopyable([callback, descriptor, image = std::move(image), - flow = std::move(flow)]() mutable { - // We are going to terminate the trace flow here. Flows cannot - // terminate without a base trace. Add one explicitly. - TRACE_EVENT0("flutter", "ImageDecodeCallback"); - flow.End(); - callback(std::move(image)); - })); - }; - - if (!descriptor->data() || descriptor->data()->size() == 0) { + auto result = + [callback, raw_descriptor, ui_runner = runners_.GetUITaskRunner()]( + SkiaGPUObject image, fml::tracing::TraceFlow flow) { + ui_runner->PostTask(fml::MakeCopyable( + [callback, raw_descriptor, image = std::move(image), + flow = std::move(flow)]() mutable { + // We are going to terminate the trace flow here. Flows cannot + // terminate without a base trace. Add one explicitly. + TRACE_EVENT0("flutter", "ImageDecodeCallback"); + flow.End(); + callback(std::move(image)); + raw_descriptor->Release(); + })); + }; + + if (!raw_descriptor->data() || raw_descriptor->data()->size() == 0) { result({}, std::move(flow)); return; } concurrent_task_runner_->PostTask( - fml::MakeCopyable([descriptor, // + fml::MakeCopyable([raw_descriptor, // io_manager = io_manager_, // io_runner = runners_.GetIOTaskRunner(), // result, // @@ -258,16 +275,15 @@ void ImageDecoder::Decode(fml::RefPtr descriptor, // Step 1: Decompress the image. // On Worker. - auto decompressed = - descriptor->is_compressed() - ? ImageFromCompressedData(std::move(descriptor), // - target_width, // - target_height, // - flow) - : ImageFromDecompressedData(std::move(descriptor), // - target_width, // - target_height, // - flow); + auto decompressed = raw_descriptor->is_compressed() + ? ImageFromCompressedData(raw_descriptor, // + target_width, // + target_height, // + flow) + : ImageFromDecompressedData(raw_descriptor, // + target_width, // + target_height, // + flow); if (!decompressed) { FML_LOG(ERROR) << "Could not decompress image."; @@ -283,7 +299,8 @@ void ImageDecoder::Decode(fml::RefPtr descriptor, std::move(flow)]() mutable { if (!io_manager) { FML_LOG(ERROR) << "Could not acquire IO manager."; - return result({}, std::move(flow)); + result({}, std::move(flow)); + return; } // If the IO manager does not have a resource context, the caller diff --git a/lib/ui/painting/image_decoder.h b/lib/ui/painting/image_decoder.h index dbcf4701d04ad..e5f2fba6df1ad 100644 --- a/lib/ui/painting/image_decoder.h +++ b/lib/ui/painting/image_decoder.h @@ -61,7 +61,7 @@ class ImageDecoder { FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoder); }; -sk_sp ImageFromCompressedData(fml::RefPtr descriptor, +sk_sp ImageFromCompressedData(ImageDescriptor* descriptor, uint32_t target_width, uint32_t target_height, const fml::tracing::TraceFlow& flow); diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 0a6dfbaed4dea..2e37af243f254 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -556,10 +556,10 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto descriptor = fml::MakeRefCounted(std::move(data), std::move(codec)); - ASSERT_EQ( - ImageFromCompressedData(descriptor, 6, 2, fml::tracing::TraceFlow("")) - ->dimensions(), - SkISize::Make(6, 2)); + ASSERT_EQ(ImageFromCompressedData(descriptor.get(), 6, 2, + fml::tracing::TraceFlow("")) + ->dimensions(), + SkISize::Make(6, 2)); } TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { @@ -574,8 +574,8 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { ASSERT_EQ(SkISize::Make(600, 200), image->dimensions()); auto decode = [descriptor](uint32_t target_width, uint32_t target_height) { - return ImageFromCompressedData(descriptor, target_width, target_height, - fml::tracing::TraceFlow("")); + return ImageFromCompressedData(descriptor.get(), target_width, + target_height, fml::tracing::TraceFlow("")); }; auto expected_data = OpenFixtureAsSkData("Horizontal.png");