From 9623154706691acff4ffda32bbae9b0fae210fd0 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 28 Apr 2022 17:45:26 -0700 Subject: [PATCH 01/25] Add support for loading asset directly from ImmutableBuffer --- lib/ui/painting.dart | 68 ++++++++++++++++++++++++++ lib/ui/painting/immutable_buffer.cc | 46 +++++++++++++++++ lib/ui/painting/immutable_buffer.h | 13 +++++ lib/ui/window/platform_configuration.h | 7 +++ lib/web_ui/lib/painting.dart | 19 +++++++ runtime/runtime_controller.cc | 5 ++ runtime/runtime_controller.h | 4 ++ runtime/runtime_delegate.h | 3 ++ shell/common/engine.h | 4 +- shell/common/engine_unittests.cc | 1 + testing/dart/assets_test.dart | 17 +++++++ 11 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 testing/dart/assets_test.dart diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 2a33b976926b7..147933a81c92c 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2069,6 +2069,56 @@ Future instantiateImageCodec( ); } +/// Instantiates an image [Codec]. +/// +/// This method is a convenience wrapper around the [ImageDescriptor] API, and +/// using [ImageDescriptor] directly is preferred since it allows the caller to +/// make better determinations about how and whether to use the `targetWidth` +/// and `targetHeight` parameters. +/// +/// The [buffer] parameter is the binary image data (e.g a PNG or GIF binary data). +/// The data can be for either static or animated images. The following image +/// formats are supported: {@macro dart.ui.imageFormats} +/// +/// The [targetWidth] and [targetHeight] arguments specify the size of the +/// output image, in image pixels. If they are not equal to the intrinsic +/// dimensions of the image, then the image will be scaled after being decoded. +/// If the `allowUpscaling` parameter is not set to true, both dimensions will +/// be capped at the intrinsic dimensions of the image, even if only one of +/// them would have exceeded those intrinsic dimensions. If exactly one of these +/// two arguments is specified, then the aspect ratio will be maintained while +/// forcing the image to match the other given dimension. If neither is +/// specified, then the image maintains its intrinsic size. +/// +/// Scaling the image to larger than its intrinsic size should usually be +/// avoided, since it causes the image to use more memory than necessary. +/// Instead, prefer scaling the [Canvas] transform. If the image must be scaled +/// up, the `allowUpscaling` parameter must be set to true. +/// +/// The returned future can complete with an error if the image decoding has +/// failed. +Future instantiateImageCodecFromBuffer( + ImmutableBuffer buffer, { + int? targetWidth, + int? targetHeight, + bool allowUpscaling = true, +}) async { + final ImageDescriptor descriptor = await ImageDescriptor.encoded(buffer); + if (!allowUpscaling) { + if (targetWidth != null && targetWidth > descriptor.width) { + targetWidth = descriptor.width; + } + if (targetHeight != null && targetHeight > descriptor.height) { + targetHeight = descriptor.height; + } + } + buffer.dispose(); + return descriptor.instantiateCodec( + targetWidth: targetWidth, + targetHeight: targetHeight, + ); +} + /// Loads a single image frame from a byte array into an [Image] object. /// /// This is a convenience wrapper around [instantiateImageCodec]. Prefer using @@ -5521,8 +5571,26 @@ class ImmutableBuffer extends NativeFieldWrapperClass1 { instance._init(list, callback); }).then((_) => instance); } + + /// Create a buffer from the asset with key [assetKey]. + /// + /// Returns `null` if the asset does not exist. + static ImmutableBuffer? fromAsset(String assetKey) { + final ImmutableBuffer buffer = ImmutableBuffer._(0); + bool success = false; + buffer._initFromAsset(assetKey, (void result) { + success = true; + }); + if (!success) { + return null; + } + return buffer; + } + void _init(Uint8List list, _Callback callback) native 'ImmutableBuffer_init'; + String? _initFromAsset(String assetKey, _Callback callback) native 'ImmutableBuffer_initFromAsset'; + /// The length, in bytes, of the underlying data. final int length; diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index e4d3f630de72f..b2e3033658ffe 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -7,6 +7,7 @@ #include #include "flutter/lib/ui/ui_dart_state.h" +#include "flutter/lib/ui/window/platform_configuration.h" #include "third_party/tonic/converter/dart_converter.h" #include "third_party/tonic/dart_args.h" #include "third_party/tonic/dart_binding_macros.h" @@ -30,6 +31,9 @@ ImmutableBuffer::~ImmutableBuffer() {} void ImmutableBuffer::RegisterNatives(tonic::DartLibraryNatives* natives) { natives->Register({{"ImmutableBuffer_init", ImmutableBuffer::init, 3, true}, FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); + natives->Register({{"ImmutableBuffer_initFromAsset", + ImmutableBuffer::initFromAsset, 3, true}, + FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); } void ImmutableBuffer::init(Dart_NativeArguments args) { @@ -49,6 +53,48 @@ void ImmutableBuffer::init(Dart_NativeArguments args) { tonic::DartInvoke(callback_handle, {Dart_TypeVoid()}); } +void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { + UIDartState::ThrowIfUIOperationsProhibited(); + Dart_Handle callback_handle = Dart_GetNativeArgument(args, 2); + if (!Dart_IsClosure(callback_handle)) { + Dart_SetReturnValue(args, tonic::ToDart("Callback must be a function")); + return; + } + Dart_Handle asset_name_handle = Dart_GetNativeArgument(args, 1); + uint8_t* chars = nullptr; + intptr_t asset_length = 0; + Dart_Handle result = + Dart_StringToUTF8(asset_name_handle, &chars, &asset_length); + if (Dart_IsError(result)) { + Dart_SetReturnValue(args, tonic::ToDart("Asset must be valid UTF8")); + return; + } + Dart_Handle immutable_buffer = Dart_GetNativeArgument(args, 0); + + std::string asset_name = std::string{reinterpret_cast(chars), + static_cast(asset_length)}; + + std::shared_ptr asset_manager = UIDartState::Current() + ->platform_configuration() + ->client() + ->GetAssetManager(); + std::unique_ptr data = asset_manager->GetAsMapping(asset_name); + if (data == nullptr) { + Dart_SetReturnValue(args, tonic::ToDart("Asset not found")); + return; + } + + const void* bytes = static_cast(data->GetMapping()); + auto size = data->GetSize(); + void* peer = reinterpret_cast(data.release()); + SkData::ReleaseProc proc = [](const void* ptr, void* context) {}; + + auto sk_data = SkData::MakeWithProc(bytes, size, proc, peer); + auto buffer = fml::MakeRefCounted(sk_data); + buffer->AssociateWithDartWrapper(immutable_buffer); + tonic::DartInvoke(callback_handle, {Dart_TypeVoid()}); +} + size_t ImmutableBuffer::GetAllocationSize() const { return sizeof(ImmutableBuffer) + data_->size(); } diff --git a/lib/ui/painting/immutable_buffer.h b/lib/ui/painting/immutable_buffer.h index 6306dd99f0a73..1dd94a5ee920e 100644 --- a/lib/ui/painting/immutable_buffer.h +++ b/lib/ui/painting/immutable_buffer.h @@ -39,6 +39,19 @@ class ImmutableBuffer : public RefCountedDartWrappable { /// when the copy has completed. static void init(Dart_NativeArguments args); + /// Initializes a new ImmutableData from an asset matching a provided + /// asset string. + /// + /// The zero indexed argument is the caller that will be registered as the + /// Dart peer of the native ImmutableBuffer object. + /// + /// The first indexed argumented is a String corresponding to the asset + /// to load. + /// + /// The second indexed argument is expected to be a void callback to signal + /// when the copy has completed. + static void initFromAsset(Dart_NativeArguments args); + /// The length of the data in bytes. size_t length() const { FML_DCHECK(data_); diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index a8f3cf4ab3ac8..83a6f67caf1f8 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -11,6 +11,7 @@ #include #include +#include "flutter/assets/asset_manager.h" #include "flutter/fml/time/time_point.h" #include "flutter/lib/ui/semantics/semantics_update.h" #include "flutter/lib/ui/window/pointer_data_packet.h" @@ -98,6 +99,12 @@ class PlatformConfigurationClient { /// creation. virtual FontCollection& GetFontCollection() = 0; + + //-------------------------------------------------------------------------- + /// @brief Returns the current collection of assets available on the + /// platform. + virtual std::shared_ptr GetAssetManager() = 0; + //-------------------------------------------------------------------------- /// @brief Notifies this client of the name of the root isolate and its /// port when that isolate is launched, restarted (in the diff --git a/lib/web_ui/lib/painting.dart b/lib/web_ui/lib/painting.dart index 09c249115e193..656c9ba8c043d 100644 --- a/lib/web_ui/lib/painting.dart +++ b/lib/web_ui/lib/painting.dart @@ -484,6 +484,20 @@ Future instantiateImageCodec( } } +Future instantiateImageCodecFromBuffer( + ImmutableBuffer buffer, { + int? targetWidth, + int? targetHeight, + bool allowUpscaling = true, +}) async { + if (engine.useCanvasKit) { + return engine.skiaInstantiateImageCodec(buffer._list!, targetWidth, targetHeight); + } else { + final html.Blob blob = html.Blob([buffer._list!.buffer]); + return engine.HtmlBlobCodec(blob); + } +} + Future webOnlyInstantiateImageCodecFromUrl(Uri uri, {engine.WebOnlyImageCodecChunkCallback? chunkCallback}) { if (engine.useCanvasKit) { @@ -742,6 +756,11 @@ class ImmutableBuffer { return instance; } + static ImmutableBuffer? fromAsset(String assetKey) { + assert(false, 'ImmutableBuffer.fromAsset is not supported on the web.'); + return null; + } + Uint8List? _list; final int length; diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index e82f3391a5526..c865be32b2fcf 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -299,6 +299,11 @@ FontCollection& RuntimeController::GetFontCollection() { return client_.GetFontCollection(); } +// |PlatfromConfigurationClient| +std::shared_ptr RuntimeController::GetAssetManager() { + return client_.GetAssetManager(); +} + // |PlatformConfigurationClient| void RuntimeController::UpdateIsolateDescription(const std::string isolate_name, int64_t isolate_port) { diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 6f476cbe3dbf0..bcb9b4a2e85f5 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -8,6 +8,7 @@ #include #include +#include "flutter/assets/asset_manager.h" #include "flutter/common/task_runners.h" #include "flutter/flow/layers/layer_tree.h" #include "flutter/fml/macros.h" @@ -614,6 +615,9 @@ class RuntimeController : public PlatformConfigurationClient { // |PlatformConfigurationClient| FontCollection& GetFontCollection() override; + // |PlatformConfigurationClient| + std::shared_ptr GetAssetManager() override; + // |PlatformConfigurationClient| void UpdateIsolateDescription(const std::string isolate_name, int64_t isolate_port) override; diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index ed57722e532f9..540e47dd09ba4 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -8,6 +8,7 @@ #include #include +#include "flutter/assets/asset_manager.h" #include "flutter/flow/layers/layer_tree.h" #include "flutter/lib/ui/semantics/custom_accessibility_action.h" #include "flutter/lib/ui/semantics/semantics_node.h" @@ -33,6 +34,8 @@ class RuntimeDelegate { virtual FontCollection& GetFontCollection() = 0; + virtual std::shared_ptr GetAssetManager() = 0; + virtual void OnRootIsolateCreated() = 0; virtual void UpdateIsolateDescription(const std::string isolate_name, diff --git a/shell/common/engine.h b/shell/common/engine.h index 89d433222f522..401a2ec028fb1 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -754,8 +754,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { // |RuntimeDelegate| FontCollection& GetFontCollection() override; - // Return the asset manager associated with the current engine, or nullptr. - std::shared_ptr GetAssetManager(); + // |RuntimeDelegate| + std::shared_ptr GetAssetManager() override; // Return the weak_ptr of ImageDecoder. fml::WeakPtr GetImageDecoderWeakPtr(); diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 4305633b53c53..ce6d9b2a0e956 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -53,6 +53,7 @@ class MockRuntimeDelegate : public RuntimeDelegate { void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates)); MOCK_METHOD1(HandlePlatformMessage, void(std::unique_ptr)); MOCK_METHOD0(GetFontCollection, FontCollection&()); + MOCK_METHOD0(GetAssetManager, std::shared_ptr()); MOCK_METHOD0(OnRootIsolateCreated, void()); MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t)); MOCK_METHOD1(SetNeedsReportTimings, void(bool)); diff --git a/testing/dart/assets_test.dart b/testing/dart/assets_test.dart new file mode 100644 index 0000000000000..d5926f8aeeabf --- /dev/null +++ b/testing/dart/assets_test.dart @@ -0,0 +1,17 @@ +// 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. + +import 'dart:ui' as ui; + +import 'package:litetest/litetest.dart'; + +void main() { + test('Loading an asset that does not exist returns null', () { + expect(ui.ImmutableBuffer.fromAsset('ThisDoesNotExist'), null); + }); + + test('returns the bytes of a bundled asset', () { + expect(ui.ImmutableBuffer.fromAsset('FontManifest.json'), isNotNull); + }); +} From ba639bae3286d65cc3d94811a6ab63f827bfe96a Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 28 Apr 2022 17:58:04 -0700 Subject: [PATCH 02/25] Update platform_configuration.h --- lib/ui/window/platform_configuration.h | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 83a6f67caf1f8..fb2dd7f6cd71f 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -99,7 +99,6 @@ class PlatformConfigurationClient { /// creation. virtual FontCollection& GetFontCollection() = 0; - //-------------------------------------------------------------------------- /// @brief Returns the current collection of assets available on the /// platform. From fe93694531019b0929e8a84946cbdba9bc372889 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 28 Apr 2022 18:00:47 -0700 Subject: [PATCH 03/25] make delegation the same --- lib/ui/painting.dart | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 147933a81c92c..7a38d0978ac3f 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2053,19 +2053,11 @@ Future instantiateImageCodec( bool allowUpscaling = true, }) async { final ImmutableBuffer buffer = await ImmutableBuffer.fromUint8List(list); - final ImageDescriptor descriptor = await ImageDescriptor.encoded(buffer); - if (!allowUpscaling) { - if (targetWidth != null && targetWidth > descriptor.width) { - targetWidth = descriptor.width; - } - if (targetHeight != null && targetHeight > descriptor.height) { - targetHeight = descriptor.height; - } - } - buffer.dispose(); - return descriptor.instantiateCodec( + return instantiateImageCodecFromBuffer( + buffer, targetWidth: targetWidth, targetHeight: targetHeight, + allowUpscaling: allowUpscaling, ); } From 77261516dd9f477f63f812395d277de21681675d Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 12:59:02 -0700 Subject: [PATCH 04/25] update gn file --- testing/dart/BUILD.gn | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/dart/BUILD.gn b/testing/dart/BUILD.gn index 08355bd26c1a0..da0730d6f7028 100644 --- a/testing/dart/BUILD.gn +++ b/testing/dart/BUILD.gn @@ -5,6 +5,7 @@ import("//flutter/testing/dart/compile_test.gni") tests = [ + "assets_test.dart", "canvas_test.dart", "channel_buffers_test.dart", "codec_test.dart", From 9f458886706c490b4a163ff2c66d8bc1ae0afeec Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 13:41:00 -0700 Subject: [PATCH 05/25] use fixture --- testing/dart/assets_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/dart/assets_test.dart b/testing/dart/assets_test.dart index d5926f8aeeabf..7283cf3070fa2 100644 --- a/testing/dart/assets_test.dart +++ b/testing/dart/assets_test.dart @@ -12,6 +12,6 @@ void main() { }); test('returns the bytes of a bundled asset', () { - expect(ui.ImmutableBuffer.fromAsset('FontManifest.json'), isNotNull); + expect(ui.ImmutableBuffer.fromAsset('assets/DashInNooglerHat.jpg'), isNotNull); }); } From 6963e653757e6da9ba3c5561d066cb534f30eeab Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 15:03:15 -0700 Subject: [PATCH 06/25] add asset dir --- testing/run_tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/run_tests.py b/testing/run_tests.py index 2171ce8615d31..37bfc250f29c9 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -281,6 +281,7 @@ def RunDartTest(build_dir, test_packages, dart_file, verbose_dart_snapshot, mult command_args += [ '--use-test-fonts', '--icu-data-file-path=%s' % os.path.join(build_dir, 'icudtl.dat'), + '--flutter-assets-dir=%s' % os.path.join(build_dir, 'gen', 'flutter', 'lib', 'ui'), kernel_file_output, ] From 769a9d853d0874cb9369700eb967d62666c67d39 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 16:51:04 -0700 Subject: [PATCH 07/25] make async --- lib/ui/painting.dart | 17 ++++++----------- testing/dart/assets_test.dart | 17 +++++++++++++---- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 7a38d0978ac3f..0df3152f762e6 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -5566,17 +5566,12 @@ class ImmutableBuffer extends NativeFieldWrapperClass1 { /// Create a buffer from the asset with key [assetKey]. /// - /// Returns `null` if the asset does not exist. - static ImmutableBuffer? fromAsset(String assetKey) { - final ImmutableBuffer buffer = ImmutableBuffer._(0); - bool success = false; - buffer._initFromAsset(assetKey, (void result) { - success = true; - }); - if (!success) { - return null; - } - return buffer; + /// Throws an [Exception] if the asset does not exist. + static Future fromAsset(String assetKey) { + final ImmutableBuffer instance = ImmutableBuffer._(0); + return _futurize((_Callback callback) { + return instance._initFromAsset(assetKey, callback); + }).then((_) => instance); } void _init(Uint8List list, _Callback callback) native 'ImmutableBuffer_init'; diff --git a/testing/dart/assets_test.dart b/testing/dart/assets_test.dart index 7283cf3070fa2..f3fc820daf3e7 100644 --- a/testing/dart/assets_test.dart +++ b/testing/dart/assets_test.dart @@ -7,11 +7,20 @@ import 'dart:ui' as ui; import 'package:litetest/litetest.dart'; void main() { - test('Loading an asset that does not exist returns null', () { - expect(ui.ImmutableBuffer.fromAsset('ThisDoesNotExist'), null); + test('Loading an asset that does not exist returns null', () async { + Object? error; + try { + await ui.ImmutableBuffer.fromAsset('ThisDoesNotExist'); + } catch (err) { + error = err; + } + expect(error, isNotNull); + expect(error is Exception, true); }); - test('returns the bytes of a bundled asset', () { - expect(ui.ImmutableBuffer.fromAsset('assets/DashInNooglerHat.jpg'), isNotNull); + test('returns the bytes of a bundled asset', () async { + final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromAsset('assets/DashInNooglerHat.jpg'); + + expect(buffer.length != 0, true); }); } From 68f5b9ba2506361bddd105bdbc916bc35f4e9bbb Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 16:52:45 -0700 Subject: [PATCH 08/25] Update painting.dart --- lib/web_ui/lib/painting.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/lib/painting.dart b/lib/web_ui/lib/painting.dart index 656c9ba8c043d..aea312ba87317 100644 --- a/lib/web_ui/lib/painting.dart +++ b/lib/web_ui/lib/painting.dart @@ -756,9 +756,8 @@ class ImmutableBuffer { return instance; } - static ImmutableBuffer? fromAsset(String assetKey) { - assert(false, 'ImmutableBuffer.fromAsset is not supported on the web.'); - return null; + static Future fromAsset(String assetKey) async { + throw UnsupportedError('ImmutableBuffer.fromAsset is not supported on the web.'); } Uint8List? _list; From f216c48ade9babab3f7fb78a03434005fadf75a8 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 17:17:06 -0700 Subject: [PATCH 09/25] Actually set length --- lib/ui/painting.dart | 11 ++++++----- lib/ui/painting/immutable_buffer.cc | 2 +- testing/dart/assets_test.dart | 1 + 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 0df3152f762e6..c4e29c1d5e8e5 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -5553,7 +5553,7 @@ class Shadow { /// The creator of this object is responsible for calling [dispose] when it is /// no longer needed. class ImmutableBuffer extends NativeFieldWrapperClass1 { - ImmutableBuffer._(this.length); + ImmutableBuffer._(this._length); /// Creates a copy of the data from a [Uint8List] suitable for internal use /// in the engine. @@ -5569,17 +5569,18 @@ class ImmutableBuffer extends NativeFieldWrapperClass1 { /// Throws an [Exception] if the asset does not exist. static Future fromAsset(String assetKey) { final ImmutableBuffer instance = ImmutableBuffer._(0); - return _futurize((_Callback callback) { + return _futurize((_Callback callback) { return instance._initFromAsset(assetKey, callback); - }).then((_) => instance); + }).then((int length) => instance.._length = length); } void _init(Uint8List list, _Callback callback) native 'ImmutableBuffer_init'; - String? _initFromAsset(String assetKey, _Callback callback) native 'ImmutableBuffer_initFromAsset'; + String? _initFromAsset(String assetKey, _Callback callback) native 'ImmutableBuffer_initFromAsset'; /// The length, in bytes, of the underlying data. - final int length; + int get length => _length; + int _length; bool _debugDisposed = false; diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index b2e3033658ffe..53f67de1ec73b 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -92,7 +92,7 @@ void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { auto sk_data = SkData::MakeWithProc(bytes, size, proc, peer); auto buffer = fml::MakeRefCounted(sk_data); buffer->AssociateWithDartWrapper(immutable_buffer); - tonic::DartInvoke(callback_handle, {Dart_TypeVoid()}); + tonic::DartInvoke(callback_handle, {tonic::ToDart(size)}); } size_t ImmutableBuffer::GetAllocationSize() const { diff --git a/testing/dart/assets_test.dart b/testing/dart/assets_test.dart index f3fc820daf3e7..b911e80381b71 100644 --- a/testing/dart/assets_test.dart +++ b/testing/dart/assets_test.dart @@ -21,6 +21,7 @@ void main() { test('returns the bytes of a bundled asset', () async { final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromAsset('assets/DashInNooglerHat.jpg'); + print(buffer.length); expect(buffer.length != 0, true); }); } From 61ec3a39e190e83cfe56a71fb11d0af71767d4d3 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 18:14:10 -0700 Subject: [PATCH 10/25] Update painting.dart --- lib/web_ui/lib/painting.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/painting.dart b/lib/web_ui/lib/painting.dart index aea312ba87317..742104b4c07a5 100644 --- a/lib/web_ui/lib/painting.dart +++ b/lib/web_ui/lib/painting.dart @@ -749,7 +749,7 @@ class ImageShader extends Shader { } class ImmutableBuffer { - ImmutableBuffer._(this.length); + ImmutableBuffer._(this._length); static Future fromUint8List(Uint8List list) async { final ImmutableBuffer instance = ImmutableBuffer._(list.length); instance._list = list; @@ -761,7 +761,9 @@ class ImmutableBuffer { } Uint8List? _list; - final int length; + + int get length => _length; + int _length; bool get debugDisposed { late bool disposed; From 614a9974339536a9fd8128575b60b936c071d325 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 18:26:12 -0700 Subject: [PATCH 11/25] Update painting.dart --- lib/web_ui/lib/painting.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/painting.dart b/lib/web_ui/lib/painting.dart index 742104b4c07a5..ede65f8ddb591 100644 --- a/lib/web_ui/lib/painting.dart +++ b/lib/web_ui/lib/painting.dart @@ -761,7 +761,7 @@ class ImmutableBuffer { } Uint8List? _list; - + int get length => _length; int _length; From 6bb5767698af5c087a47ebdd32682947588d538e Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 19:26:41 -0700 Subject: [PATCH 12/25] Update assets_test.dart --- testing/dart/assets_test.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/testing/dart/assets_test.dart b/testing/dart/assets_test.dart index b911e80381b71..d976b5f378be0 100644 --- a/testing/dart/assets_test.dart +++ b/testing/dart/assets_test.dart @@ -21,7 +21,6 @@ void main() { test('returns the bytes of a bundled asset', () async { final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromAsset('assets/DashInNooglerHat.jpg'); - print(buffer.length); - expect(buffer.length != 0, true); + expect(buffer.length == 354679, true); }); } From 8d31015cf52cef4084362c0ff74c798faf3845c7 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 29 Apr 2022 23:06:43 -0700 Subject: [PATCH 13/25] something like this --- lib/ui/painting/immutable_buffer.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index 53f67de1ec73b..c210d047572d6 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -87,7 +87,9 @@ void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { const void* bytes = static_cast(data->GetMapping()); auto size = data->GetSize(); void* peer = reinterpret_cast(data.release()); - SkData::ReleaseProc proc = [](const void* ptr, void* context) {}; + SkData::ReleaseProc proc = [](const void* ptr, void* context) { + auto* mapping = static_cast(const_cast(ptr)); + }; auto sk_data = SkData::MakeWithProc(bytes, size, proc, peer); auto buffer = fml::MakeRefCounted(sk_data); From 784254fbd6dac33fc8a68f12d626a53b81fa75e3 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sat, 30 Apr 2022 14:42:47 -0700 Subject: [PATCH 14/25] get release proc --- fml/mapping.cc | 34 +++++++++++++++++++ fml/mapping.h | 19 +++++++++++ lib/ui/painting/immutable_buffer.cc | 4 +-- shell/platform/android/apk_asset_provider.cc | 8 +++++ .../darwin/common/buffer_conversions.mm | 8 +++++ .../flutter/file_in_namespace_buffer.cc | 8 +++++ .../flutter/file_in_namespace_buffer.h | 3 ++ 7 files changed, 81 insertions(+), 3 deletions(-) diff --git a/fml/mapping.cc b/fml/mapping.cc index 1b3f1e65d13bd..1c9664056bc1c 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -64,6 +64,14 @@ std::unique_ptr FileMapping::CreateReadExecute( return mapping; } +MappingReleaseProc FileMapping::GetReleaseProc() { + MappingReleaseProc proc = [](const void* ptr, void* context) { + auto* mapping = static_cast(context); + mapping->~FileMapping(); + }; + return proc; +} + // Data Mapping DataMapping::DataMapping(std::vector data) : data_(std::move(data)) {} @@ -85,6 +93,11 @@ bool DataMapping::IsDontNeedSafe() const { return false; } +MappingReleaseProc DataMapping::GetReleaseProc() { + MappingReleaseProc proc = [](const void* ptr, void* context) { }; + return proc; +} + // NonOwnedMapping NonOwnedMapping::NonOwnedMapping(const uint8_t* data, size_t size, @@ -113,6 +126,14 @@ bool NonOwnedMapping::IsDontNeedSafe() const { return dontneed_safe_; } +MappingReleaseProc NonOwnedMapping::GetReleaseProc() { + MappingReleaseProc proc = [](const void* ptr, void* context) { + auto* mapping = static_cast(context); + mapping->~NonOwnedMapping(); + }; + return proc; +} + // MallocMapping MallocMapping::MallocMapping() : data_(nullptr), size_(0) {} @@ -157,6 +178,14 @@ uint8_t* MallocMapping::Release() { return result; } +MappingReleaseProc MallocMapping::GetReleaseProc() { + MappingReleaseProc proc = [](const void* ptr, void* context) { + auto* mapping = static_cast(context); + mapping->~MallocMapping(); + }; + return proc; +} + // Symbol Mapping SymbolMapping::SymbolMapping(fml::RefPtr native_library, @@ -194,4 +223,9 @@ bool SymbolMapping::IsDontNeedSafe() const { return true; } +MappingReleaseProc SymbolMapping::GetReleaseProc() { + MappingReleaseProc proc = [](const void* ptr, void* context) { }; + return proc; +} + } // namespace fml diff --git a/fml/mapping.h b/fml/mapping.h index 3bf50841cd130..9ea3914863cb6 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -18,6 +18,8 @@ namespace fml { +using MappingReleaseProc = void(*)(const void* ptr, void* context); + class Mapping { public: Mapping(); @@ -32,6 +34,8 @@ class Mapping { // Generally true for file-mapped memory and false for anonymous memory. virtual bool IsDontNeedSafe() const = 0; + virtual MappingReleaseProc GetReleaseProc() = 0; + private: FML_DISALLOW_COPY_AND_ASSIGN(Mapping); }; @@ -72,6 +76,9 @@ class FileMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; + // |Mapping| + MappingReleaseProc GetReleaseProc() override; + uint8_t* GetMutableMapping(); bool IsValid() const; @@ -106,6 +113,9 @@ class DataMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; + // |Mapping| + MappingReleaseProc GetReleaseProc() override; + private: std::vector data_; @@ -131,6 +141,9 @@ class NonOwnedMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; + // |Mapping| + MappingReleaseProc GetReleaseProc() override; + private: const uint8_t* const data_; const size_t size_; @@ -180,6 +193,9 @@ class MallocMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; + // |Mapping| + MappingReleaseProc GetReleaseProc() override; + /// Removes ownership of the data buffer. /// After this is called; the mapping will point to nullptr. [[nodiscard]] uint8_t* Release(); @@ -207,6 +223,9 @@ class SymbolMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; + // |Mapping| + MappingReleaseProc GetReleaseProc() override; + private: fml::RefPtr native_library_; const uint8_t* mapping_ = nullptr; diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index c210d047572d6..bb74a030984cd 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -84,12 +84,10 @@ void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { return; } + SkData::ReleaseProc proc = data->GetReleaseProc(); const void* bytes = static_cast(data->GetMapping()); auto size = data->GetSize(); void* peer = reinterpret_cast(data.release()); - SkData::ReleaseProc proc = [](const void* ptr, void* context) { - auto* mapping = static_cast(const_cast(ptr)); - }; auto sk_data = SkData::MakeWithProc(bytes, size, proc, peer); auto buffer = fml::MakeRefCounted(sk_data); diff --git a/shell/platform/android/apk_asset_provider.cc b/shell/platform/android/apk_asset_provider.cc index 79372684f41c1..13bf63cd56800 100644 --- a/shell/platform/android/apk_asset_provider.cc +++ b/shell/platform/android/apk_asset_provider.cc @@ -50,6 +50,14 @@ class APKAssetMapping : public fml::Mapping { bool IsDontNeedSafe() const override { return !AAsset_isAllocated(asset_); } + MappingReleaseProc GetReleaseProc() override { + MappingReleaseProc proc = [](const void* ptr, void* context) { + auto* mapping = static_cast(context); + mapping->~APKAssetMapping(); + }; + return proc; + } + private: AAsset* const asset_; diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index 6ba9b1e4a53ab..0a85e9de7d621 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -20,6 +20,14 @@ explicit NSDataMapping(NSData* data) : data_([data retain]) {} bool IsDontNeedSafe() const override { return false; } + MappingReleaseProc GetReleaseProc() override { + MappingReleaseProc proc = [](const void* ptr, void* context) { + auto* mapping = static_cast(context); + [mapping->data_ release]; + }; + return proc; + } + private: fml::scoped_nsobject data_; FML_DISALLOW_COPY_AND_ASSIGN(NSDataMapping); diff --git a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc index 698f7fb9263c8..50b5b5b390785 100644 --- a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc +++ b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc @@ -64,6 +64,14 @@ bool FileInNamespaceBuffer::IsDontNeedSafe() const { return true; } +MappingReleaseProc FileInNamespaceBuffer::GetReleaseProc() { + MappingReleaseProc proc = [](const void* ptr, void* context) { + auto* mapping = static_cast(context); + mapping->~FileInNamespaceBuffer(); + }; + return proc; +} + std::unique_ptr LoadFile(int namespace_fd, const char* path, bool executable) { diff --git a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h index 2a075edbd0f94..5684f12482b60 100644 --- a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h +++ b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h @@ -28,6 +28,9 @@ class FileInNamespaceBuffer final : public fml::Mapping { // |fml::Mapping| bool IsDontNeedSafe() const override; + // |fml::Mapping| + MappingReleaseProc GetReleaseProc() override; + private: /// The address that was mapped to the buffer. void* address_; From 4f307211a8a58cb37f5088ded4b911a5fb4f9990 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sat, 30 Apr 2022 16:09:30 -0700 Subject: [PATCH 15/25] ++ --- shell/platform/android/apk_asset_provider.cc | 4 ++-- shell/platform/darwin/common/buffer_conversions.mm | 4 ++-- shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/shell/platform/android/apk_asset_provider.cc b/shell/platform/android/apk_asset_provider.cc index 13bf63cd56800..ff5c1fd3085c1 100644 --- a/shell/platform/android/apk_asset_provider.cc +++ b/shell/platform/android/apk_asset_provider.cc @@ -50,8 +50,8 @@ class APKAssetMapping : public fml::Mapping { bool IsDontNeedSafe() const override { return !AAsset_isAllocated(asset_); } - MappingReleaseProc GetReleaseProc() override { - MappingReleaseProc proc = [](const void* ptr, void* context) { + fml::MappingReleaseProc GetReleaseProc() override { + fml::MappingReleaseProc proc = [](const void* ptr, void* context) { auto* mapping = static_cast(context); mapping->~APKAssetMapping(); }; diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index 0a85e9de7d621..24d7b4a776957 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -20,8 +20,8 @@ explicit NSDataMapping(NSData* data) : data_([data retain]) {} bool IsDontNeedSafe() const override { return false; } - MappingReleaseProc GetReleaseProc() override { - MappingReleaseProc proc = [](const void* ptr, void* context) { + fml::MappingReleaseProc GetReleaseProc() override { + fml:: MappingReleaseProc proc = [](const void* ptr, void* context) { auto* mapping = static_cast(context); [mapping->data_ release]; }; diff --git a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc index 50b5b5b390785..a76ee8791efbd 100644 --- a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc +++ b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc @@ -64,8 +64,8 @@ bool FileInNamespaceBuffer::IsDontNeedSafe() const { return true; } -MappingReleaseProc FileInNamespaceBuffer::GetReleaseProc() { - MappingReleaseProc proc = [](const void* ptr, void* context) { +fml::MappingReleaseProc FileInNamespaceBuffer::GetReleaseProc() { + fml::MappingReleaseProc proc = [](const void* ptr, void* context) { auto* mapping = static_cast(context); mapping->~FileInNamespaceBuffer(); }; From bf0c61421106b9b678e3e2c9f83c914387de17b6 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sat, 30 Apr 2022 17:52:06 -0700 Subject: [PATCH 16/25] Update apk_asset_provider.cc --- shell/platform/android/apk_asset_provider.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/apk_asset_provider.cc b/shell/platform/android/apk_asset_provider.cc index ff5c1fd3085c1..5f1935aba0123 100644 --- a/shell/platform/android/apk_asset_provider.cc +++ b/shell/platform/android/apk_asset_provider.cc @@ -52,7 +52,7 @@ class APKAssetMapping : public fml::Mapping { fml::MappingReleaseProc GetReleaseProc() override { fml::MappingReleaseProc proc = [](const void* ptr, void* context) { - auto* mapping = static_cast(context); + auto* mapping = static_cast(context); mapping->~APKAssetMapping(); }; return proc; From a13380bf2d72b97d6932d555a0745f9e4ebcde00 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sat, 30 Apr 2022 18:09:59 -0700 Subject: [PATCH 17/25] Update file_in_namespace_buffer.h --- shell/platform/fuchsia/flutter/file_in_namespace_buffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h index 5684f12482b60..d0897417f5b73 100644 --- a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h +++ b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h @@ -29,7 +29,7 @@ class FileInNamespaceBuffer final : public fml::Mapping { bool IsDontNeedSafe() const override; // |fml::Mapping| - MappingReleaseProc GetReleaseProc() override; + fml::MappingReleaseProc GetReleaseProc() override; private: /// The address that was mapped to the buffer. From 77b4eac2ab2af2f404ec6a0d1c3345efc8e41774 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sat, 30 Apr 2022 18:57:29 -0700 Subject: [PATCH 18/25] format and dispose test --- shell/platform/darwin/common/buffer_conversions.mm | 2 +- testing/dart/assets_test.dart | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index 24d7b4a776957..7c25032ffe780 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -21,7 +21,7 @@ explicit NSDataMapping(NSData* data) : data_([data retain]) {} bool IsDontNeedSafe() const override { return false; } fml::MappingReleaseProc GetReleaseProc() override { - fml:: MappingReleaseProc proc = [](const void* ptr, void* context) { + fml:: MappingReleaseProc proc = [](const void* ptr, void* context) { auto* mapping = static_cast(context); [mapping->data_ release]; }; diff --git a/testing/dart/assets_test.dart b/testing/dart/assets_test.dart index d976b5f378be0..de975f471695e 100644 --- a/testing/dart/assets_test.dart +++ b/testing/dart/assets_test.dart @@ -23,4 +23,10 @@ void main() { expect(buffer.length == 354679, true); }); + + test('can dispose immutable buffer', () async { + final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromAsset('assets/DashInNooglerHat.jpg'); + + buffer.dispose(); + }); } From 940d8d19988f9637a5bb11a027e694aae3cb86c2 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sat, 30 Apr 2022 18:58:25 -0700 Subject: [PATCH 19/25] format++ --- fml/mapping.cc | 4 ++-- fml/mapping.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fml/mapping.cc b/fml/mapping.cc index 1c9664056bc1c..d6b4aba38c08b 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -94,7 +94,7 @@ bool DataMapping::IsDontNeedSafe() const { } MappingReleaseProc DataMapping::GetReleaseProc() { - MappingReleaseProc proc = [](const void* ptr, void* context) { }; + MappingReleaseProc proc = [](const void* ptr, void* context) {}; return proc; } @@ -224,7 +224,7 @@ bool SymbolMapping::IsDontNeedSafe() const { } MappingReleaseProc SymbolMapping::GetReleaseProc() { - MappingReleaseProc proc = [](const void* ptr, void* context) { }; + MappingReleaseProc proc = [](const void* ptr, void* context) {}; return proc; } diff --git a/fml/mapping.h b/fml/mapping.h index 9ea3914863cb6..8709de9e803ed 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -18,7 +18,7 @@ namespace fml { -using MappingReleaseProc = void(*)(const void* ptr, void* context); +using MappingReleaseProc = void (*)(const void* ptr, void* context); class Mapping { public: From 56029fd58ac9efd86ff032e32fc02d955274b19f Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sat, 30 Apr 2022 19:07:24 -0700 Subject: [PATCH 20/25] Update buffer_conversions.mm --- shell/platform/darwin/common/buffer_conversions.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index 7c25032ffe780..b030b6c9cba0e 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -21,7 +21,7 @@ explicit NSDataMapping(NSData* data) : data_([data retain]) {} bool IsDontNeedSafe() const override { return false; } fml::MappingReleaseProc GetReleaseProc() override { - fml:: MappingReleaseProc proc = [](const void* ptr, void* context) { + fml::MappingReleaseProc proc = [](const void* ptr, void* context) { auto* mapping = static_cast(context); [mapping->data_ release]; }; From 512a2b66747ffc5f10d65635f15d7decf9aa529b Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 2 May 2022 11:03:26 -0700 Subject: [PATCH 21/25] use delete --- fml/mapping.cc | 34 ------------------- fml/mapping.h | 19 ----------- lib/ui/painting/immutable_buffer.cc | 4 ++- shell/platform/android/apk_asset_provider.cc | 8 ----- .../darwin/common/buffer_conversions.mm | 8 ----- .../flutter/file_in_namespace_buffer.cc | 8 ----- .../flutter/file_in_namespace_buffer.h | 3 -- 7 files changed, 3 insertions(+), 81 deletions(-) diff --git a/fml/mapping.cc b/fml/mapping.cc index d6b4aba38c08b..1b3f1e65d13bd 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -64,14 +64,6 @@ std::unique_ptr FileMapping::CreateReadExecute( return mapping; } -MappingReleaseProc FileMapping::GetReleaseProc() { - MappingReleaseProc proc = [](const void* ptr, void* context) { - auto* mapping = static_cast(context); - mapping->~FileMapping(); - }; - return proc; -} - // Data Mapping DataMapping::DataMapping(std::vector data) : data_(std::move(data)) {} @@ -93,11 +85,6 @@ bool DataMapping::IsDontNeedSafe() const { return false; } -MappingReleaseProc DataMapping::GetReleaseProc() { - MappingReleaseProc proc = [](const void* ptr, void* context) {}; - return proc; -} - // NonOwnedMapping NonOwnedMapping::NonOwnedMapping(const uint8_t* data, size_t size, @@ -126,14 +113,6 @@ bool NonOwnedMapping::IsDontNeedSafe() const { return dontneed_safe_; } -MappingReleaseProc NonOwnedMapping::GetReleaseProc() { - MappingReleaseProc proc = [](const void* ptr, void* context) { - auto* mapping = static_cast(context); - mapping->~NonOwnedMapping(); - }; - return proc; -} - // MallocMapping MallocMapping::MallocMapping() : data_(nullptr), size_(0) {} @@ -178,14 +157,6 @@ uint8_t* MallocMapping::Release() { return result; } -MappingReleaseProc MallocMapping::GetReleaseProc() { - MappingReleaseProc proc = [](const void* ptr, void* context) { - auto* mapping = static_cast(context); - mapping->~MallocMapping(); - }; - return proc; -} - // Symbol Mapping SymbolMapping::SymbolMapping(fml::RefPtr native_library, @@ -223,9 +194,4 @@ bool SymbolMapping::IsDontNeedSafe() const { return true; } -MappingReleaseProc SymbolMapping::GetReleaseProc() { - MappingReleaseProc proc = [](const void* ptr, void* context) {}; - return proc; -} - } // namespace fml diff --git a/fml/mapping.h b/fml/mapping.h index 8709de9e803ed..3bf50841cd130 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -18,8 +18,6 @@ namespace fml { -using MappingReleaseProc = void (*)(const void* ptr, void* context); - class Mapping { public: Mapping(); @@ -34,8 +32,6 @@ class Mapping { // Generally true for file-mapped memory and false for anonymous memory. virtual bool IsDontNeedSafe() const = 0; - virtual MappingReleaseProc GetReleaseProc() = 0; - private: FML_DISALLOW_COPY_AND_ASSIGN(Mapping); }; @@ -76,9 +72,6 @@ class FileMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; - // |Mapping| - MappingReleaseProc GetReleaseProc() override; - uint8_t* GetMutableMapping(); bool IsValid() const; @@ -113,9 +106,6 @@ class DataMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; - // |Mapping| - MappingReleaseProc GetReleaseProc() override; - private: std::vector data_; @@ -141,9 +131,6 @@ class NonOwnedMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; - // |Mapping| - MappingReleaseProc GetReleaseProc() override; - private: const uint8_t* const data_; const size_t size_; @@ -193,9 +180,6 @@ class MallocMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; - // |Mapping| - MappingReleaseProc GetReleaseProc() override; - /// Removes ownership of the data buffer. /// After this is called; the mapping will point to nullptr. [[nodiscard]] uint8_t* Release(); @@ -223,9 +207,6 @@ class SymbolMapping final : public Mapping { // |Mapping| bool IsDontNeedSafe() const override; - // |Mapping| - MappingReleaseProc GetReleaseProc() override; - private: fml::RefPtr native_library_; const uint8_t* mapping_ = nullptr; diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index bb74a030984cd..abb104adb3aac 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -84,7 +84,9 @@ void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { return; } - SkData::ReleaseProc proc = data->GetReleaseProc(); + SkData::ReleaseProc proc = [](const void* ptr, void* context) { + delete static_cast(context); + }; const void* bytes = static_cast(data->GetMapping()); auto size = data->GetSize(); void* peer = reinterpret_cast(data.release()); diff --git a/shell/platform/android/apk_asset_provider.cc b/shell/platform/android/apk_asset_provider.cc index 5f1935aba0123..79372684f41c1 100644 --- a/shell/platform/android/apk_asset_provider.cc +++ b/shell/platform/android/apk_asset_provider.cc @@ -50,14 +50,6 @@ class APKAssetMapping : public fml::Mapping { bool IsDontNeedSafe() const override { return !AAsset_isAllocated(asset_); } - fml::MappingReleaseProc GetReleaseProc() override { - fml::MappingReleaseProc proc = [](const void* ptr, void* context) { - auto* mapping = static_cast(context); - mapping->~APKAssetMapping(); - }; - return proc; - } - private: AAsset* const asset_; diff --git a/shell/platform/darwin/common/buffer_conversions.mm b/shell/platform/darwin/common/buffer_conversions.mm index b030b6c9cba0e..6ba9b1e4a53ab 100644 --- a/shell/platform/darwin/common/buffer_conversions.mm +++ b/shell/platform/darwin/common/buffer_conversions.mm @@ -20,14 +20,6 @@ explicit NSDataMapping(NSData* data) : data_([data retain]) {} bool IsDontNeedSafe() const override { return false; } - fml::MappingReleaseProc GetReleaseProc() override { - fml::MappingReleaseProc proc = [](const void* ptr, void* context) { - auto* mapping = static_cast(context); - [mapping->data_ release]; - }; - return proc; - } - private: fml::scoped_nsobject data_; FML_DISALLOW_COPY_AND_ASSIGN(NSDataMapping); diff --git a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc index a76ee8791efbd..698f7fb9263c8 100644 --- a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc +++ b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc @@ -64,14 +64,6 @@ bool FileInNamespaceBuffer::IsDontNeedSafe() const { return true; } -fml::MappingReleaseProc FileInNamespaceBuffer::GetReleaseProc() { - fml::MappingReleaseProc proc = [](const void* ptr, void* context) { - auto* mapping = static_cast(context); - mapping->~FileInNamespaceBuffer(); - }; - return proc; -} - std::unique_ptr LoadFile(int namespace_fd, const char* path, bool executable) { diff --git a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h index d0897417f5b73..2a075edbd0f94 100644 --- a/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h +++ b/shell/platform/fuchsia/flutter/file_in_namespace_buffer.h @@ -28,9 +28,6 @@ class FileInNamespaceBuffer final : public fml::Mapping { // |fml::Mapping| bool IsDontNeedSafe() const override; - // |fml::Mapping| - fml::MappingReleaseProc GetReleaseProc() override; - private: /// The address that was mapped to the buffer. void* address_; From d27b4ebe54b363bdce9627961f5f92a140ee9547 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 10 May 2022 14:45:15 -0700 Subject: [PATCH 22/25] special android copy --- lib/ui/painting/immutable_buffer.cc | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index abb104adb3aac..f47c8af3336df 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -84,14 +84,20 @@ void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { return; } + auto size = data->GetSize(); + const void* bytes = static_cast(data->GetMapping()); + +#if FML_OS_ANDROID + // fml::Mapping backed by android assets are not thread safe. + auto sk_data = MakeSkDataWithCopy(bytes, size); +#else SkData::ReleaseProc proc = [](const void* ptr, void* context) { delete static_cast(context); }; - const void* bytes = static_cast(data->GetMapping()); - auto size = data->GetSize(); void* peer = reinterpret_cast(data.release()); - auto sk_data = SkData::MakeWithProc(bytes, size, proc, peer); +#endif // FML_OS_ANDROID + auto buffer = fml::MakeRefCounted(sk_data); buffer->AssociateWithDartWrapper(immutable_buffer); tonic::DartInvoke(callback_handle, {tonic::ToDart(size)}); From 75b86f9d19f09cff5859efb8833800092d1a7c27 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 10 May 2022 14:46:38 -0700 Subject: [PATCH 23/25] ++ --- lib/ui/painting/immutable_buffer.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index f47c8af3336df..82635378f718d 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -96,7 +96,7 @@ void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { }; void* peer = reinterpret_cast(data.release()); auto sk_data = SkData::MakeWithProc(bytes, size, proc, peer); -#endif // FML_OS_ANDROID +#endif // FML_OS_ANDROID auto buffer = fml::MakeRefCounted(sk_data); buffer->AssociateWithDartWrapper(immutable_buffer); From 7082bdb66bb6a69b43aeb87c88a8e704395e5b50 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 10 May 2022 17:00:09 -0700 Subject: [PATCH 24/25] always copy --- lib/ui/painting/immutable_buffer.cc | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index 82635378f718d..4b024b5c43695 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -88,14 +88,9 @@ void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { const void* bytes = static_cast(data->GetMapping()); #if FML_OS_ANDROID - // fml::Mapping backed by android assets are not thread safe. auto sk_data = MakeSkDataWithCopy(bytes, size); #else - SkData::ReleaseProc proc = [](const void* ptr, void* context) { - delete static_cast(context); - }; - void* peer = reinterpret_cast(data.release()); - auto sk_data = SkData::MakeWithProc(bytes, size, proc, peer); + auto sk_data = SkData::MakeWithCopy(bytes, size); #endif // FML_OS_ANDROID auto buffer = fml::MakeRefCounted(sk_data); From 66ccfc59bcdac34f3005dda851b181ccd1e737ed Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 10 May 2022 17:36:23 -0700 Subject: [PATCH 25/25] Update immutable_buffer.cc --- lib/ui/painting/immutable_buffer.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index 4b024b5c43695..8c9ddc9dedb15 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -86,13 +86,7 @@ void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) { auto size = data->GetSize(); const void* bytes = static_cast(data->GetMapping()); - -#if FML_OS_ANDROID auto sk_data = MakeSkDataWithCopy(bytes, size); -#else - auto sk_data = SkData::MakeWithCopy(bytes, size); -#endif // FML_OS_ANDROID - auto buffer = fml::MakeRefCounted(sk_data); buffer->AssociateWithDartWrapper(immutable_buffer); tonic::DartInvoke(callback_handle, {tonic::ToDart(size)});