Pass SurfaceFrame to SubmitFrame (#18709)

ci/licenses_golden/licenses_flutter

@@ -616,6 +616,8 @@ FILE: ../../../flutter/shell/common/skia_event_tracer_impl.cc
 FILE: ../../../flutter/shell/common/skia_event_tracer_impl.h
 FILE: ../../../flutter/shell/common/surface.cc
 FILE: ../../../flutter/shell/common/surface.h
+FILE: ../../../flutter/shell/common/surface_frame.cc
+FILE: ../../../flutter/shell/common/surface_frame.h
 FILE: ../../../flutter/shell/common/switches.cc
 FILE: ../../../flutter/shell/common/switches.h
 FILE: ../../../flutter/shell/common/thread_host.cc

flow/BUILD.gn

@@ -73,6 +73,7 @@ source_set("flow") {
   deps = [
     "//flutter/common",
     "//flutter/fml",
+    "//flutter/shell/common:surface_frame",
     "//third_party/skia",
   ]
 

flow/embedded_views.cc

@@ -7,12 +7,10 @@
 namespace flutter {
 
 bool ExternalViewEmbedder::SubmitFrame(GrContext* context,
-                                       SkCanvas* background_canvas) {
-  return false;
+                                       std::unique_ptr<SurfaceFrame> frame) {
+  return frame->Submit();
 };
 
-void ExternalViewEmbedder::FinishFrame(){};
-
 void MutatorsStack::PushClipRect(const SkRect& rect) {
   std::shared_ptr<Mutator> element = std::make_shared<Mutator>(rect);
   vector_.push_back(element);

flow/embedded_views.h

@@ -9,6 +9,7 @@
 
 #include "flutter/fml/memory/ref_counted.h"
 #include "flutter/fml/raster_thread_merger.h"
+#include "flutter/shell/common/surface_frame.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "third_party/skia/include/core/SkPoint.h"
@@ -248,10 +249,13 @@ class ExternalViewEmbedder {
   // Must be called on the UI thread.
   virtual SkCanvas* CompositeEmbeddedView(int view_id) = 0;
 
-  virtual bool SubmitFrame(GrContext* context, SkCanvas* background_canvas);
-
-  // This is called after submitting the embedder frame and the surface frame.
-  virtual void FinishFrame();
+  // Implementers must submit the frame by calling frame.Submit().
+  //
+  // This method can mutate the root Skia canvas before submitting the frame.
+  //
+  // It can also allocate frames for overlay surfaces to compose hybrid views.
+  virtual bool SubmitFrame(GrContext* context,
+                           std::unique_ptr<SurfaceFrame> frame);
 
   // This should only be called after |SubmitFrame|.
   // This method provides the embedder a way to do additional tasks after

shell/common/BUILD.gn

@@ -120,6 +120,7 @@ source_set("common") {
   ]
 
   deps = [
+    ":surface_frame",
     "//flutter/assets",
     "//flutter/common",
     "//flutter/flow",
@@ -141,6 +142,18 @@ source_set("common") {
   public_configs = [ "//flutter:config" ]
 }
 
+source_set("surface_frame") {
+  sources = [
+    "surface_frame.cc",
+    "surface_frame.h",
+  ]
+
+  deps = [
+    "//flutter/fml",
+    "//third_party/skia",
+  ]
+}
+
 template("shell_host_executable") {
   executable(target_name) {
     testonly = true

shell/common/rasterizer.cc

@@ -421,14 +421,9 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
       return raster_status;
     }
     if (external_view_embedder != nullptr) {
+      FML_DCHECK(!frame->IsSubmitted());
       external_view_embedder->SubmitFrame(surface_->GetContext(),
-                                          root_surface_canvas);
-      // The external view embedder may mutate the root surface canvas while
-      // submitting the frame.
-      // Therefore, submit the final frame after asking the external view
-      // embedder to submit the frame.
-      frame->Submit();
-      external_view_embedder->FinishFrame();
+                                          std::move(frame));
     } else {
       frame->Submit();
     }

shell/common/shell_test_external_view_embedder.cc

@@ -33,14 +33,12 @@ SkCanvas* ShellTestExternalViewEmbedder::CompositeEmbeddedView(int view_id) {
 }
 
 // |ExternalViewEmbedder|
-bool ShellTestExternalViewEmbedder::SubmitFrame(GrContext* context,
-                                                SkCanvas* background_canvas) {
-  return true;
+bool ShellTestExternalViewEmbedder::SubmitFrame(
+    GrContext* context,
+    std::unique_ptr<SurfaceFrame> frame) {
+  return frame->Submit();
 }
 
-// |ExternalViewEmbedder|
-void ShellTestExternalViewEmbedder::FinishFrame() {}
-
 // |ExternalViewEmbedder|
 void ShellTestExternalViewEmbedder::EndFrame(
     fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {

shell/common/shell_test_external_view_embedder.h

@@ -49,10 +49,8 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder {
   SkCanvas* CompositeEmbeddedView(int view_id) override;
 
   // |ExternalViewEmbedder|
-  bool SubmitFrame(GrContext* context, SkCanvas* background_canvas) override;
-
-  // |ExternalViewEmbedder|
-  void FinishFrame() override;
+  bool SubmitFrame(GrContext* context,
+                   std::unique_ptr<SurfaceFrame> frame) override;
 
   // |ExternalViewEmbedder|
   void EndFrame(

shell/common/surface.cc

@@ -4,58 +4,8 @@
 
 #include "flutter/shell/common/surface.h"
 
-#include "flutter/fml/logging.h"
-#include "third_party/skia/include/core/SkSurface.h"
-
 namespace flutter {
 
-SurfaceFrame::SurfaceFrame(sk_sp<SkSurface> surface,
-                           bool supports_readback,
-                           const SubmitCallback& submit_callback)
-    : submitted_(false),
-      surface_(surface),
-      supports_readback_(supports_readback),
-      submit_callback_(submit_callback) {
-  FML_DCHECK(submit_callback_);
-}
-
-SurfaceFrame::~SurfaceFrame() {
-  if (submit_callback_ && !submitted_) {
-    // Dropping without a Submit.
-    submit_callback_(*this, nullptr);
-  }
-}
-
-bool SurfaceFrame::Submit() {
-  if (submitted_) {
-    return false;
-  }
-
-  submitted_ = PerformSubmit();
-
-  return submitted_;
-}
-
-SkCanvas* SurfaceFrame::SkiaCanvas() {
-  return surface_ != nullptr ? surface_->getCanvas() : nullptr;
-}
-
-sk_sp<SkSurface> SurfaceFrame::SkiaSurface() const {
-  return surface_;
-}
-
-bool SurfaceFrame::PerformSubmit() {
-  if (submit_callback_ == nullptr) {
-    return false;
-  }
-
-  if (submit_callback_(*this, SkiaCanvas())) {
-    return true;
-  }
-
-  return false;
-}
-
 Surface::Surface() = default;
 
 Surface::~Surface() = default;

shell/common/surface.h

@@ -10,42 +10,10 @@
 #include "flutter/flow/compositor_context.h"
 #include "flutter/flow/embedded_views.h"
 #include "flutter/fml/macros.h"
-#include "third_party/skia/include/core/SkCanvas.h"
+#include "flutter/shell/common/surface_frame.h"
 
 namespace flutter {
 
-/// Represents a Frame that has been fully configured for the underlying client
-/// rendering API. A frame may only be submitted once.
-class SurfaceFrame {
- public:
-  using SubmitCallback =
-      std::function<bool(const SurfaceFrame& surface_frame, SkCanvas* canvas)>;
-
-  SurfaceFrame(sk_sp<SkSurface> surface,
-               bool supports_readback,
-               const SubmitCallback& submit_callback);
-
-  ~SurfaceFrame();
-
-  bool Submit();
-
-  SkCanvas* SkiaCanvas();
-
-  sk_sp<SkSurface> SkiaSurface() const;
-
-  bool supports_readback() { return supports_readback_; }
-
- private:
-  bool submitted_;
-  sk_sp<SkSurface> surface_;
-  bool supports_readback_;
-  SubmitCallback submit_callback_;
-
-  bool PerformSubmit();
-
-  FML_DISALLOW_COPY_AND_ASSIGN(SurfaceFrame);
-};
-
 /// Abstract Base Class that represents where we will be rendering content.
 class Surface {
  public:

shell/common/surface_frame.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/shell/common/surface_frame.h"
+
+#include "flutter/fml/logging.h"
+
+namespace flutter {
+
+SurfaceFrame::SurfaceFrame(sk_sp<SkSurface> surface,
+                           bool supports_readback,
+                           const SubmitCallback& submit_callback)
+    : surface_(surface),
+      supports_readback_(supports_readback),
+      submit_callback_(submit_callback) {
+  FML_DCHECK(submit_callback_);
+}
+
+SurfaceFrame::~SurfaceFrame() {
+  if (submit_callback_ && !submitted_) {
+    // Dropping without a Submit.
+    submit_callback_(*this, nullptr);
+  }
+}
+
+bool SurfaceFrame::Submit() {
+  if (submitted_) {
+    return false;
+  }
+
+  submitted_ = PerformSubmit();
+
+  return submitted_;
+}
+
+bool SurfaceFrame::IsSubmitted() const {
+  return submitted_;
+}
+
+SkCanvas* SurfaceFrame::SkiaCanvas() {
+  return surface_ != nullptr ? surface_->getCanvas() : nullptr;
+}
+
+sk_sp<SkSurface> SurfaceFrame::SkiaSurface() const {
+  return surface_;
+}
+
+bool SurfaceFrame::PerformSubmit() {
+  if (submit_callback_ == nullptr) {
+    return false;
+  }
+
+  if (submit_callback_(*this, SkiaCanvas())) {
+    return true;
+  }
+
+  return false;
+}
+
+}  // namespace flutter

shell/common/surface_frame.h

+// 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_SHELL_COMMON_SURFACE_FRAME_H_
+#define FLUTTER_SHELL_COMMON_SURFACE_FRAME_H_
+
+#include <memory>
+
+#include "flutter/fml/macros.h"
+#include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkSurface.h"
+
+namespace flutter {
+
+/// Represents a Frame that has been fully configured for the underlying client
+/// rendering API. A frame may only be submitted once.
+class SurfaceFrame {
+ public:
+  using SubmitCallback =
+      std::function<bool(const SurfaceFrame& surface_frame, SkCanvas* canvas)>;
+
+  SurfaceFrame(sk_sp<SkSurface> surface,
+               bool supports_readback,
+               const SubmitCallback& submit_callback);
+
+  ~SurfaceFrame();
+
+  bool Submit();
+
+  bool IsSubmitted() const;
+
+  SkCanvas* SkiaCanvas();
+
+  sk_sp<SkSurface> SkiaSurface() const;
+
+  bool supports_readback() { return supports_readback_; }
+
+ private:
+  bool submitted_ = false;
+  sk_sp<SkSurface> surface_;
+  bool supports_readback_;
+  SubmitCallback submit_callback_;
+
+  bool PerformSubmit();
+
+  FML_DISALLOW_COPY_AND_ASSIGN(SurfaceFrame);
+};
+
+}  // namespace flutter
+
+#endif  // FLUTTER_SHELL_COMMON_SURFACE_FRAME_H_

shell/platform/android/external_view_embedder/external_view_embedder.cc

@@ -40,17 +40,18 @@ std::vector<SkCanvas*> AndroidExternalViewEmbedder::GetCurrentCanvases() {
 }
 
 // |ExternalViewEmbedder|
-bool AndroidExternalViewEmbedder::SubmitFrame(GrContext* context,
-                                              SkCanvas* background_canvas) {
+bool AndroidExternalViewEmbedder::SubmitFrame(
+    GrContext* context,
+    std::unique_ptr<SurfaceFrame> frame) {
   // TODO(egarciad): Implement hybrid composition.
   // https://github.com/flutter/flutter/issues/55270
   TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::SubmitFrame");
   for (size_t i = 0; i < composition_order_.size(); i++) {
     int64_t view_id = composition_order_[i];
-    background_canvas->drawPicture(
+    frame->SkiaCanvas()->drawPicture(
         picture_recorders_[view_id]->finishRecordingAsPicture());
   }
-  return true;
+  return frame->Submit();
 }
 
 // |ExternalViewEmbedder|
@@ -83,11 +84,6 @@ void AndroidExternalViewEmbedder::CancelFrame() {
   ClearFrame();
 }
 
-// |ExternalViewEmbedder|
-void AndroidExternalViewEmbedder::FinishFrame() {
-  ClearFrame();
-}
-
 // |ExternalViewEmbedder|
 void AndroidExternalViewEmbedder::EndFrame(
     fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {}

shell/platform/android/external_view_embedder/external_view_embedder.h

@@ -25,7 +25,8 @@ class AndroidExternalViewEmbedder : public ExternalViewEmbedder {
   std::vector<SkCanvas*> GetCurrentCanvases() override;
 
   // |ExternalViewEmbedder|
-  bool SubmitFrame(GrContext* context, SkCanvas* background_canvas) override;
+  bool SubmitFrame(GrContext* context,
+                   std::unique_ptr<SurfaceFrame> frame) override;
 
   // |ExternalViewEmbedder|
   PostPrerollResult PostPrerollAction(
@@ -42,9 +43,6 @@ class AndroidExternalViewEmbedder : public ExternalViewEmbedder {
   // |ExternalViewEmbedder|
   void CancelFrame() override;
 
-  // |ExternalViewEmbedder|
-  void FinishFrame() override;
-
   // |ExternalViewEmbedder|
   void EndFrame(
       fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) override;

shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc

@@ -37,17 +37,6 @@ TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) {
   ASSERT_TRUE(embedder->CompositeEmbeddedView(1) != nullptr);
 }
 
-TEST(AndroidExternalViewEmbedder, FinishFrame) {
-  auto embedder = new AndroidExternalViewEmbedder();
-
-  embedder->PrerollCompositeEmbeddedView(
-      0, std::make_unique<EmbeddedViewParams>());
-  embedder->FinishFrame();
-
-  auto canvases = embedder->GetCurrentCanvases();
-  ASSERT_EQ(0UL, canvases.size());
-}
-
 TEST(AndroidExternalViewEmbedder, CancelFrame) {
   auto embedder = new AndroidExternalViewEmbedder();
 

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

@@ -470,7 +470,7 @@ SkRect FlutterPlatformViewsController::GetPlatformViewRect(int view_id) {
 
 bool FlutterPlatformViewsController::SubmitFrame(GrContext* gr_context,
                                                  std::shared_ptr<IOSContext> ios_context,
-                                                 SkCanvas* background_canvas) {
+                                                 std::unique_ptr<SurfaceFrame> frame) {
   if (merge_threads_) {
     // Threads are about to be merged, we drop everything from this frame
     // and possibly resubmit the same layer tree in the next frame.
@@ -488,6 +488,8 @@ bool FlutterPlatformViewsController::SubmitFrame(GrContext* gr_context,
 
   DisposeViews();
 
+  SkCanvas* background_canvas = frame->SkiaCanvas();
+
   // Resolve all pending GPU operations before allocating a new surface.
   background_canvas->flush();
   // Clipping the background canvas before drawing the picture recorders requires to
@@ -569,6 +571,8 @@ bool FlutterPlatformViewsController::SubmitFrame(GrContext* gr_context,
   // Reset the composition order, so next frame starts empty.
   composition_order_.clear();
 
+  did_submit &= frame->Submit();
+
   return did_submit;
 }
 

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h

@@ -163,7 +163,7 @@ class FlutterPlatformViewsController {
 
   bool SubmitFrame(GrContext* gr_context,
                    std::shared_ptr<IOSContext> ios_context,
-                   SkCanvas* background_canvas);
+                   std::unique_ptr<SurfaceFrame> frame);
 
   // Invoked at the very end of a frame.
   // After invoking this method, nothing should happen on the current TaskRunner during the same

shell/platform/darwin/ios/ios_surface.h

@@ -78,10 +78,7 @@ class IOSSurface : public ExternalViewEmbedder {
   SkCanvas* CompositeEmbeddedView(int view_id) override;
 
   // |ExternalViewEmbedder|
-  bool SubmitFrame(GrContext* context, SkCanvas* background_canvas) override;
-
-  // |ExternalViewEmbedder|
-  void FinishFrame() override;
+  bool SubmitFrame(GrContext* context, std::unique_ptr<SurfaceFrame> frame) override;
 
   // |ExternalViewEmbedder|
   void EndFrame(fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) override;

shell/platform/darwin/ios/ios_surface.mm

@@ -132,11 +132,16 @@ SkCanvas* IOSSurface::CompositeEmbeddedView(int view_id) {
 }
 
 // |ExternalViewEmbedder|
-bool IOSSurface::SubmitFrame(GrContext* context, SkCanvas* background_canvas) {
+bool IOSSurface::SubmitFrame(GrContext* context, std::unique_ptr<SurfaceFrame> frame) {
   TRACE_EVENT0("flutter", "IOSSurface::SubmitFrame");
   FML_CHECK(platform_views_controller_ != nullptr);
   bool submitted =
-      platform_views_controller_->SubmitFrame(std::move(context), ios_context_, background_canvas);
+      platform_views_controller_->SubmitFrame(std::move(context), ios_context_, std::move(frame));
+
+  if (submitted) {
+    TRACE_EVENT0("flutter", "IOSSurface::DidSubmitFrame");
+    [CATransaction commit];
+  }
   return submitted;
 }
 
@@ -147,9 +152,4 @@ void IOSSurface::EndFrame(fml::RefPtr<fml::RasterThreadMerger> raster_thread_mer
   return platform_views_controller_->EndFrame(raster_thread_merger);
 }
 
-// |ExternalViewEmbedder|
-void IOSSurface::FinishFrame() {
-  TRACE_EVENT0("flutter", "IOSSurface::DidSubmitFrame");
-  [CATransaction commit];
-}
 }  // namespace flutter

shell/platform/embedder/embedder_external_view_embedder.cc

@@ -129,8 +129,9 @@ static FlutterBackingStoreConfig MakeBackingStoreConfig(
 }
 
 // |ExternalViewEmbedder|
-bool EmbedderExternalViewEmbedder::SubmitFrame(GrContext* context,
-                                               SkCanvas* background_canvas) {
+bool EmbedderExternalViewEmbedder::SubmitFrame(
+    GrContext* context,
+    std::unique_ptr<SurfaceFrame> frame) {
   auto [matched_render_targets, pending_keys] =
       render_target_cache_.GetExistingTargetsInCache(pending_views_);
 
@@ -263,10 +264,7 @@ bool EmbedderExternalViewEmbedder::SubmitFrame(GrContext* context,
                                            std::move(render_target.second));
   }
 
-  return true;
+  return frame->Submit();
 }
 
-// |ExternalViewEmbedder|
-void EmbedderExternalViewEmbedder::FinishFrame() {}
-
 }  // namespace flutter

shell/platform/embedder/embedder_external_view_embedder.h

@@ -89,10 +89,8 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder {
   SkCanvas* CompositeEmbeddedView(int view_id) override;
 
   // |ExternalViewEmbedder|
-  bool SubmitFrame(GrContext* context, SkCanvas* background_canvas) override;
-
-  // |ExternalViewEmbedder|
-  void FinishFrame() override;
+  bool SubmitFrame(GrContext* context,
+                   std::unique_ptr<SurfaceFrame> frame) override;
 
   // |ExternalViewEmbedder|
   SkCanvas* GetRootCanvas() override;