Skip to content

Commit

Permalink
Add callback to Embedder API to respond to new channel listeners, and…
Browse files Browse the repository at this point in the history
… use for Windows lifecycle (flutter#44827)

Supplant the temporary solution
flutter#44238 with a more elegant
solution with am embedder API callback. The windows engine provides a
callback that enables graceful exit and app lifecycle when the platform
and lifecycle channels are listened to, respectively.

flutter/flutter#131616

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

---------

Co-authored-by: Chris Bracken <[email protected]>
  • Loading branch information
yaakovschectman and cbracken authored Aug 29, 2023
1 parent 01a1579 commit 1feb930
Show file tree
Hide file tree
Showing 31 changed files with 269 additions and 24 deletions.
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;

//--------------------------------------------------------------------------
/// @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);

//--------------------------------------------------------------------------
/// @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;

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

0 comments on commit 1feb930

Please sign in to comment.