Refactor platform message logic (flutter#22181)

matanlurey · Nov 5, 2020 · ad79d23 · ad79d23
1 parent 2c975c0
commit ad79d23
Show file tree

Hide file tree

Showing 14 changed files with 1,421 additions and 404 deletions.
diff --git a/lib/ui/channel_buffers.dart b/lib/ui/channel_buffers.dart
diff --git a/lib/ui/hooks.dart b/lib/ui/hooks.dart
@@ -159,6 +159,10 @@ void _invoke(void Function()? callback, Zone zone) {
 }
 
 /// Invokes [callback] inside the given [zone] passing it [arg].
+///
+/// The 1 in the name refers to the number of arguments expected by
+/// the callback (and thus passed to this function, in addition to the
+/// callback itself and the zone in which the callback is executed).
 void _invoke1<A>(void Function(A a)? callback, Zone zone, A arg) {
   if (callback == null) {
     return;
@@ -173,14 +177,33 @@ void _invoke1<A>(void Function(A a)? callback, Zone zone, A arg) {
   }
 }
 
+/// Invokes [callback] inside the given [zone] passing it [arg1] and [arg2].
+///
+/// The 2 in the name refers to the number of arguments expected by
+/// the callback (and thus passed to this function, in addition to the
+/// callback itself and the zone in which the callback is executed).
+void _invoke2<A1, A2>(void Function(A1 a1, A2 a2)? callback, Zone zone, A1 arg1, A2 arg2) {
+  if (callback == null) {
+    return;
+  }
+
+  assert(zone != null); // ignore: unnecessary_null_comparison
+
+  if (identical(zone, Zone.current)) {
+    callback(arg1, arg2);
+  } else {
+    zone.runGuarded(() {
+      callback(arg1, arg2);
+    });
+  }
+}
+
 /// Invokes [callback] inside the given [zone] passing it [arg1], [arg2], and [arg3].
-void _invoke3<A1, A2, A3>(
-  void Function(A1 a1, A2 a2, A3 a3)? callback,
-  Zone zone,
-  A1 arg1,
-  A2 arg2,
-  A3 arg3,
-) {
+///
+/// The 3 in the name refers to the number of arguments expected by
+/// the callback (and thus passed to this function, in addition to the
+/// callback itself and the zone in which the callback is executed).
+void _invoke3<A1, A2, A3>(void Function(A1 a1, A2 a2, A3 a3)? callback, Zone zone, A1 arg1, A2 arg2, A3 arg3) {
   if (callback == null) {
     return;
   }

diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart
@@ -38,6 +38,7 @@ typedef SemanticsActionCallback = void Function(int id, SemanticsAction action,
 typedef PlatformMessageResponseCallback = void Function(ByteData? data);
 
 /// Signature for [PlatformDispatcher.onPlatformMessage].
+// TODO(ianh): deprecate once framework uses [ChannelBuffers.setListener].
 typedef PlatformMessageCallback = void Function(String name, ByteData? data, PlatformMessageResponseCallback? callback);
 
 // Signature for _setNeedsReportTimings.
@@ -409,6 +410,8 @@ class PlatformDispatcher {
   ///
   /// The framework invokes this callback in the same zone in which the callback
   /// was set.
+  // TODO(ianh): Deprecate onPlatformMessage once the framework is moved over
+  // to using channel buffers exclusively.
   PlatformMessageCallback? get onPlatformMessage => _onPlatformMessage;
   PlatformMessageCallback? _onPlatformMessage;
   Zone _onPlatformMessageZone = Zone.root;
@@ -438,13 +441,15 @@ class PlatformDispatcher {
     };
   }
 
-  // Called from the engine, via hooks.dart
+  /// Send a message to the framework using the [ChannelBuffers].
+  ///
+  /// This method constructs the appropriate callback to respond
+  /// with the given `responseId`. It should only be called for messages
+  /// from the platform.
   void _dispatchPlatformMessage(String name, ByteData? data, int responseId) {
     if (name == ChannelBuffers.kControlChannelName) {
       try {
         channelBuffers.handleMessage(data!);
-      } catch (ex) {
-        _printDebug('Message to "$name" caused exception $ex');
       } finally {
         _respondToPlatformMessage(responseId, null);
       }
@@ -454,7 +459,7 @@ class PlatformDispatcher {
         _onPlatformMessageZone,
         name,
         data,
-            (ByteData? responseData) {
+        (ByteData? responseData) {
           _respondToPlatformMessage(responseId, responseData);
         },
       );

diff --git a/lib/ui/text.dart b/lib/ui/text.dart
@@ -2273,11 +2273,18 @@ final ByteData _fontChangeMessage = utf8.encoder.convert(
 ).buffer.asByteData();
 
 FutureOr<void> _sendFontChangeMessage() async {
-  PlatformDispatcher.instance.onPlatformMessage?.call(
-    'flutter/system',
-    _fontChangeMessage,
-    (_) {},
-  );
+  const String kSystemChannelName = 'flutter/system';
+  if (PlatformDispatcher.instance.onPlatformMessage != null) {
+    _invoke3<String, ByteData?, PlatformMessageResponseCallback>(
+      PlatformDispatcher.instance.onPlatformMessage,
+      PlatformDispatcher.instance._onPlatformMessageZone,
+      kSystemChannelName,
+      _fontChangeMessage,
+      (ByteData? responseData) { },
+    );
+  } else {
+    channelBuffers.push(kSystemChannelName, _fontChangeMessage, (ByteData? responseData) { });
+  }
 }
 
 // TODO(gspencergoog): remove this template block once the framework templates

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
@@ -696,6 +696,7 @@ class SingletonFlutterWindow extends FlutterWindow {
   ///
   /// The framework invokes this callback in the same zone in which the
   /// callback was set.
+  // TODO(ianh): deprecate once framework uses [ChannelBuffers.setListener].
   PlatformMessageCallback? get onPlatformMessage => platformDispatcher.onPlatformMessage;
   set onPlatformMessage(PlatformMessageCallback? callback) {
     platformDispatcher.onPlatformMessage = callback;

diff --git a/lib/web_ui/lib/src/engine/keyboard.dart b/lib/web_ui/lib/src/engine/keyboard.dart
@@ -81,10 +81,6 @@ class Keyboard {
 
     final html.KeyboardEvent keyboardEvent = event;
 
-    if (EnginePlatformDispatcher.instance._onPlatformMessage == null) {
-      return;
-    }
-
     if (_shouldPreventDefault(event)) {
       event.preventDefault();
     }

diff --git a/lib/web_ui/lib/src/engine/navigation/history.dart b/lib/web_ui/lib/src/engine/navigation/history.dart
@@ -151,17 +151,15 @@ class MultiEntriesBrowserHistory extends BrowserHistory {
           currentPath);
     }
     _lastSeenSerialCount = _currentSerialCount;
-    if (EnginePlatformDispatcher.instance._onPlatformMessage != null) {
-      EnginePlatformDispatcher.instance.invokeOnPlatformMessage(
-        'flutter/navigation',
-        const JSONMethodCodec().encodeMethodCall(
-            MethodCall('pushRouteInformation', <dynamic, dynamic>{
-          'location': currentPath,
-          'state': event.state?['state'],
-        })),
-        (_) {},
-      );
-    }
+    EnginePlatformDispatcher.instance.invokeOnPlatformMessage(
+      'flutter/navigation',
+      const JSONMethodCodec().encodeMethodCall(
+          MethodCall('pushRouteInformation', <dynamic, dynamic>{
+        'location': currentPath,
+        'state': event.state?['state'],
+      })),
+      (_) {},
+    );
   }
 
   @override
@@ -272,13 +270,11 @@ class SingleEntryBrowserHistory extends BrowserHistory {
       _setupFlutterEntry(urlStrategy!);
 
       // 2. Send a 'popRoute' platform message so the app can handle it accordingly.
-      if (EnginePlatformDispatcher.instance._onPlatformMessage != null) {
-        EnginePlatformDispatcher.instance.invokeOnPlatformMessage(
-          'flutter/navigation',
-          const JSONMethodCodec().encodeMethodCall(_popRouteMethodCall),
-          (_) {},
-        );
-      }
+      EnginePlatformDispatcher.instance.invokeOnPlatformMessage(
+        'flutter/navigation',
+        const JSONMethodCodec().encodeMethodCall(_popRouteMethodCall),
+        (_) {},
+      );
     } else if (_isFlutterEntry(event.state)) {
       // We get into this scenario when the user changes the url manually. It
       // causes a new entry to be pushed on top of our "flutter" one. When this
@@ -291,15 +287,13 @@ class SingleEntryBrowserHistory extends BrowserHistory {
       _userProvidedRouteName = null;
 
       // Send a 'pushRoute' platform message so the app handles it accordingly.
-      if (EnginePlatformDispatcher.instance._onPlatformMessage != null) {
-        EnginePlatformDispatcher.instance.invokeOnPlatformMessage(
-          'flutter/navigation',
-          const JSONMethodCodec().encodeMethodCall(
-            MethodCall('pushRoute', newRouteName),
-          ),
-          (_) {},
-        );
-      }
+      EnginePlatformDispatcher.instance.invokeOnPlatformMessage(
+        'flutter/navigation',
+        const JSONMethodCodec().encodeMethodCall(
+          MethodCall('pushRoute', newRouteName),
+        ),
+        (_) {},
+      );
     } else {
       // The user has pushed a new entry on top of our flutter entry. This could
       // happen when the user modifies the hash part of the url directly, for