Skip to content
This repository has been archived by the owner on Feb 25, 2025. It is now read-only.

[dart:ui] add an explicit API for loading assets #21153

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4960,6 +4960,22 @@ class ImmutableBuffer extends NativeFieldWrapperClass2 {
}
void _init(Uint8List list, _Callback<void> callback) native 'ImmutableBuffer_init';

/// 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);
Comment on lines +4966 to +4967
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd want to make this a factory so we could have _fromAsset return the length of the asset in bytes, and set the lenght properly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could make the length a getter on a private field that is mutable.

bool success = false;
_fromAsset(assetKey, buffer, (void result) {
success = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is racy - see below. We want to use _futurize for this, or make our own completer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this actually racy? I'm not really familiar with the native interop but it appears to be fully synchronous to me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone sets the return value in the C++ side before calling the completer here I believe it will have unexpected results.

});
if (!success) {
return null;
}
return buffer;
}
static void _fromAsset(String assetKey, ImmutableBuffer buffer, _Callback<void> callback) native 'ImmutableBuffer_fromAsset';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more typical signature for this is String? _fromAsset(ImmutableBuffer outBuffer, String assetKey, _Callback<void> callback) native 'ImmutableBuffer_fromAsset;. The String is used to return an error message rather than throwing in C++, which has undesirable runtime overhead/stack trace strangeness. You can then use _futureize with it.

It doesn't need to be static, and if the callback calls back with an int it could return the length.


/// The length, in bytes, of the underlying data.
final int length;

Expand Down
48 changes: 48 additions & 0 deletions lib/ui/painting/immutable_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include "third_party/tonic/converter/dart_converter.h"
#include "third_party/tonic/dart_args.h"
#include "third_party/tonic/dart_binding_macros.h"
#include "flutter/lib/ui/ui_dart_state.h"
#include "flutter/lib/ui/window/platform_configuration.h"

#if OS_ANDROID
#include <sys/mman.h>
Expand All @@ -30,6 +32,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_fromAsset", ImmutableBuffer::fromAsset, 3, true},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do it this way, we don't have to pass the ImmutableBuffer itself as an argument. We also don't want it to be static.

FOR_EACH_BINDING(DART_REGISTER_NATIVE)});

}

void ImmutableBuffer::init(Dart_NativeArguments args) {
Expand All @@ -53,6 +58,49 @@ size_t ImmutableBuffer::GetAllocationSize() const {
return sizeof(ImmutableBuffer) + data_->size();
}

void ImmutableBuffer::fromAsset(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, 0);
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_PropagateError(result);
return;
Comment on lines +74 to +75
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we try to avoid this and instead use a String error.

}
Dart_Handle immutable_buffer = Dart_GetNativeArgument(args, 1);

std::string asset_name = std::string{reinterpret_cast<const char*>(chars),
static_cast<size_t>(asset_length)};

std::shared_ptr<AssetManager> asset_manager = UIDartState::Current()
->platform_configuration()
->client()
->GetAssetManager();
std::unique_ptr<fml::Mapping> data = asset_manager->GetAsMapping(asset_name);

if (data == nullptr) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return a Dart String here with something like "Asset not found".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, Dart_SetReturnValue

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contract for asset loading is already that we return null if it isn't found, and the framework can turn this into an exception

}
const void* bytes = static_cast<const void*>(data->GetMapping());
void* peer = reinterpret_cast<void*>(data.release());
auto size = data->GetSize();
SkData::ReleaseProc proc = [](const void* ptr, void* context) {
// TODO: what do
};

auto sk_data = SkData::MakeWithProc(bytes, size, proc, peer);
Comment on lines +94 to +98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to just store the mapping object in our heap which should keep it alive, and then reset it in the release proc. I think that matches the semantics of the fml::Mapping, but @jason-simmons or @chinmaygarde would know for sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The peer will be passed to the ReleaseProc as the context parameter. The ReleaseProc can cast it back to an fml::Mapping* and delete it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will that be enough to keep the Mapping pointer alive?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mapping pointer should remain valid as long as the fml::Mapping and the AssetManager that created it have not been deleted.

auto buffer = fml::MakeRefCounted<ImmutableBuffer>(sk_data);
buffer->AssociateWithDartWrapper(immutable_buffer);
tonic::DartInvoke(callback_handle, {Dart_TypeVoid()});
}

#if OS_ANDROID

// Compressed image buffers are allocated on the UI thread but are deleted on a
Expand Down
9 changes: 9 additions & 0 deletions lib/ui/painting/immutable_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ class ImmutableBuffer : public RefCountedDartWrappable<ImmutableBuffer> {
/// when the copy has completed.
static void init(Dart_NativeArguments args);

/// Create a new ImmutableData from a Dart asset key.
///
/// The zero indexed argument is the UTF8 encoded string which corresponds to
/// the asset manager key.
///
/// The first indexed argument is a callback that can optionally be invoked
/// with the resulting ImmutableData.
static void fromAsset(Dart_NativeArguments args);

/// The length of the data in bytes.
size_t length() const {
FML_DCHECK(data_);
Expand Down
3 changes: 3 additions & 0 deletions lib/ui/window/platform_configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <unordered_map>
#include <vector>

#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/platform_message.h"
Expand Down Expand Up @@ -111,6 +112,8 @@ class PlatformConfigurationClient {
/// creation.
virtual FontCollection& GetFontCollection() = 0;

virtual std::shared_ptr<AssetManager> GetAssetManager() = 0;

//--------------------------------------------------------------------------
/// @brief Notifies this client of the name of the root isolate and its
/// port when that isolate is launched, restarted (in the
Expand Down
17 changes: 17 additions & 0 deletions lib/web_ui/lib/src/ui/assets.dart
Original file line number Diff line number Diff line change
@@ -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.

// @dart = 2.10
part of ui;

/// Load the asset bytes specified by [assetKey].
///
/// The [assetKey] is generally the filepath of the asset which is bundled
/// into the flutter application. This API is not supported on the
/// Web.
///
/// If the [assetKey] does not correspond to a real asset, returns `null`.
ByteData? loadAsset(String assetKey) {
return null;
}
1 change: 1 addition & 0 deletions lib/web_ui/lib/ui.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ part 'src/ui/test_embedding.dart';
part 'src/ui/text.dart';
part 'src/ui/tile_mode.dart';
part 'src/ui/window.dart';
part 'src/ui/assets.dart';

/// Provides a compile time constant to customize flutter framework and other
/// users of ui engine for web runtime.
Expand Down
6 changes: 6 additions & 0 deletions runtime/runtime_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include "flutter/runtime/runtime_controller.h"
#include "flutter/assets/asset_manager.h"

#include "flutter/fml/message_loop.h"
#include "flutter/fml/trace_event.h"
Expand Down Expand Up @@ -335,6 +336,11 @@ FontCollection& RuntimeController::GetFontCollection() {
return client_.GetFontCollection();
}

// |PlatfromConfigurationClient|
std::shared_ptr<AssetManager> RuntimeController::GetAssetManager() {
return client_.GetAssetManager();
}

// |PlatformConfigurationClient|
void RuntimeController::UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) {
Expand Down
4 changes: 4 additions & 0 deletions runtime/runtime_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <vector>

#include "flutter/assets/asset_manager.h"
#include "flutter/common/task_runners.h"
#include "flutter/flow/layers/layer_tree.h"
#include "flutter/fml/macros.h"
Expand Down Expand Up @@ -508,6 +509,9 @@ class RuntimeController : public PlatformConfigurationClient {
// |PlatformConfigurationClient|
FontCollection& GetFontCollection() override;

// |PlatformConfigurationClient|
std::shared_ptr<AssetManager> GetAssetManager() override;

// |PlatformConfigurationClient|
void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) override;
Expand Down
3 changes: 3 additions & 0 deletions runtime/runtime_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <vector>

#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"
Expand All @@ -32,6 +33,8 @@ class RuntimeDelegate {

virtual FontCollection& GetFontCollection() = 0;

virtual std::shared_ptr<AssetManager> GetAssetManager() = 0;

virtual void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) = 0;

Expand Down
4 changes: 4 additions & 0 deletions shell/common/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,10 @@ FontCollection& Engine::GetFontCollection() {
return font_collection_;
}

std::shared_ptr<AssetManager> Engine::GetAssetManager() {
return asset_manager_;
}

void Engine::DoDispatchPacket(std::unique_ptr<PointerDataPacket> packet,
uint64_t trace_flow_id) {
animator_->EnqueueTraceFlowId(trace_flow_id);
Expand Down
3 changes: 3 additions & 0 deletions shell/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,9 @@ class Engine final : public RuntimeDelegate,
// |RuntimeDelegate|
FontCollection& GetFontCollection() override;

// |RuntimeDelegate|
std::shared_ptr<AssetManager> GetAssetManager() override;

// |PointerDataDispatcher::Delegate|
void DoDispatchPacket(std::unique_ptr<PointerDataPacket> packet,
uint64_t trace_flow_id) override;
Expand Down
1 change: 1 addition & 0 deletions shell/common/engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class MockRuntimeDelegate : public RuntimeDelegate {
void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates));
MOCK_METHOD1(HandlePlatformMessage, void(fml::RefPtr<PlatformMessage>));
MOCK_METHOD0(GetFontCollection, FontCollection&());
MOCK_METHOD0(GetAssetManager, std::shared_ptr<AssetManager>());
MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
MOCK_METHOD1(SetNeedsReportTimings, void(bool));
MOCK_METHOD1(ComputePlatformResolvedLocale,
Expand Down
18 changes: 18 additions & 0 deletions testing/dart/assets_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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.

// @dart = 2.6
import 'dart:ui' as ui;

import 'package:test/test.dart';

void main() {
test('Loading an asset that does not exist returns null', () {
expect(ui.loadAsset('ThisDoesNotExist'), null);
});

test('returns the bytes of a bundled asset', () {
expect(ui.loadAsset('FontManifest.json'), isNotNull);
});
}