From 10c2ef8b402b8e09e6fe4fb85b07e4ec2c97b038 Mon Sep 17 00:00:00 2001
From: Jason Simmons <jason-simmons@users.noreply.github.com>
Date: Tue, 23 Apr 2024 10:24:19 -0700
Subject: [PATCH] [Impeller] Do not call std::forward on the serialized
 arguments in the canvas recorder (flutter/engine#52307)

std::forward has move semantics and can not safely be called twice on the same arguments.

Also fix CanvasRecorder's resolution of Canvas::Save, which has a default parameter value.
---
 engine/src/flutter/impeller/aiks/canvas_recorder.h  |  5 +++--
 .../impeller/aiks/canvas_recorder_unittests.cc      | 13 ++++++++++---
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/engine/src/flutter/impeller/aiks/canvas_recorder.h b/engine/src/flutter/impeller/aiks/canvas_recorder.h
index 1e38927ca2d..ec4bec26bb7 100644
--- a/engine/src/flutter/impeller/aiks/canvas_recorder.h
+++ b/engine/src/flutter/impeller/aiks/canvas_recorder.h
@@ -89,7 +89,7 @@ class CanvasRecorder {
       -> decltype((std::declval<Canvas>().*
                    canvasMethod)(std::forward<Args>(args)...)) {
     // Serialize each argument
-    (serializer_.Write(std::forward<Args>(args)), ...);
+    (serializer_.Write(args), ...);
     serializer_.Write(op);
     return (canvas_.*canvasMethod)(std::forward<Args>(args)...);
   }
@@ -110,7 +110,8 @@ class CanvasRecorder {
   //////////////////////////////////////////////////////////////////////////////
 
   void Save(uint32_t total_content_depth = Canvas::kMaxDepth) {
-    return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(Save),
+    void (Canvas::*save_method)(uint32_t) = &Canvas::Save;
+    return ExecuteAndSerialize(CanvasRecorderOp::kSave, save_method,
                                total_content_depth);
   }
 
diff --git a/engine/src/flutter/impeller/aiks/canvas_recorder_unittests.cc b/engine/src/flutter/impeller/aiks/canvas_recorder_unittests.cc
index a18e092aab9..6cd1d18693a 100644
--- a/engine/src/flutter/impeller/aiks/canvas_recorder_unittests.cc
+++ b/engine/src/flutter/impeller/aiks/canvas_recorder_unittests.cc
@@ -12,7 +12,7 @@ class Serializer {
  public:
   void Write(CanvasRecorderOp op) { last_op_ = op; }
 
-  void Write(const Paint& paint) {}
+  void Write(const Paint& paint) { last_paint_ = paint; }
 
   void Write(const std::optional<Rect> optional_rect) {}
 
@@ -59,6 +59,7 @@ class Serializer {
   void Write(const ContentBoundsPromise& promise) {}
 
   CanvasRecorderOp last_op_;
+  Paint last_paint_;
 };
 }  // namespace
 
@@ -152,8 +153,11 @@ TEST(CanvasRecorder, DrawPath) {
 
 TEST(CanvasRecorder, DrawPaint) {
   CanvasRecorder<Serializer> recorder;
-  recorder.DrawPaint(Paint());
+  Paint paint;
+  paint.color = Color::Red();
+  recorder.DrawPaint(paint);
   ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::kDrawPaint);
+  ASSERT_EQ(recorder.GetSerializer().last_paint_.color, paint.color);
 }
 
 TEST(CanvasRecorder, DrawLine) {
@@ -164,8 +168,11 @@ TEST(CanvasRecorder, DrawLine) {
 
 TEST(CanvasRecorder, DrawRect) {
   CanvasRecorder<Serializer> recorder;
-  recorder.DrawRect(Rect(), Paint());
+  Paint paint;
+  paint.color = Color::Blue();
+  recorder.DrawRect(Rect(), paint);
   ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::kDrawRect);
+  ASSERT_EQ(recorder.GetSerializer().last_paint_.color, paint.color);
 }
 
 TEST(CanvasRecorder, DrawOval) {