Revert "Avoid a copy in EncodeImage (#19504)" (#19739)

This reverts commit f9acd080.

ci/licenses_golden/licenses_flutter

@@ -337,7 +337,6 @@ FILE: ../../../flutter/lib/ui/painting/image_decoder.h
 FILE: ../../../flutter/lib/ui/painting/image_decoder_unittests.cc
 FILE: ../../../flutter/lib/ui/painting/image_encoding.cc
 FILE: ../../../flutter/lib/ui/painting/image_encoding.h
-FILE: ../../../flutter/lib/ui/painting/image_encoding_unittests.cc
 FILE: ../../../flutter/lib/ui/painting/image_filter.cc
 FILE: ../../../flutter/lib/ui/painting/image_filter.h
 FILE: ../../../flutter/lib/ui/painting/image_shader.cc

lib/ui/BUILD.gn

@@ -178,7 +178,6 @@ if (enable_unittests) {
     public_configs = [ "//flutter:export_dynamic_symbols" ]
 
     sources = [
-      "painting/image_encoding_unittests.cc",
       "painting/vertices_unittests.cc",
       "window/pointer_data_packet_converter_unittests.cc",
     ]

lib/ui/fixtures/ui_test.dart

@@ -44,24 +44,3 @@ void frameCallback(FrameInfo info) {
 @pragma('vm:entry-point')
 void messageCallback(dynamic data) {
 }
-
-
-// Draw a circle on a Canvas that has a PictureRecorder. Take the image from
-// the PictureRecorder, and encode it as png. Check that the png data is
-// backed by an external Uint8List.
-@pragma('vm:entry-point')
-Future<void> encodeImageProducesExternalUint8List() async {
-  final PictureRecorder pictureRecorder = PictureRecorder();
-  final Canvas canvas = Canvas(pictureRecorder);
-  final Paint paint = Paint()
-    ..color = Color.fromRGBO(255, 255, 255, 1.0)
-    ..style = PaintingStyle.fill;
-  final Offset c = Offset(50.0, 50.0);
-  canvas.drawCircle(c, 25.0, paint);
-  final Picture picture = pictureRecorder.endRecording();
-  final Image image = await picture.toImage(100, 100);
-  _encodeImage(image, ImageByteFormat.png.index, _validateExternal);
-}
-void _encodeImage(Image i, int format, void Function(Uint8List result))
-  native 'EncodeImage';
-void _validateExternal(Uint8List result) native 'ValidateExternal';

lib/ui/painting.dart

@@ -1600,20 +1600,17 @@ class Image extends NativeFieldWrapperClass2 {
   /// The number of image pixels along the image's vertical axis.
   int get height native 'Image_height';
 
-  /// Converts the [Image] object into a read-only byte array.
+  /// Converts the [Image] object into a byte array.
   ///
   /// The [format] argument specifies the format in which the bytes will be
   /// returned.
   ///
   /// Returns a future that completes with the binary image data or an error
-  /// if encoding fails. Note that attempting to write to the returned
-  /// [ByteData] will result in an [UnsupportedError] exception.
+  /// if encoding fails.
   Future<ByteData?> toByteData({ImageByteFormat format = ImageByteFormat.rawRgba}) {
     return _futurize((_Callback<ByteData> callback) {
       return _toByteData(format.index, (Uint8List? encoded) {
-        // [encoded] wraps a read-only SkData buffer, so we wrap it here in
-        // an [UnmodifiableByteDataView].
-        callback(UnmodifiableByteDataView(encoded!.buffer.asByteData()));
+        callback(encoded!.buffer.asByteData());
       });
     });
   }

lib/ui/painting/image_encoding.cc

@@ -35,13 +35,6 @@ enum ImageByteFormat {
   kPNG,
 };
 
-void FinalizeSkData(void* isolate_callback_data,
-                    Dart_WeakPersistentHandle handle,
-                    void* peer) {
-  SkData* buffer = reinterpret_cast<SkData*>(peer);
-  buffer->unref();
-}
-
 void InvokeDataCallback(std::unique_ptr<DartPersistentValue> callback,
                         sk_sp<SkData> buffer) {
   std::shared_ptr<tonic::DartState> dart_state = callback->dart_state().lock();
@@ -51,15 +44,11 @@ void InvokeDataCallback(std::unique_ptr<DartPersistentValue> callback,
   tonic::DartState::Scope scope(dart_state);
   if (!buffer) {
     DartInvoke(callback->value(), {Dart_Null()});
-    return;
+  } else {
+    Dart_Handle dart_data = tonic::DartConverter<tonic::Uint8List>::ToDart(
+        buffer->bytes(), buffer->size());
+    DartInvoke(callback->value(), {dart_data});
   }
-  // SkData are generally read-only.
-  void* bytes = const_cast<void*>(buffer->data());
-  const intptr_t length = buffer->size();
-  void* peer = reinterpret_cast<void*>(buffer.release());
-  Dart_Handle dart_data = Dart_NewExternalTypedDataWithFinalizer(
-      Dart_TypedData_kUint8, bytes, length, peer, length, FinalizeSkData);
-  DartInvoke(callback->value(), {dart_data});
 }
 
 sk_sp<SkImage> ConvertToRasterUsingResourceContext(
@@ -233,10 +222,9 @@ void EncodeImageAndInvokeDataCallback(
   auto encode_task = [callback_task = std::move(callback_task), format,
                       ui_task_runner](sk_sp<SkImage> raster_image) {
     sk_sp<SkData> encoded = EncodeImage(std::move(raster_image), format);
-    ui_task_runner->PostTask([callback_task = std::move(callback_task),
-                              encoded = std::move(encoded)]() mutable {
-      callback_task(std::move(encoded));
-    });
+    ui_task_runner->PostTask(
+        [callback_task = std::move(callback_task),
+         encoded = std::move(encoded)] { callback_task(encoded); });
   };
 
   ConvertImageToRaster(std::move(image), encode_task, raster_task_runner,

lib/ui/painting/image_encoding_unittests.cc

-// Copyright 2013 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "flutter/common/task_runners.h"
-#include "flutter/fml/synchronization/waitable_event.h"
-#include "flutter/lib/ui/painting/image_encoding.h"
-#include "flutter/runtime/dart_vm.h"
-#include "flutter/shell/common/shell_test.h"
-#include "flutter/shell/common/thread_host.h"
-#include "flutter/testing/testing.h"
-
-namespace flutter {
-namespace testing {
-
-TEST_F(ShellTest, EncodeImageGivesExternalTypedData) {
-  fml::AutoResetWaitableEvent message_latch;
-
-  auto nativeEncodeImage = [&](Dart_NativeArguments args) {
-    auto image_handle = Dart_GetNativeArgument(args, 0);
-    auto format_handle = Dart_GetNativeArgument(args, 1);
-    auto callback_handle = Dart_GetNativeArgument(args, 2);
-
-    intptr_t peer = 0;
-    Dart_Handle result = Dart_GetNativeInstanceField(
-        image_handle, tonic::DartWrappable::kPeerIndex, &peer);
-    ASSERT_FALSE(Dart_IsError(result));
-    CanvasImage* canvas_image = reinterpret_cast<CanvasImage*>(peer);
-
-    int64_t format = -1;
-    result = Dart_IntegerToInt64(format_handle, &format);
-    ASSERT_FALSE(Dart_IsError(result));
-
-    result = EncodeImage(canvas_image, format, callback_handle);
-    ASSERT_TRUE(Dart_IsNull(result));
-  };
-
-  auto nativeValidateExternal = [&](Dart_NativeArguments args) {
-    auto handle = Dart_GetNativeArgument(args, 0);
-
-    auto typed_data_type = Dart_GetTypeOfExternalTypedData(handle);
-    EXPECT_EQ(typed_data_type, Dart_TypedData_kUint8);
-
-    message_latch.Signal();
-  };
-
-  Settings settings = CreateSettingsForFixture();
-  TaskRunners task_runners("test",                  // label
-                           GetCurrentTaskRunner(),  // platform
-                           CreateNewThread(),       // raster
-                           CreateNewThread(),       // ui
-                           CreateNewThread()        // io
-  );
-
-  AddNativeCallback("EncodeImage", CREATE_NATIVE_ENTRY(nativeEncodeImage));
-  AddNativeCallback("ValidateExternal",
-                    CREATE_NATIVE_ENTRY(nativeValidateExternal));
-
-  std::unique_ptr<Shell> shell =
-      CreateShell(std::move(settings), std::move(task_runners));
-
-  ASSERT_TRUE(shell->IsSetup());
-  auto configuration = RunConfiguration::InferFromSettings(settings);
-  configuration.SetEntrypoint("encodeImageProducesExternalUint8List");
-
-  shell->RunEngine(std::move(configuration), [&](auto result) {
-    ASSERT_EQ(result, Engine::RunStatus::Success);
-  });
-
-  message_latch.Wait();
-  DestroyShell(std::move(shell), std::move(task_runners));
-}
-
-}  // namespace testing
-}  // namespace flutter