[Impeller] Started throwing errors if dart:ui/Image.toByteData fails (f…

…lutter#46738)

issue: flutter/flutter#135245

This is a first step.  Next we'll implement a retry.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

ci/licenses_golden/licenses_flutter

@@ -1617,6 +1617,7 @@ ORIGIN: ../../../flutter/fml/shared_thread_merger.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/fml/shared_thread_merger.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/fml/size.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/fml/status.h + ../../../flutter/LICENSE
+ORIGIN: ../../../flutter/fml/status_or.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/fml/string_conversion.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/fml/string_conversion.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/fml/synchronization/atomic_object.h + ../../../flutter/LICENSE
@@ -4377,6 +4378,7 @@ FILE: ../../../flutter/fml/shared_thread_merger.cc
 FILE: ../../../flutter/fml/shared_thread_merger.h
 FILE: ../../../flutter/fml/size.h
 FILE: ../../../flutter/fml/status.h
+FILE: ../../../flutter/fml/status_or.h
 FILE: ../../../flutter/fml/string_conversion.cc
 FILE: ../../../flutter/fml/string_conversion.h
 FILE: ../../../flutter/fml/synchronization/atomic_object.h

fml/status_or.h

@@ -0,0 +1,81 @@
+// 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.
+
+#ifndef FLUTTER_FML_STATUS_OR_H_
+#define FLUTTER_FML_STATUS_OR_H_
+
+#include <optional>
+
+#include "flutter/fml/status.h"
+
+namespace fml {
+
+// TODO(https://github.com/flutter/flutter/issues/134741): Replace with
+// absl::StatusOr.
+/// Represents a union type of an object of type `T` and an fml::Status.
+///
+/// This is often used as a replacement for C++ exceptions where a function that
+/// could fail may return an error or a result. These are typically used for
+/// errors that are meant to be recovered from. If there is no recovery
+/// available `FML_CHECK` is more appropriate.
+///
+/// Example:
+/// StatusOr<int> div(int n, int d) {
+///   if (d == 0) {
+///     return Status(StatusCode::kFailedPrecondition, "div by zero");
+///   }
+///   return n / d;
+/// }
+template <typename T>
+class StatusOr {
+ public:
+  StatusOr(const T& value) : status_(), value_(value) {}
+  StatusOr(const Status& status) : status_(status), value_() {}
+
+  StatusOr(const StatusOr&) = default;
+  StatusOr(StatusOr&&) = default;
+
+  StatusOr& operator=(const StatusOr&) = default;
+  StatusOr& operator=(StatusOr&&) = default;
+
+  StatusOr& operator=(const T& value) {
+    status_ = Status();
+    value_ = value;
+    return *this;
+  }
+
+  StatusOr& operator=(const T&& value) {
+    status_ = Status();
+    value_ = std::move(value);
+    return *this;
+  }
+
+  StatusOr& operator=(const Status& value) {
+    status_ = value;
+    value_ = std::nullopt;
+    return *this;
+  }
+
+  const Status& status() const { return status_; }
+
+  bool ok() const { return status_.ok(); }
+
+  const T& value() const {
+    FML_CHECK(status_.ok());
+    return value_.value();
+  }
+
+  T& value() {
+    FML_CHECK(status_.ok());
+    return value_.value();
+  }
+
+ private:
+  Status status_;
+  std::optional<T> value_;
+};
+
+}  // namespace fml
+
+#endif

lib/ui/fixtures/ui_test.dart

@@ -310,7 +310,7 @@ Future<void> encodeImageProducesExternalUint8List() async {
   canvas.drawCircle(c, 25.0, paint);
   final Picture picture = pictureRecorder.endRecording();
   final Image image = await picture.toImage(100, 100);
-  _encodeImage(image, ImageByteFormat.png.index, (Uint8List result) {
+  _encodeImage(image, ImageByteFormat.png.index, (Uint8List result, String? error) {
     // The buffer should be non-null and writable.
     result[0] = 0;
     // The buffer should be external typed data.
@@ -319,9 +319,33 @@ Future<void> encodeImageProducesExternalUint8List() async {
 }
 
 @pragma('vm:external-name', 'EncodeImage')
-external void _encodeImage(Image i, int format, void Function(Uint8List result));
+external void _encodeImage(Image i, int format, void Function(Uint8List result, String? error));
 @pragma('vm:external-name', 'ValidateExternal')
 external void _validateExternal(Uint8List result);
+@pragma('vm:external-name', 'ValidateError')
+external void _validateError(String? error);
+@pragma('vm:external-name', 'TurnOffGPU')
+external void _turnOffGPU();
+
+@pragma('vm:entry-point')
+Future<void> toByteDataWithoutGPU() 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);
+  _turnOffGPU();
+  try {
+    ByteData? byteData = await image.toByteData();
+    _validateError(null);
+  } catch (ex) {
+    _validateError(ex.toString());
+  }
+}
 
 @pragma('vm:entry-point')
 Future<void> pumpImage() async {

lib/ui/painting.dart

@@ -1936,16 +1936,20 @@ base class _Image extends NativeFieldWrapperClass1 {
   external int get height;
 
   Future<ByteData?> toByteData({ImageByteFormat format = ImageByteFormat.rawRgba}) {
-    return _futurize((_Callback<ByteData> callback) {
-      return _toByteData(format.index, (Uint8List? encoded) {
-        callback(encoded!.buffer.asByteData());
+    return _futurizeWithError((_CallbackWithError<ByteData?> callback) {
+      return _toByteData(format.index, (Uint8List? encoded, String? error) {
+        if (error == null && encoded != null) {
+          callback(encoded.buffer.asByteData(), null);
+        } else {
+          callback(null, error);
+        }
       });
     });
   }
 
   /// Returns an error message on failure, null on success.
   @Native<Handle Function(Pointer<Void>, Int32, Handle)>(symbol: 'Image::toByteData')
-  external String? _toByteData(int format, _Callback<Uint8List?> callback);
+  external String? _toByteData(int format, void Function(Uint8List?, String?) callback);
 
   bool _disposed = false;
   void dispose() {
@@ -6884,12 +6888,19 @@ base class _NativeImageDescriptor extends NativeFieldWrapperClass1 implements Im
 /// Generic callback signature, used by [_futurize].
 typedef _Callback<T> = void Function(T result);
 
+/// Generic callback signature, used by [_futurizeWithError].
+typedef _CallbackWithError<T> = void Function(T result, String? error);
+
 /// Signature for a method that receives a [_Callback].
 ///
 /// Return value should be null on success, and a string error message on
 /// failure.
 typedef _Callbacker<T> = String? Function(_Callback<T?> callback);
 
+/// Signature for a method that receives a [_CallbackWithError].
+/// See also: [_Callbacker]
+typedef _CallbackerWithError<T> = String? Function(_CallbackWithError<T?> callback);
+
 // Converts a method that receives a value-returning callback to a method that
 // returns a Future.
 //
@@ -6941,6 +6952,31 @@ Future<T> _futurize<T>(_Callbacker<T> callbacker) {
   return completer.future;
 }
 
+/// A variant of `_futurize` that can communicate specific errors.
+Future<T> _futurizeWithError<T>(_CallbackerWithError<T> callbacker) {
+  final Completer<T> completer = Completer<T>.sync();
+  // If the callback synchronously throws an error, then synchronously
+  // rethrow that error instead of adding it to the completer. This
+  // prevents the Zone from receiving an uncaught exception.
+  bool isSync = true;
+  final String? error = callbacker((T? t, String? error) {
+    if (t != null) {
+      completer.complete(t);
+    } else {
+      if (isSync) {
+        throw Exception(error ?? 'operation failed');
+      } else {
+        completer.completeError(Exception(error ?? 'operation failed'));
+      }
+    }
+  });
+  isSync = false;
+  if (error != null) {
+    throw Exception(error);
+  }
+  return completer.future;
+}
+
 /// An exception thrown by [Canvas.drawImage] and related methods when drawing
 /// an [Image] created via [Picture.toImageSync] that is in an invalid state.
 ///

lib/ui/painting/image_encoding.cc

@@ -11,6 +11,7 @@
 #include "flutter/common/task_runners.h"
 #include "flutter/fml/build_config.h"
 #include "flutter/fml/make_copyable.h"
+#include "flutter/fml/status_or.h"
 #include "flutter/fml/trace_event.h"
 #include "flutter/lib/ui/painting/image.h"
 #if IMPELLER_SUPPORTS_RENDERING
@@ -40,25 +41,27 @@ void FinalizeSkData(void* isolate_callback_data, void* peer) {
 }
 
 void InvokeDataCallback(std::unique_ptr<DartPersistentValue> callback,
-                        sk_sp<SkData> buffer) {
+                        fml::StatusOr<sk_sp<SkData>>&& buffer) {
   std::shared_ptr<tonic::DartState> dart_state = callback->dart_state().lock();
   if (!dart_state) {
     return;
   }
   tonic::DartState::Scope scope(dart_state);
-  if (!buffer) {
-    DartInvoke(callback->value(), {Dart_Null()});
+  if (!buffer.ok()) {
+    std::string error_copy(buffer.status().message());
+    Dart_Handle dart_string = ToDart(error_copy);
+    DartInvoke(callback->value(), {Dart_Null(), dart_string});
     return;
   }
   // Skia will not modify the buffer, and it is backed by memory that is
   // read/write, so Dart can be given direct access to the buffer through an
   // external Uint8List.
-  void* bytes = const_cast<void*>(buffer->data());
-  const intptr_t length = buffer->size();
-  void* peer = reinterpret_cast<void*>(buffer.release());
+  void* bytes = const_cast<void*>(buffer.value()->data());
+  const intptr_t length = buffer.value()->size();
+  void* peer = reinterpret_cast<void*>(buffer.value().release());
   Dart_Handle dart_data = Dart_NewExternalTypedDataWithFinalizer(
       Dart_TypedData_kUint8, bytes, length, peer, length, FinalizeSkData);
-  DartInvoke(callback->value(), {dart_data});
+  DartInvoke(callback->value(), {dart_data, Dart_Null()});
 }
 
 sk_sp<SkData> CopyImageByteData(const sk_sp<SkImage>& raster_image,
@@ -110,21 +113,30 @@ void EncodeImageAndInvokeDataCallback(
     const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch,
     const std::shared_ptr<impeller::Context>& impeller_context,
     bool is_impeller_enabled) {
-  auto callback_task = fml::MakeCopyable(
-      [callback = std::move(callback)](sk_sp<SkData> encoded) mutable {
+  auto callback_task =
+      fml::MakeCopyable([callback = std::move(callback)](
+                            fml::StatusOr<sk_sp<SkData>>&& encoded) mutable {
         InvokeDataCallback(std::move(callback), std::move(encoded));
       });
   // The static leak checker gets confused by the use of fml::MakeCopyable in
   // EncodeImage.
   // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks)
-  auto encode_task = [callback_task = std::move(callback_task), format,
-                      ui_task_runner](const sk_sp<SkImage>& raster_image) {
-    sk_sp<SkData> encoded = EncodeImage(raster_image, format);
-    ui_task_runner->PostTask([callback_task = callback_task,
-                              encoded = std::move(encoded)]() mutable {
-      callback_task(std::move(encoded));
-    });
-  };
+  auto encode_task =
+      [callback_task = std::move(callback_task), format,
+       ui_task_runner](const fml::StatusOr<sk_sp<SkImage>>& raster_image) {
+        if (raster_image.ok()) {
+          sk_sp<SkData> encoded = EncodeImage(raster_image.value(), format);
+          ui_task_runner->PostTask([callback_task = callback_task,
+                                    encoded = std::move(encoded)]() mutable {
+            callback_task(std::move(encoded));
+          });
+        } else {
+          ui_task_runner->PostTask([callback_task = callback_task,
+                                    raster_image = raster_image]() mutable {
+            callback_task(raster_image.status());
+          });
+        }
+      };
 
   FML_DCHECK(image);
 #if IMPELLER_SUPPORTS_RENDERING

lib/ui/painting/image_encoding_impeller.cc

@@ -58,12 +58,15 @@ sk_sp<SkImage> ConvertBufferToSkImage(
 
 void DoConvertImageToRasterImpeller(
     const sk_sp<DlImage>& dl_image,
-    std::function<void(sk_sp<SkImage>)> encode_task,
+    std::function<void(fml::StatusOr<sk_sp<SkImage>>)> encode_task,
     const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch,
     const std::shared_ptr<impeller::Context>& impeller_context) {
   is_gpu_disabled_sync_switch->Execute(
       fml::SyncSwitch::Handlers()
-          .SetIfTrue([&encode_task] { encode_task(nullptr); })
+          .SetIfTrue([&encode_task] {
+            encode_task(
+                fml::Status(fml::StatusCode::kUnavailable, "GPU unavailable."));
+          })
           .SetIfFalse([&dl_image, &encode_task, &impeller_context] {
             ImageEncodingImpeller::ConvertDlImageToSkImage(
                 dl_image, std::move(encode_task), impeller_context);
@@ -74,34 +77,34 @@ void DoConvertImageToRasterImpeller(
 
 void ImageEncodingImpeller::ConvertDlImageToSkImage(
     const sk_sp<DlImage>& dl_image,
-    std::function<void(sk_sp<SkImage>)> encode_task,
+    std::function<void(fml::StatusOr<sk_sp<SkImage>>)> encode_task,
     const std::shared_ptr<impeller::Context>& impeller_context) {
   auto texture = dl_image->impeller_texture();
 
   if (impeller_context == nullptr) {
-    FML_LOG(ERROR) << "Impeller context was null.";
-    encode_task(nullptr);
+    encode_task(fml::Status(fml::StatusCode::kFailedPrecondition,
+                            "Impeller context was null."));
     return;
   }
 
   if (texture == nullptr) {
-    FML_LOG(ERROR) << "Image was null.";
-    encode_task(nullptr);
+    encode_task(
+        fml::Status(fml::StatusCode::kFailedPrecondition, "Image was null."));
     return;
   }
 
   auto dimensions = dl_image->dimensions();
   auto color_type = ToSkColorType(texture->GetTextureDescriptor().format);
 
   if (dimensions.isEmpty()) {
-    FML_LOG(ERROR) << "Image dimensions were empty.";
-    encode_task(nullptr);
+    encode_task(fml::Status(fml::StatusCode::kFailedPrecondition,
+                            "Image dimensions were empty."));
     return;
   }
 
   if (!color_type.has_value()) {
-    FML_LOG(ERROR) << "Failed to get color type from pixel format.";
-    encode_task(nullptr);
+    encode_task(fml::Status(fml::StatusCode::kUnimplemented,
+                            "Failed to get color type from pixel format."));
     return;
   }
 
@@ -121,7 +124,7 @@ void ImageEncodingImpeller::ConvertDlImageToSkImage(
                      encode_task = std::move(encode_task)](
                         impeller::CommandBuffer::Status status) {
     if (status != impeller::CommandBuffer::Status::kCompleted) {
-      encode_task(nullptr);
+      encode_task(fml::Status(fml::StatusCode::kUnknown, ""));
       return;
     }
     auto sk_image = ConvertBufferToSkImage(buffer, color_type, dimensions);
@@ -135,14 +138,14 @@ void ImageEncodingImpeller::ConvertDlImageToSkImage(
 
 void ImageEncodingImpeller::ConvertImageToRaster(
     const sk_sp<DlImage>& dl_image,
-    std::function<void(sk_sp<SkImage>)> encode_task,
+    std::function<void(fml::StatusOr<sk_sp<SkImage>>)> encode_task,
     const fml::RefPtr<fml::TaskRunner>& raster_task_runner,
     const fml::RefPtr<fml::TaskRunner>& io_task_runner,
     const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch,
     const std::shared_ptr<impeller::Context>& impeller_context) {
   auto original_encode_task = std::move(encode_task);
   encode_task = [original_encode_task = std::move(original_encode_task),
-                 io_task_runner](sk_sp<SkImage> image) mutable {
+                 io_task_runner](fml::StatusOr<sk_sp<SkImage>> image) mutable {
     fml::TaskRunner::RunNowOrPostTask(
         io_task_runner,
         [original_encode_task = std::move(original_encode_task),