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

yaakovschectman · cbracken · web-flow · commit 1feb9302050c · 2023-08-29T16:12:35.000-04:00
Supplant the temporary solution flutter/engine#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#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].  [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 <chris@bracken.jp>
diff --git a/lib/ui/channel_buffers.dart b/lib/ui/channel_buffers.dart
@@ -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.
@@ -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.
diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc
@@ -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)                          \
diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc
@@ -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) {
diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h
@@ -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.
@@ -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
diff --git a/lib/web_ui/lib/channel_buffers.dart b/lib/web_ui/lib/channel_buffers.dart
@@ -257,6 +257,8 @@ class ChannelBuffers {
       return true;
     }());
   }
+
+  void sendChannelUpdate(String name, {required bool listening}) {}
 }
 
 final ChannelBuffers channelBuffers = ChannelBuffers();
diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc
@@ -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;
diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h
@@ -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;
diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h
@@ -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;
 
diff --git a/shell/common/engine.cc b/shell/common/engine.cc
@@ -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,
diff --git a/shell/common/engine.h b/shell/common/engine.h
@@ -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.
@@ -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;
diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc
@@ -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));
 };
@@ -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));
 };
diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc
@@ -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()) {
diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h
@@ -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
diff --git a/shell/common/shell.cc b/shell/common/shell.cc
@@ -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();
 
diff --git a/shell/common/shell.h b/shell/common/shell.h
@@ -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;
diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc
@@ -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) {
@@ -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(
diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h
@@ -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 {
@@ -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
diff --git a/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart
@@ -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();
+}
diff --git a/shell/platform/embedder/platform_view_embedder.cc b/shell/platform/embedder/platform_view_embedder.cc
@@ -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_;
diff --git a/shell/platform/embedder/platform_view_embedder.h b/shell/platform/embedder/platform_view_embedder.h
@@ -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
@@ -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.
@@ -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);
 };
 
diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc
@@ -114,6 +114,7 @@ EmbedderConfigBuilder::EmbedderConfigBuilder(
     SetSemanticsCallbackHooks();
     SetLogMessageCallbackHook();
     SetLocalizationCallbackHooks();
+    SetChannelUpdateCallbackHook();
     AddCommandLineArgument("--disable-vm-service");
 
     if (preference == InitializationPreference::kSnapshotsInitialize ||
@@ -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();
diff --git a/shell/platform/embedder/tests/embedder_config_builder.h b/shell/platform/embedder/tests/embedder_config_builder.h
@@ -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);
 
diff --git a/shell/platform/embedder/tests/embedder_test_context.cc b/shell/platform/embedder/tests/embedder_test_context.cc
@@ -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_) {
@@ -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_);
 }
diff --git a/shell/platform/embedder/tests/embedder_test_context.h b/shell/platform/embedder/tests/embedder_test_context.h
diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc
diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc
diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h
diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc
diff --git a/shell/platform/windows/platform_handler.cc b/shell/platform/windows/platform_handler.cc
diff --git a/shell/platform/windows/windows_lifecycle_manager.cc b/shell/platform/windows/windows_lifecycle_manager.cc
diff --git a/shell/platform/windows/windows_lifecycle_manager.h b/shell/platform/windows/windows_lifecycle_manager.h

Original file line number	Diff line number	Diff line change
`@@ -257,6 +257,8 @@ class ChannelBuffers {`
`257`	`257`	`return true;`
`258`	`258`	`}());`
`259`	`259`	`}`
	`260`	`+`
	`261`	`+ void sendChannelUpdate(String name, {required bool listening}) {}`
`260`	`262`	`}`
`261`	`263`
`262`	`264`	`final ChannelBuffers channelBuffers = ChannelBuffers();`