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

Add callback to Embedder API to respond to new channel listeners, and use for Windows lifecycle #44827

Merged
merged 39 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
bf3c3c7
Skeleton cpp function
yaakovschectman Aug 16, 2023
839f8d1
Calling
yaakovschectman Aug 16, 2023
5dd476e
Message makes it to Shell
yaakovschectman Aug 16, 2023
0bdacd0
Part of project args
yaakovschectman Aug 16, 2023
c2091cf
FlutterWindowsEngine creates a callback
yaakovschectman Aug 16, 2023
b3cf373
Engine listens
yaakovschectman Aug 17, 2023
0ca00f7
Remove OnApplicationLifecycleEnabled
yaakovschectman Aug 17, 2023
0ea137c
Unit test
yaakovschectman Aug 17, 2023
af8cf2d
Doc comments
yaakovschectman Aug 17, 2023
711ebac
Formatting
yaakovschectman Aug 17, 2023
17c85e1
Update to info struct
yaakovschectman Aug 18, 2023
770bcfe
Merge branch 'main' into embedder_channel
yaakovschectman Aug 18, 2023
da9083a
Update test
yaakovschectman Aug 18, 2023
ba08343
Merge branch 'main' into embedder_channel
yaakovschectman Aug 21, 2023
f10ec72
Web parity
yaakovschectman Aug 21, 2023
6d86f71
Add callback to test context
yaakovschectman Aug 21, 2023
c7a6454
Start unit test
yaakovschectman Aug 23, 2023
6286c00
Merge branch 'main' into embedder_channel
yaakovschectman Aug 23, 2023
60f5ac9
Merge branch 'main' into embedder_channel
yaakovschectman Aug 23, 2023
d136a93
Run expired tasks
yaakovschectman Aug 23, 2023
3413635
Rename methods
yaakovschectman Aug 24, 2023
ff1e6c2
Copy channel name in lambda
yaakovschectman Aug 24, 2023
bf67370
Merge branch 'main' into embedder_channel
yaakovschectman Aug 24, 2023
6cf7338
Meet mac tidy
yaakovschectman Aug 24, 2023
f0ac397
Required named param
yaakovschectman Aug 24, 2023
bff347a
Check for listener set
yaakovschectman Aug 24, 2023
2e87fa9
Check values in unittest
yaakovschectman Aug 24, 2023
cf671c8
PR feedback
yaakovschectman Aug 24, 2023
77316c9
Merge branch 'main' into embedder_channel
yaakovschectman Aug 24, 2023
54d0826
Merge branch 'main' into embedder_channel
yaakovschectman Aug 28, 2023
62b015c
Remove const reference
yaakovschectman Aug 28, 2023
478b6dd
STREQ
yaakovschectman Aug 28, 2023
96e841c
Test lifecycle begins
yaakovschectman Aug 28, 2023
d01b399
Test lifecycle begins
yaakovschectman Aug 28, 2023
48a17ff
Update shell/platform/embedder/tests/embedder_unittests.cc
yaakovschectman Aug 28, 2023
6b9b572
Update shell/common/platform_view.h
yaakovschectman Aug 28, 2023
5b6a221
Merge branch 'main' into embedder_channel
yaakovschectman Aug 28, 2023
bdbb829
Add struct size
yaakovschectman Aug 28, 2023
f4d292b
Reintroduce const ref
yaakovschectman Aug 29, 2023
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
7 changes: 7 additions & 0 deletions lib/ui/channel_buffers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ class ChannelBuffers {
assert(!name.contains('\u0000'), 'Channel names must not contain U+0000 NULL characters.');
final _Channel channel = _channels.putIfAbsent(name, () => _Channel());
channel.setListener(callback);
sendChannelUpdate(name, listening: true);
}

/// Clears the listener for the specified channel.
Expand All @@ -392,9 +393,15 @@ class ChannelBuffers {
final _Channel? channel = _channels[name];
if (channel != null) {
channel.clearListener();
sendChannelUpdate(name, listening: false);
}
}

@Native<Void Function(Handle, Bool)>(symbol: 'PlatformConfigurationNativeApi::SendChannelUpdate')
external static void _sendChannelUpdate(String name, bool listening);

void sendChannelUpdate(String name, {required bool listening}) => _sendChannelUpdate(name, listening);

/// Deprecated. Migrate to [setListener] instead.
///
/// Remove and process all stored messages for a given channel.
Expand Down
1 change: 1 addition & 0 deletions lib/ui/dart_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ typedef CanvasPath Path;
V(PlatformConfigurationNativeApi::GetRootIsolateToken, 0) \
V(PlatformConfigurationNativeApi::RegisterBackgroundIsolate, 1) \
V(PlatformConfigurationNativeApi::SendPortPlatformMessage, 4) \
V(PlatformConfigurationNativeApi::SendChannelUpdate, 2) \
V(PlatformConfigurationNativeApi::GetScaledFontSize, 2) \
V(DartRuntimeHooks::Logger_PrintDebugString, 1) \
V(DartRuntimeHooks::Logger_PrintString, 1) \
Expand Down
6 changes: 6 additions & 0 deletions lib/ui/window/platform_configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,12 @@ void PlatformConfigurationNativeApi::RegisterBackgroundIsolate(
dart_state->SetPlatformMessageHandler(weak_platform_message_handler);
}

void PlatformConfigurationNativeApi::SendChannelUpdate(const std::string& name,
bool listening) {
UIDartState::Current()->platform_configuration()->client()->SendChannelUpdate(
name, listening);
}

double PlatformConfigurationNativeApi::GetScaledFontSize(
double unscaled_font_size,
int configuration_id) {
Expand Down
13 changes: 13 additions & 0 deletions lib/ui/window/platform_configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,17 @@ class PlatformConfigurationClient {
///
virtual void RequestDartDeferredLibrary(intptr_t loading_unit_id) = 0;

//--------------------------------------------------------------------------
/// @brief Invoked when a listener is registered on a platform channel.
///
/// @param[in] name The name of the platform channel to which a
/// listener has been registered or cleared.
///
/// @param[in] listening Whether the listener has been set (true) or
/// cleared (false).
///
virtual void SendChannelUpdate(std::string name, bool listening) = 0;
Copy link
Member

@cbracken cbracken Aug 28, 2023

Choose a reason for hiding this comment

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

This signature doesn't appear to match the implementation. std::string vs const std::string& name. Based on @loic-sharma's comment, looks like maybe this is the correct signature but the one lower down static void SendChannelUpdate(const std::string& name, bool listening); should be corrected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What non-matching implementation to this signature are you looking at? It is implemented in runtime_controller.cc:405, where the signatures match.


//--------------------------------------------------------------------------
/// @brief Synchronously invokes platform-specific APIs to apply the
/// system text scaling on the given unscaled font size.
Expand Down Expand Up @@ -571,6 +582,8 @@ class PlatformConfigurationNativeApi {
static void RespondToPlatformMessage(int response_id,
const tonic::DartByteData& data);

static void SendChannelUpdate(const std::string& name, bool listening);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using const std::string&, could we use std::string instead and std::move it down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently, it seems this may not be an option. I tried that and pushed the change, but ironically, it seems doing so actually results in errors reported by the clang tidy tests explicitly saying to make the parameters const references. This is only the case for some of the lambdas/methods, but the signature of PlatformView::SendChannelUpdate must necessarily match that of PlatformViewEmbedder::SendChannelUpdate.


//--------------------------------------------------------------------------
/// @brief Requests the Dart VM to adjusts the GC heuristics based on
/// the requested `performance_mode`. Returns the old performance
Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/lib/channel_buffers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ class ChannelBuffers {
return true;
}());
}

void sendChannelUpdate(String name, {required bool listening}) {}
}

final ChannelBuffers channelBuffers = ChannelBuffers();
5 changes: 5 additions & 0 deletions runtime/runtime_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,11 @@ RuntimeController::ComputePlatformResolvedLocale(
return client_.ComputePlatformResolvedLocale(supported_locale_data);
}

// |PlatformConfigurationClient|
void RuntimeController::SendChannelUpdate(std::string name, bool listening) {
client_.SendChannelUpdate(std::move(name), listening);
}

Dart_Port RuntimeController::GetMainPort() {
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->main_port() : ILLEGAL_PORT;
Expand Down
3 changes: 3 additions & 0 deletions runtime/runtime_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,9 @@ class RuntimeController : public PlatformConfigurationClient {
std::unique_ptr<std::vector<std::string>> ComputePlatformResolvedLocale(
const std::vector<std::string>& supported_locale_data) override;

// |PlatformConfigurationClient|
void SendChannelUpdate(std::string name, bool listening) override;

// |PlatformConfigurationClient|
double GetScaledFontSize(double unscaled_font_size,
int configuration_id) const override;
Expand Down
2 changes: 2 additions & 0 deletions runtime/runtime_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class RuntimeDelegate {
virtual std::weak_ptr<PlatformMessageHandler> GetPlatformMessageHandler()
const = 0;

virtual void SendChannelUpdate(std::string name, bool listening) = 0;

virtual double GetScaledFontSize(double unscaled_font_size,
int configuration_id) const = 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 @@ -570,6 +570,10 @@ std::weak_ptr<PlatformMessageHandler> Engine::GetPlatformMessageHandler()
return delegate_.GetPlatformMessageHandler();
}

void Engine::SendChannelUpdate(std::string name, bool listening) {
delegate_.OnEngineChannelUpdate(std::move(name), listening);
}

void Engine::LoadDartDeferredLibrary(
intptr_t loading_unit_id,
std::unique_ptr<const fml::Mapping> snapshot_data,
Expand Down
14 changes: 14 additions & 0 deletions shell/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,17 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
virtual const std::shared_ptr<PlatformMessageHandler>&
GetPlatformMessageHandler() const = 0;

//--------------------------------------------------------------------------
/// @brief Invoked when a listener is registered on a platform channel.
///
/// @param[in] name The name of the platform channel to which a
/// listener has been registered or cleared.
///
/// @param[in] listening Whether the listener has been set (true) or
/// cleared (false).
///
virtual void OnEngineChannelUpdate(std::string name, bool listening) = 0;

//--------------------------------------------------------------------------
/// @brief Synchronously invokes platform-specific APIs to apply the
/// system text scaling on the given unscaled font size.
Expand Down Expand Up @@ -977,6 +988,9 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
std::weak_ptr<PlatformMessageHandler> GetPlatformMessageHandler()
const override;

// |RuntimeDelegate|
void SendChannelUpdate(std::string name, bool listening) override;

// |RuntimeDelegate|
double GetScaledFontSize(double unscaled_font_size,
int configuration_id) const override;
Expand Down
2 changes: 2 additions & 0 deletions shell/common/engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class MockDelegate : public Engine::Delegate {
MOCK_METHOD0(GetCurrentTimePoint, fml::TimePoint());
MOCK_CONST_METHOD0(GetPlatformMessageHandler,
const std::shared_ptr<PlatformMessageHandler>&());
MOCK_METHOD2(OnEngineChannelUpdate, void(std::string, bool));
MOCK_CONST_METHOD2(GetScaledFontSize,
double(double font_size, int configuration_id));
};
Expand Down Expand Up @@ -69,6 +70,7 @@ class MockRuntimeDelegate : public RuntimeDelegate {
MOCK_METHOD1(RequestDartDeferredLibrary, void(intptr_t));
MOCK_CONST_METHOD0(GetPlatformMessageHandler,
std::weak_ptr<PlatformMessageHandler>());
MOCK_METHOD2(SendChannelUpdate, void(std::string, bool));
MOCK_CONST_METHOD2(GetScaledFontSize,
double(double font_size, int configuration_id));
};
Expand Down
2 changes: 2 additions & 0 deletions shell/common/platform_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ void PlatformView::UpdateSemantics(
// NOLINTNEXTLINE(performance-unnecessary-value-param)
CustomAccessibilityActionUpdates actions) {}

void PlatformView::SendChannelUpdate(const std::string& name, bool listening) {}

void PlatformView::HandlePlatformMessage(
std::unique_ptr<PlatformMessage> message) {
if (auto response = message->response()) {
Expand Down
11 changes: 11 additions & 0 deletions shell/common/platform_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,17 @@ class PlatformView {
virtual void UpdateSemantics(SemanticsNodeUpdates updates,
CustomAccessibilityActionUpdates actions);

//----------------------------------------------------------------------------
/// @brief Used by the framework to tell the embedder that it has
/// registered a listener on a given channel.
///
/// @param[in] name The name of the channel on which the listener has
/// set or cleared a listener.
/// @param[in] listening True if a listener has been set, false if it has
/// been cleared.
///
virtual void SendChannelUpdate(const std::string& name, bool listening);

//----------------------------------------------------------------------------
/// @brief Used by embedders to specify the updated viewport metrics for
/// a view. In response to this call, on the raster thread, the
Expand Down
12 changes: 12 additions & 0 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,18 @@ void Shell::OnEngineHandlePlatformMessage(
}
}

void Shell::OnEngineChannelUpdate(std::string name, bool listening) {
FML_DCHECK(is_set_up_);
FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread());

task_runners_.GetPlatformTaskRunner()->PostTask(
[view = platform_view_->GetWeakPtr(), name = std::move(name), listening] {
if (view) {
view->SendChannelUpdate(name, listening);
}
});
}

void Shell::HandleEngineSkiaMessage(std::unique_ptr<PlatformMessage> message) {
const auto& data = message->data();

Expand Down
3 changes: 3 additions & 0 deletions shell/common/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,9 @@ class Shell final : public PlatformView::Delegate,
// |Engine::Delegate|
fml::TimePoint GetCurrentTimePoint() override;

// |Engine::Delegate|
void OnEngineChannelUpdate(std::string name, bool listening) override;

// |Engine::Delegate|
double GetScaledFontSize(double unscaled_font_size,
int configuration_id) const override;
Expand Down
12 changes: 12 additions & 0 deletions shell/platform/embedder/embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1881,6 +1881,17 @@ FlutterEngineResult FlutterEngineInitialize(size_t version,
user_data]() { return ptr(user_data); };
}

flutter::PlatformViewEmbedder::ChanneUpdateCallback channel_update_callback =
nullptr;
if (SAFE_ACCESS(args, channel_update_callback, nullptr) != nullptr) {
channel_update_callback = [ptr = args->channel_update_callback, user_data](
const std::string& name, bool listening) {
FlutterChannelUpdate update{sizeof(FlutterChannelUpdate), name.c_str(),
listening};
ptr(&update, user_data);
};
}

auto external_view_embedder_result = InferExternalViewEmbedderFromArgs(
SAFE_ACCESS(args, compositor, nullptr), settings.enable_impeller);
if (external_view_embedder_result.second) {
Expand All @@ -1895,6 +1906,7 @@ FlutterEngineResult FlutterEngineInitialize(size_t version,
vsync_callback, //
compute_platform_resolved_locale_callback, //
on_pre_engine_restart_callback, //
channel_update_callback, //
};

auto on_create_platform_view = InferPlatformViewCreationCallback(
Expand Down
19 changes: 19 additions & 0 deletions shell/platform/embedder/embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -1402,6 +1402,20 @@ typedef void (*FlutterUpdateSemanticsCallback2)(
const FlutterSemanticsUpdate2* /* semantics update */,
void* /* user data*/);

/// An update to whether a message channel has a listener set or not.
typedef struct {
// The size of the struct. Must be sizeof(FlutterChannelUpdate).
size_t struct_size;
/// The name of the channel.
const char* channel;
/// True if a listener has been set, false if one has been cleared.
bool listening;
} FlutterChannelUpdate;
Copy link
Member

Choose a reason for hiding this comment

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

Are we 100% certain that we'll never want to add more members to this struct? If not, add a size_t struct_size member as the first field in the struct and use safe dereference macros when reading from it.

Copy link
Member

Choose a reason for hiding this comment

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

Oh derp I completely forgot this 🤦. Yup let's add a struct_size to be safe


typedef void (*FlutterChannelUpdateCallback)(
const FlutterChannelUpdate* /* channel update */,
void* /* user data */);

typedef struct _FlutterTaskRunner* FlutterTaskRunner;

typedef struct {
Expand Down Expand Up @@ -2194,6 +2208,11 @@ typedef struct {
/// and `update_semantics_callback2` may be provided; the others must be set
/// to null.
FlutterUpdateSemanticsCallback2 update_semantics_callback2;

/// The callback invoked by the engine in response to a channel listener
/// being registered on the framework side. The callback is invoked from
/// a task posted to the UI task runner.
FlutterChannelUpdateCallback channel_update_callback;
} FlutterProjectArgs;

#ifndef FLUTTER_ENGINE_NO_PROTOTYPES
Expand Down
9 changes: 9 additions & 0 deletions shell/platform/embedder/fixtures/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1289,3 +1289,12 @@ void pointer_data_packet() {

signalNativeTest();
}

@pragma('vm:entry-point')
void channel_listener_response() async {
channelBuffers.setListener('test/listen',
(ByteData? data, PlatformMessageResponseCallback callback) {
callback(null);
});
signalNativeTest();
}
8 changes: 8 additions & 0 deletions shell/platform/embedder/platform_view_embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,14 @@ void PlatformViewEmbedder::OnPreEngineRestart() const {
}
}

// |PlatformView|
void PlatformViewEmbedder::SendChannelUpdate(const std::string& name,
bool listening) {
if (platform_dispatch_table_.on_channel_update != nullptr) {
platform_dispatch_table_.on_channel_update(name, listening);
}
}

std::shared_ptr<PlatformMessageHandler>
PlatformViewEmbedder::GetPlatformMessageHandler() const {
return platform_message_handler_;
Expand Down
5 changes: 5 additions & 0 deletions shell/platform/embedder/platform_view_embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class PlatformViewEmbedder final : public PlatformView {
std::function<std::unique_ptr<std::vector<std::string>>(
const std::vector<std::string>& supported_locale_data)>;
using OnPreEngineRestartCallback = std::function<void()>;
using ChanneUpdateCallback = std::function<void(const std::string&, bool)>;

struct PlatformDispatchTable {
UpdateSemanticsCallback update_semantics_callback; // optional
Expand All @@ -50,6 +51,7 @@ class PlatformViewEmbedder final : public PlatformView {
ComputePlatformResolvedLocaleCallback
compute_platform_resolved_locale_callback;
OnPreEngineRestartCallback on_pre_engine_restart_callback; // optional
ChanneUpdateCallback on_channel_update; // optional
};

// Create a platform view that sets up a software rasterizer.
Expand Down Expand Up @@ -134,6 +136,9 @@ class PlatformViewEmbedder final : public PlatformView {
std::unique_ptr<std::vector<std::string>> ComputePlatformResolvedLocales(
const std::vector<std::string>& supported_locale_data) override;

// |PlatformView|
void SendChannelUpdate(const std::string& name, bool listening) override;

FML_DISALLOW_COPY_AND_ASSIGN(PlatformViewEmbedder);
};

Expand Down
6 changes: 6 additions & 0 deletions shell/platform/embedder/tests/embedder_config_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ EmbedderConfigBuilder::EmbedderConfigBuilder(
SetSemanticsCallbackHooks();
SetLogMessageCallbackHook();
SetLocalizationCallbackHooks();
SetChannelUpdateCallbackHook();
AddCommandLineArgument("--disable-vm-service");

if (preference == InitializationPreference::kSnapshotsInitialize ||
Expand Down Expand Up @@ -263,6 +264,11 @@ void EmbedderConfigBuilder::SetLogMessageCallbackHook() {
EmbedderTestContext::GetLogMessageCallbackHook();
}

void EmbedderConfigBuilder::SetChannelUpdateCallbackHook() {
project_args_.channel_update_callback =
context_.GetChannelUpdateCallbackHook();
}

void EmbedderConfigBuilder::SetLogTag(std::string tag) {
log_tag_ = std::move(tag);
project_args_.log_tag = log_tag_.c_str();
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/embedder/tests/embedder_config_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ class EmbedderConfigBuilder {
// Used to set a custom log message handler.
void SetLogMessageCallbackHook();

void SetChannelUpdateCallbackHook();

// Used to set a custom log tag.
void SetLogTag(std::string tag);

Expand Down
19 changes: 19 additions & 0 deletions shell/platform/embedder/tests/embedder_test_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,11 @@ void EmbedderTestContext::SetPlatformMessageCallback(
platform_message_callback_ = callback;
}

void EmbedderTestContext::SetChannelUpdateCallback(
const ChannelUpdateCallback& callback) {
channel_update_callback_ = callback;
}

void EmbedderTestContext::PlatformMessageCallback(
const FlutterPlatformMessage* message) {
if (platform_message_callback_) {
Expand Down Expand Up @@ -232,6 +237,20 @@ EmbedderTestContext::GetComputePlatformResolvedLocaleCallbackHook() {
};
}

FlutterChannelUpdateCallback
EmbedderTestContext::GetChannelUpdateCallbackHook() {
if (channel_update_callback_ == nullptr) {
return nullptr;
}

return [](const FlutterChannelUpdate* update, void* user_data) {
auto context = reinterpret_cast<EmbedderTestContext*>(user_data);
if (context->channel_update_callback_) {
context->channel_update_callback_(update);
}
};
}

FlutterTransformation EmbedderTestContext::GetRootSurfaceTransformation() {
return FlutterTransformationMake(root_surface_transformation_);
}
Expand Down
Loading