EngineLayer::dispose

ci/licenses_golden/licenses_flutter

@@ -313,6 +313,7 @@ FILE: ../../../flutter/lib/ui/compositing/scene.cc
 FILE: ../../../flutter/lib/ui/compositing/scene.h
 FILE: ../../../flutter/lib/ui/compositing/scene_builder.cc
 FILE: ../../../flutter/lib/ui/compositing/scene_builder.h
+FILE: ../../../flutter/lib/ui/compositing/scene_builder_unittests.cc
 FILE: ../../../flutter/lib/ui/compositing/scene_host.cc
 FILE: ../../../flutter/lib/ui/compositing/scene_host.h
 FILE: ../../../flutter/lib/ui/dart_runtime_hooks.cc

lib/ui/BUILD.gn

@@ -159,7 +159,7 @@ source_set("ui") {
       "compositing/scene_host.h",
     ]
 
-    deps += [
+    public_deps += [
       "$fuchsia_sdk_root/pkg:async-cpp",
       "//flutter/shell/platform/fuchsia/dart-pkg/fuchsia",
       "//flutter/shell/platform/fuchsia/dart-pkg/zircon",
@@ -201,6 +201,7 @@ if (enable_unittests) {
     public_configs = [ "//flutter:export_dynamic_symbols" ]
 
     sources = [
+      "compositing/scene_builder_unittests.cc",
       "painting/image_dispose_unittests.cc",
       "painting/image_encoding_unittests.cc",
       "painting/image_generator_registry_unittests.cc",

lib/ui/compositing.dart

@@ -57,9 +57,19 @@ class Scene extends NativeFieldWrapperClass2 {
 // passed as `oldLayer` to `pushTransform`. This is achieved by having one
 // concrete subclass of this class per push method.
 abstract class _EngineLayerWrapper implements EngineLayer {
-  _EngineLayerWrapper._(this._nativeLayer);
+  _EngineLayerWrapper._(EngineLayer nativeLayer) : _nativeLayer = nativeLayer;
 
-  EngineLayer _nativeLayer;
+  EngineLayer? _nativeLayer;
+
+  @override
+  void dispose() {
+    assert(_nativeLayer != null, 'Object disposed');
+    _nativeLayer!.dispose();
+    assert(() {
+      _nativeLayer = null;
+      return true;
+    }());
+  }
 
   // Children of this layer.
   //
@@ -230,6 +240,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
       if (layer == null) {
         return true;
       }
+      assert(layer._nativeLayer != null, 'Object disposed');
       layer._debugCheckNotUsedAsOldLayer();
       assert(_debugCheckUsedOnce(layer, 'oldLayer in $methodName'));
       layer._debugWasUsedAsOldLayer = true;
@@ -638,6 +649,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
     assert(() {
       final _EngineLayerWrapper layer = retainedLayer as _EngineLayerWrapper;
 
+      assert(layer._nativeLayer != null);
+
       void recursivelyCheckChildrenUsedOnce(_EngineLayerWrapper parentLayer) {
         _debugCheckUsedOnce(parentLayer, 'retained layer');
         parentLayer._debugCheckNotUsedAsOldLayer();
@@ -655,7 +668,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
     }());
 
     final _EngineLayerWrapper wrapper = retainedLayer as _EngineLayerWrapper;
-    _addRetained(wrapper._nativeLayer);
+    _addRetained(wrapper._nativeLayer!);
   }
 
   void _addRetained(EngineLayer retainedLayer) native 'SceneBuilder_addRetained';

lib/ui/compositing/scene.cc

@@ -62,6 +62,7 @@ Scene::Scene(std::shared_ptr<flutter::Layer> rootLayer,
 Scene::~Scene() {}
 
 void Scene::dispose() {
+  layer_tree_.reset();
   ClearDartWrapper();
 }
 

lib/ui/compositing/scene_builder.cc

@@ -347,9 +347,10 @@ void SceneBuilder::setCheckerboardOffscreenLayers(bool checkerboard) {
 void SceneBuilder::build(Dart_Handle scene_handle) {
   FML_DCHECK(layer_stack_.size() >= 1);
 
-  Scene::create(scene_handle, layer_stack_[0], rasterizer_tracing_threshold_,
-                checkerboard_raster_cache_images_,
-                checkerboard_offscreen_layers_);
+  Scene::create(
+      scene_handle, std::move(layer_stack_[0]), rasterizer_tracing_threshold_,
+      checkerboard_raster_cache_images_, checkerboard_offscreen_layers_);
+  layer_stack_.clear();
   ClearDartWrapper();  // may delete this object.
 }
 

lib/ui/compositing/scene_builder.h

@@ -132,6 +132,10 @@ class SceneBuilder : public RefCountedDartWrappable<SceneBuilder> {
 
   void build(Dart_Handle scene_handle);
 
+  const std::vector<std::shared_ptr<ContainerLayer>>& layer_stack() {
+    return layer_stack_;
+  }
+
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 
  private:

lib/ui/compositing/scene_builder_unittests.cc

@@ -0,0 +1,158 @@
+// 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/lib/ui/compositing/scene_builder.h"
+
+#include <memory>
+
+#include "flutter/common/task_runners.h"
+#include "flutter/fml/synchronization/waitable_event.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, SceneBuilderBuildAndSceneDisposeReleasesLayerStack) {
+  auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
+
+  // prevent ClearDartWrapper() from deleting the scene builder.
+  SceneBuilder* retained_scene_builder;
+  Scene* retained_scene;
+
+  auto validate_builder_has_layers =
+      [&retained_scene_builder](Dart_NativeArguments args) {
+        auto handle = Dart_GetNativeArgument(args, 0);
+        intptr_t peer = 0;
+        Dart_Handle result = Dart_GetNativeInstanceField(
+            handle, tonic::DartWrappable::kPeerIndex, &peer);
+        ASSERT_FALSE(Dart_IsError(result));
+        retained_scene_builder = reinterpret_cast<SceneBuilder*>(peer);
+        retained_scene_builder->AddRef();
+        ASSERT_TRUE(retained_scene_builder);
+        ASSERT_EQ(retained_scene_builder->layer_stack().size(), 2ul);
+      };
+
+  auto validate_builder_has_no_layers =
+      [&retained_scene_builder](Dart_NativeArguments args) {
+        ASSERT_EQ(retained_scene_builder->layer_stack().size(), 0ul);
+        retained_scene_builder->Release();
+        retained_scene_builder = nullptr;
+      };
+
+  auto capture_scene = [&retained_scene](Dart_NativeArguments args) {
+    auto handle = Dart_GetNativeArgument(args, 0);
+    intptr_t peer = 0;
+    Dart_Handle result = Dart_GetNativeInstanceField(
+        handle, tonic::DartWrappable::kPeerIndex, &peer);
+    ASSERT_FALSE(Dart_IsError(result));
+    retained_scene = reinterpret_cast<Scene*>(peer);
+    retained_scene->AddRef();
+    ASSERT_TRUE(retained_scene);
+  };
+
+  auto validate_scene_has_no_layers =
+      [message_latch, &retained_scene](Dart_NativeArguments args) {
+        EXPECT_FALSE(retained_scene->takeLayerTree());
+        retained_scene->Release();
+        retained_scene = nullptr;
+        message_latch->Signal();
+      };
+
+  Settings settings = CreateSettingsForFixture();
+  TaskRunners task_runners("test",                  // label
+                           GetCurrentTaskRunner(),  // platform
+                           CreateNewThread(),       // raster
+                           CreateNewThread(),       // ui
+                           CreateNewThread()        // io
+  );
+
+  AddNativeCallback("ValidateBuilderHasLayers",
+                    CREATE_NATIVE_ENTRY(validate_builder_has_layers));
+  AddNativeCallback("ValidateBuilderHasNoLayers",
+                    CREATE_NATIVE_ENTRY(validate_builder_has_no_layers));
+
+  AddNativeCallback("CaptureScene", CREATE_NATIVE_ENTRY(capture_scene));
+  AddNativeCallback("ValidateSceneHasNoLayers",
+                    CREATE_NATIVE_ENTRY(validate_scene_has_no_layers));
+
+  std::unique_ptr<Shell> shell =
+      CreateShell(std::move(settings), std::move(task_runners));
+
+  ASSERT_TRUE(shell->IsSetup());
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("validateSceneBuilderAndScene");
+
+  shell->RunEngine(std::move(configuration), [](auto result) {
+    ASSERT_EQ(result, Engine::RunStatus::Success);
+  });
+
+  message_latch->Wait();
+  DestroyShell(std::move(shell), std::move(task_runners));
+}
+
+TEST_F(ShellTest, EngineLayerDisposeReleasesReference) {
+  auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
+
+  std::shared_ptr<ContainerLayer> root_layer;
+
+  auto capture_root_layer = [&root_layer](Dart_NativeArguments args) {
+    auto handle = Dart_GetNativeArgument(args, 0);
+    intptr_t peer = 0;
+    Dart_Handle result = Dart_GetNativeInstanceField(
+        handle, tonic::DartWrappable::kPeerIndex, &peer);
+    ASSERT_FALSE(Dart_IsError(result));
+    SceneBuilder* scene_builder = reinterpret_cast<SceneBuilder*>(peer);
+    ASSERT_TRUE(scene_builder);
+    root_layer = scene_builder->layer_stack()[0];
+    ASSERT_TRUE(root_layer);
+  };
+
+  auto validate_layer_tree_counts = [&root_layer](Dart_NativeArguments args) {
+    // One for the EngineLayer, one for our pointer in the test.
+    EXPECT_EQ(root_layer->layers().front().use_count(), 2);
+  };
+
+  auto validate_engine_layer_dispose =
+      [message_latch, &root_layer](Dart_NativeArguments args) {
+        // Just one for our pointer now.
+        EXPECT_EQ(root_layer->layers().front().use_count(), 1);
+        root_layer.reset();
+        message_latch->Signal();
+      };
+
+  Settings settings = CreateSettingsForFixture();
+  TaskRunners task_runners("test",                  // label
+                           GetCurrentTaskRunner(),  // platform
+                           CreateNewThread(),       // raster
+                           CreateNewThread(),       // ui
+                           CreateNewThread()        // io
+  );
+
+  AddNativeCallback("CaptureRootLayer",
+                    CREATE_NATIVE_ENTRY(capture_root_layer));
+  AddNativeCallback("ValidateLayerTreeCounts",
+                    CREATE_NATIVE_ENTRY(validate_layer_tree_counts));
+  AddNativeCallback("ValidateEngineLayerDispose",
+                    CREATE_NATIVE_ENTRY(validate_engine_layer_dispose));
+
+  std::unique_ptr<Shell> shell =
+      CreateShell(std::move(settings), std::move(task_runners));
+
+  ASSERT_TRUE(shell->IsSetup());
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("validateEngineLayerDispose");
+
+  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

lib/ui/fixtures/ui_test.dart

@@ -9,6 +9,37 @@ import 'dart:ui';
 
 void main() {}
 
+@pragma('vm:entry-point')
+void validateSceneBuilderAndScene() {
+  final SceneBuilder builder = SceneBuilder();
+  builder.pushOffset(10, 10);
+  _validateBuilderHasLayers(builder);
+  final Scene scene = builder.build();
+  _validateBuilderHasNoLayers();
+  _captureScene(scene);
+  scene.dispose();
+  _validateSceneHasNoLayers();
+}
+_validateBuilderHasLayers(SceneBuilder builder) native 'ValidateBuilderHasLayers';
+_validateBuilderHasNoLayers() native 'ValidateBuilderHasNoLayers';
+_captureScene(Scene scene) native 'CaptureScene';
+_validateSceneHasNoLayers() native 'ValidateSceneHasNoLayers';
+
+@pragma('vm:entry-point')
+void validateEngineLayerDispose() {
+  final SceneBuilder builder = SceneBuilder();
+  final EngineLayer layer = builder.pushOffset(10, 10);
+  _captureRootLayer(builder);
+  final Scene scene = builder.build();
+  scene.dispose();
+  _validateLayerTreeCounts();
+  layer.dispose();
+  _validateEngineLayerDispose();
+}
+_captureRootLayer(SceneBuilder sceneBuilder) native 'CaptureRootLayer';
+_validateLayerTreeCounts() native 'ValidateLayerTreeCounts';
+_validateEngineLayerDispose() native 'ValidateEngineLayerDispose';
+
 @pragma('vm:entry-point')
 Future<void> createSingleFrameCodec() async {
   final ImmutableBuffer buffer = await ImmutableBuffer.fromUint8List(Uint8List.fromList(List<int>.filled(4, 100)));

lib/ui/painting.dart

@@ -2233,6 +2233,20 @@ class EngineLayer extends NativeFieldWrapperClass2 {
   /// or extended directly.
   @pragma('vm:entry-point')
   EngineLayer._();
+
+  /// Release the resources used by this object. The object is no longer usable
+  /// after this method is called.
+  ///
+  /// EngineLayers indirectly retain platform specific graphics resources. Some
+  /// of these resources, such as images, may be memory intensive. It is
+  /// important to dispose of EngineLayer objects that will no longer be used as
+  /// soon as possible to avoid retaining these resources until the next
+  /// garbage collection.
+  ///
+  /// Once this EngineLayer is disposed, it is no longer eligible for use as a
+  /// retained layer, and must not be passed as an `oldLayer` to any of the
+  /// [SceneBuilder] methods which accept that parameter.
+  void dispose() native 'EngineLayer_dispose';
 }
 
 /// A complex, one-dimensional subset of a plane.

lib/ui/painting/engine_layer.cc

@@ -4,6 +4,7 @@
 
 #include "flutter/lib/ui/painting/engine_layer.h"
 
+#include "flutter/lib/ui/ui_dart_state.h"
 #include "third_party/tonic/converter/dart_converter.h"
 #include "third_party/tonic/dart_args.h"
 #include "third_party/tonic/dart_binding_macros.h"
@@ -13,22 +14,30 @@ using tonic::ToDart;
 
 namespace flutter {
 
+IMPLEMENT_WRAPPERTYPEINFO(ui, EngineLayer);
+
+#define FOR_EACH_BINDING(V) V(EngineLayer, dispose)
+
+DART_BIND_ALL(EngineLayer, FOR_EACH_BINDING)
+
 EngineLayer::EngineLayer(std::shared_ptr<flutter::ContainerLayer> layer)
     : layer_(layer) {}
 
 EngineLayer::~EngineLayer() = default;
 
 size_t EngineLayer::GetAllocationSize() const {
+  // TODO(dnfield): Remove this when scene disposal changes land in the
+  // framework. https://github.com/flutter/flutter/issues/81514
+
   // Provide an approximation of the total memory impact of this object to the
   // Dart GC.  The ContainerLayer may hold references to a tree of other layers,
   // which in turn may contain Skia objects.
   return 3000;
 };
 
-IMPLEMENT_WRAPPERTYPEINFO(ui, EngineLayer);
-
-#define FOR_EACH_BINDING(V)  // nothing to bind
-
-DART_BIND_ALL(EngineLayer, FOR_EACH_BINDING)
+void EngineLayer::dispose() {
+  layer_.reset();
+  ClearDartWrapper();
+}
 
 }  // namespace flutter

lib/ui/painting/engine_layer.h

@@ -32,6 +32,8 @@ class EngineLayer : public RefCountedDartWrappable<EngineLayer> {
 
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 
+  void dispose();
+
   std::shared_ptr<flutter::ContainerLayer> Layer() const { return layer_; }
 
  private:

lib/ui/painting/image.cc

@@ -44,6 +44,8 @@ Dart_Handle CanvasImage::toByteData(int format, Dart_Handle callback) {
 }
 
 void CanvasImage::dispose() {
+  // TODO(dnfield): Remove the hint freed delegate once Picture disposal is in
+  // the framework https://github.com/flutter/flutter/issues/81514
   auto hint_freed_delegate = UIDartState::Current()->GetHintFreedDelegate();
   if (hint_freed_delegate) {
     hint_freed_delegate->HintFreed(GetAllocationSize());