Reapply "Reland 2: Multiview Pipeline (flutter#47239)" (flutter#49238)

This reverts commit a379405.

flow/frame_timings.cc

@@ -254,8 +254,30 @@ const char* FrameTimingsRecorder::GetFrameNumberTraceArg() const {
   return frame_number_trace_arg_val_.c_str();
 }
 
+static const char* StateToString(FrameTimingsRecorder::State state) {
+#ifndef NDEBUG
+  switch (state) {
+    case FrameTimingsRecorder::State::kUninitialized:
+      return "kUninitialized";
+    case FrameTimingsRecorder::State::kVsync:
+      return "kVsync";
+    case FrameTimingsRecorder::State::kBuildStart:
+      return "kBuildStart";
+    case FrameTimingsRecorder::State::kBuildEnd:
+      return "kBuildEnd";
+    case FrameTimingsRecorder::State::kRasterStart:
+      return "kRasterStart";
+    case FrameTimingsRecorder::State::kRasterEnd:
+      return "kRasterEnd";
+  };
+  FML_UNREACHABLE();
+#endif
+  return "";
+}
+
 void FrameTimingsRecorder::AssertInState(State state) const {
-  FML_DCHECK(state_ == state);
+  FML_DCHECK(state_ == state) << "Expected state " << StateToString(state)
+                              << ", actual state " << StateToString(state_);
 }
 
 }  // namespace flutter

flow/frame_timings.h

@@ -31,6 +31,7 @@ class FrameTimingsRecorder {
  public:
   /// Various states that the recorder can be in. When created the recorder is
   /// in an unitialized state and transtions in sequential order of the states.
+  // After adding an item to this enum, modify StateToString accordingly.
   enum class State : uint32_t {
     kUninitialized,
     kVsync,
@@ -121,6 +122,8 @@ class FrameTimingsRecorder {
   ///
   /// Instead of adding a `GetState` method and asserting on the result, this
   /// method prevents other logic from relying on the state.
+  ///
+  /// In opt builds, this call is a no-op.
   void AssertInState(State state) const;
 
  private:

lib/ui/dart_ui.cc

@@ -98,7 +98,7 @@ typedef CanvasPath Path;
   V(NativeStringAttribute::initSpellOutStringAttribute, 3)            \
   V(PlatformConfigurationNativeApi::DefaultRouteName, 0)              \
   V(PlatformConfigurationNativeApi::ScheduleFrame, 0)                 \
-  V(PlatformConfigurationNativeApi::Render, 1)                        \
+  V(PlatformConfigurationNativeApi::Render, 2)                        \
   V(PlatformConfigurationNativeApi::UpdateSemantics, 1)               \
   V(PlatformConfigurationNativeApi::SetNeedsReportTimings, 1)         \
   V(PlatformConfigurationNativeApi::SetIsolateDebugName, 1)           \

lib/ui/painting/image_dispose_unittests.cc

@@ -5,6 +5,7 @@
 #define FML_USED_ON_EMBEDDER
 
 #include "flutter/common/task_runners.h"
+#include "flutter/fml/synchronization/count_down_latch.h"
 #include "flutter/fml/synchronization/waitable_event.h"
 #include "flutter/lib/ui/painting/canvas.h"
 #include "flutter/lib/ui/painting/image.h"
@@ -57,6 +58,10 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) {
   };
 
   Settings settings = CreateSettingsForFixture();
+  fml::CountDownLatch frame_latch{2};
+  settings.frame_rasterized_callback = [&frame_latch](const FrameTiming& t) {
+    frame_latch.CountDown();
+  };
   auto task_runner = CreateNewThread();
   TaskRunners task_runners("test",                  // label
                            GetCurrentTaskRunner(),  // platform
@@ -83,12 +88,15 @@ TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) {
   shell->RunEngine(std::move(configuration), [&](auto result) {
     ASSERT_EQ(result, Engine::RunStatus::Success);
   });
-
   message_latch_.Wait();
 
   ASSERT_TRUE(current_display_list_);
   ASSERT_TRUE(current_image_);
 
+  // Wait for 2 frames to be rasterized. The 2nd frame releases resources of the
+  // 1st frame.
+  frame_latch.Wait();
+
   // Force a drain the SkiaUnrefQueue. The engine does this normally as frames
   // pump, but we force it here to make the test more deterministic.
   message_latch_.Reset();

lib/ui/platform_dispatcher.dart

@@ -219,20 +219,20 @@ class PlatformDispatcher {
         'The implicit view ID is null, but the implicit view exists.');
     // Make sure [implicitView] never chages.
     assert(() {
-      if (_debugRecordedLastImplicitView) {
+      if (_recordedLastImplicitView) {
         assert(identical(_debugLastImplicitView, result),
           'The implicitView has changed:\n'
           'Last: $_debugLastImplicitView\nCurrent: $result');
       } else {
         _debugLastImplicitView = result;
-        _debugRecordedLastImplicitView = true;
+        _recordedLastImplicitView = true;
       }
       return true;
     }());
     return result;
   }
   FlutterView? _debugLastImplicitView;
-  bool _debugRecordedLastImplicitView = false;
+  bool _recordedLastImplicitView = false;
 
   /// A callback that is invoked whenever the [ViewConfiguration] of any of the
   /// [views] changes.
@@ -324,12 +324,12 @@ class PlatformDispatcher {
   // In release build, this variable is null, and therefore the calling rule is
   // not enforced. This is because the check might hurt cold startup delay;
   // see https://github.com/flutter/engine/pull/46919.
-  Set<FlutterView>? _debugRenderedViews;
+  Set<FlutterView>? _renderedViews;
   // A debug-only variable that temporarily stores the `_renderedViews` value
   // between `_beginFrame` and `_drawFrame`.
   //
   // In release build, this variable is null.
-  Set<FlutterView>? _debugRenderedViewsBetweenCallbacks;
+  Set<FlutterView>? _renderedViewsBetweenCallbacks;
 
   /// A callback invoked when any view begins a frame.
   ///
@@ -351,26 +351,20 @@ class PlatformDispatcher {
 
   // Called from the engine, via hooks.dart
   void _beginFrame(int microseconds) {
-    assert(_debugRenderedViews == null);
-    assert(_debugRenderedViewsBetweenCallbacks == null);
-    assert(() {
-      _debugRenderedViews = <FlutterView>{};
-      return true;
-    }());
+    assert(_renderedViews == null);
+    assert(_renderedViewsBetweenCallbacks == null);
+    _renderedViews = <FlutterView>{};
 
     _invoke1<Duration>(
       onBeginFrame,
       _onBeginFrameZone,
       Duration(microseconds: microseconds),
     );
 
-    assert(_debugRenderedViews != null);
-    assert(_debugRenderedViewsBetweenCallbacks == null);
-    assert(() {
-      _debugRenderedViewsBetweenCallbacks = _debugRenderedViews;
-      _debugRenderedViews = null;
-      return true;
-    }());
+    assert(_renderedViews != null);
+    assert(_renderedViewsBetweenCallbacks == null);
+    _renderedViewsBetweenCallbacks = _renderedViews;
+    _renderedViews = null;
   }
 
   /// A callback that is invoked for each frame after [onBeginFrame] has
@@ -388,22 +382,16 @@ class PlatformDispatcher {
 
   // Called from the engine, via hooks.dart
   void _drawFrame() {
-    assert(_debugRenderedViews == null);
-    assert(_debugRenderedViewsBetweenCallbacks != null);
-    assert(() {
-      _debugRenderedViews = _debugRenderedViewsBetweenCallbacks;
-      _debugRenderedViewsBetweenCallbacks = null;
-      return true;
-    }());
+    assert(_renderedViews == null);
+    assert(_renderedViewsBetweenCallbacks != null);
+    _renderedViews = _renderedViewsBetweenCallbacks;
+    _renderedViewsBetweenCallbacks = null;
 
     _invoke(onDrawFrame, _onDrawFrameZone);
 
-    assert(_debugRenderedViews != null);
-    assert(_debugRenderedViewsBetweenCallbacks == null);
-    assert(() {
-      _debugRenderedViews = null;
-      return true;
-    }());
+    assert(_renderedViews != null);
+    assert(_renderedViewsBetweenCallbacks == null);
+    _renderedViews = null;
   }
 
   /// A callback that is invoked when pointer data is available.

lib/ui/window.dart

@@ -373,21 +373,17 @@ class FlutterView {
   ///   painting.
   void render(Scene scene, {Size? size}) {
     // Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated
-    // by _debugRenderedViews being null) are ignored. See _debugRenderedViews.
+    // by _renderedViews being null) are ignored. See _renderedViews.
     // TODO(dkwingsmt): We should change this skip into an assertion.
     // https://github.com/flutter/flutter/issues/137073
-    bool validRender = true;
-    assert(() {
-      validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false;
-      return true;
-    }());
+    final bool validRender = platformDispatcher._renderedViews?.add(this) ?? false;
     if (validRender) {
-      _render(scene as _NativeScene, size?.width ?? physicalSize.width, size?.height ?? physicalSize.height);
+      _render(viewId, scene as _NativeScene, size?.width ?? physicalSize.width, size?.height ?? physicalSize.height);
     }
   }
 
-  @Native<Void Function(Pointer<Void>, Double, Double)>(symbol: 'PlatformConfigurationNativeApi::Render')
-  external static void _render(_NativeScene scene, double width, double height);
+  @Native<Void Function(Int64, Pointer<Void>, Double, Double)>(symbol: 'PlatformConfigurationNativeApi::Render')
+  external static void _render(int viewId, _NativeScene scene, double width, double height);
 
   /// Change the retained semantics data about this [FlutterView].
   ///

lib/ui/window/platform_configuration.cc

@@ -449,12 +449,13 @@ void PlatformConfiguration::CompletePlatformMessageResponse(
   response->Complete(std::make_unique<fml::DataMapping>(std::move(data)));
 }
 
-void PlatformConfigurationNativeApi::Render(Scene* scene,
+void PlatformConfigurationNativeApi::Render(int64_t view_id,
+                                            Scene* scene,
                                             double width,
                                             double height) {
   UIDartState::ThrowIfUIOperationsProhibited();
   UIDartState::Current()->platform_configuration()->client()->Render(
-      scene, width, height);
+      view_id, scene, width, height);
 }
 
 void PlatformConfigurationNativeApi::SetNeedsReportTimings(bool value) {

lib/ui/window/platform_configuration.h

@@ -69,7 +69,10 @@ class PlatformConfigurationClient {
   /// @brief      Updates the client's rendering on the GPU with the newly
   ///             provided Scene.
   ///
-  virtual void Render(Scene* scene, double width, double height) = 0;
+  virtual void Render(int64_t view_id,
+                      Scene* scene,
+                      double width,
+                      double height) = 0;
 
   //--------------------------------------------------------------------------
   /// @brief      Receives an updated semantics tree from the Framework.
@@ -557,7 +560,10 @@ class PlatformConfigurationNativeApi {
 
   static void ScheduleFrame();
 
-  static void Render(Scene* scene, double width, double height);
+  static void Render(int64_t view_id,
+                     Scene* scene,
+                     double width,
+                     double height);
 
   static void UpdateSemantics(SemanticsUpdate* update);