[pigeon] Update task queue handling (#8627)

Updates the task queue implementation across all supporting generators to use one task queue for an entire host API, rather than one per method, so that multiple methods using task queues will have ordering guarantees amongst them. Also changes the Obj-C implementation to no-op on macOS instead of failing to compile, so that shared iOS/macOS code can use task queues on iOS without breaking macOS. (We could deliberately add a compile error with a clear error message in a macOS ifdef if we get reports of user confusion in the future, but my expectation is that no-op is the best option for most people.)

Related changes:
- Adds integration testing that task queues work, instead of just that the generator step succeeds.
  - For platforms that don't support task queues, they are still included in the integration test to ensure that 
  - I removed the now-vestigial background_platform_channels.dart.
- Adds Swift support, since it was easier to add than to skip it in the tests.

Fixes flutter/flutter#162624
Fixes flutter/flutter#111512

Author: stuartmorgan-g

packages/pigeon/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 24.2.0
+
+* Adjusts task queues to use a shared task queue for all methods in a single
+  API instance, to give the same ordering guarantees as non-task-queue usage.
+* [swift] Adds task queue support to the Swift generator.
+
 ## 24.1.1
 
 * [swift, kotlin] Adds an error message when a ProxyAPI callback method that returns a non-null

packages/pigeon/lib/src/generator_tools.dart

@@ -14,7 +14,7 @@ import 'ast.dart';
 /// The current version of pigeon.
 ///
 /// This must match the version in pubspec.yaml.
-const String pigeonVersion = '24.1.1';
+const String pigeonVersion = '24.2.0';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {

packages/pigeon/lib/src/java/java_generator.dart

@@ -6,7 +6,7 @@ import '../ast.dart';
 import '../functional.dart';
 import '../generator.dart';
 import '../generator_tools.dart';
-import '../pigeon_lib.dart' show TaskQueueType;
+import '../types/task_queue.dart';
 
 /// Documentation open symbol.
 const String _docCommentPrefix = '/**';
@@ -835,15 +835,21 @@ if (wrapped == null) {
       indent.addScoped('{', '}', () {
         indent.writeln(
             'messageChannelSuffix = messageChannelSuffix.isEmpty() ? "" : "." + messageChannelSuffix;');
+        String? serialBackgroundQueue;
+        if (api.methods.any((Method m) =>
+            m.taskQueueType == TaskQueueType.serialBackgroundThread)) {
+          serialBackgroundQueue = 'taskQueue';
+          indent.writeln(
+              'BinaryMessenger.TaskQueue $serialBackgroundQueue = binaryMessenger.makeBackgroundTaskQueue();');
+        }
         for (final Method method in api.methods) {
-          _writeMethodSetUp(
-            generatorOptions,
-            root,
-            indent,
-            api,
-            method,
-            dartPackageName: dartPackageName,
-          );
+          _writeHostMethodMessageHandler(
+              generatorOptions, root, indent, api, method,
+              dartPackageName: dartPackageName,
+              serialBackgroundQueue:
+                  method.taskQueueType == TaskQueueType.serialBackgroundThread
+                      ? serialBackgroundQueue
+                      : null);
         }
       });
     });
@@ -887,34 +893,27 @@ if (wrapped == null) {
     indent.writeln('$returnType ${method.name}(${argSignature.join(', ')});');
   }
 
-  /// Write a static setUp function in the interface.
-  /// Example:
-  ///   static void setUp(BinaryMessenger binaryMessenger, Foo api) {...}
-  void _writeMethodSetUp(
+  /// Write a single method's handler for the setUp function.
+  void _writeHostMethodMessageHandler(
     JavaOptions generatorOptions,
     Root root,
     Indent indent,
     Api api,
     final Method method, {
     required String dartPackageName,
+    String? serialBackgroundQueue,
   }) {
     final String channelName = makeChannelName(api, method, dartPackageName);
     indent.write('');
     indent.addScoped('{', '}', () {
-      String? taskQueue;
-      if (method.taskQueueType != TaskQueueType.serial) {
-        taskQueue = 'taskQueue';
-        indent.writeln(
-            'BinaryMessenger.TaskQueue taskQueue = binaryMessenger.makeBackgroundTaskQueue();');
-      }
       indent.writeln('BasicMessageChannel<Object> channel =');
       indent.nest(2, () {
         indent.writeln('new BasicMessageChannel<>(');
         indent.nest(2, () {
           indent.write(
               'binaryMessenger, "$channelName" + messageChannelSuffix, getCodec()');
-          if (taskQueue != null) {
-            indent.addln(', $taskQueue);');
+          if (serialBackgroundQueue != null) {
+            indent.addln(', $serialBackgroundQueue);');
           } else {
             indent.addln(');');
           }

packages/pigeon/lib/src/kotlin/kotlin_generator.dart

@@ -8,7 +8,7 @@ import '../ast.dart';
 import '../functional.dart';
 import '../generator.dart';
 import '../generator_tools.dart';
-import '../pigeon_lib.dart' show TaskQueueType;
+import '../types/task_queue.dart';
 import 'templates.dart';
 
 /// Documentation open symbol.
@@ -654,6 +654,13 @@ if (wrapped == null) {
         indent.addScoped('{', '}', () {
           indent.writeln(
               r'val separatedMessageChannelSuffix = if (messageChannelSuffix.isNotEmpty()) ".$messageChannelSuffix" else ""');
+          String? serialBackgroundQueue;
+          if (api.methods.any((Method m) =>
+              m.taskQueueType == TaskQueueType.serialBackgroundThread)) {
+            serialBackgroundQueue = 'taskQueue';
+            indent.writeln(
+                'val $serialBackgroundQueue = binaryMessenger.makeBackgroundTaskQueue()');
+          }
           for (final Method method in api.methods) {
             _writeHostMethodMessageHandler(
               indent,
@@ -664,6 +671,10 @@ if (wrapped == null) {
               parameters: method.parameters,
               returnType: method.returnType,
               isAsynchronous: method.isAsynchronous,
+              serialBackgroundQueue:
+                  method.taskQueueType == TaskQueueType.serialBackgroundThread
+                      ? serialBackgroundQueue
+                      : null,
             );
           }
         });
@@ -1071,8 +1082,8 @@ if (wrapped == null) {
           fun error(errorCode: String, errorMessage: String?, errorDetails: Any?) {
             sink.error(errorCode, errorMessage, errorDetails)
           }
-  
-          fun endOfStream() { 
+
+          fun endOfStream() {
             sink.endOfStream()
           }
         }
@@ -1249,24 +1260,18 @@ if (wrapped == null) {
     required TypeDeclaration returnType,
     String setHandlerCondition = 'api != null',
     bool isAsynchronous = false,
+    String? serialBackgroundQueue,
     String Function(List<String> safeArgNames, {required String apiVarName})?
         onCreateCall,
   }) {
     indent.write('run ');
     indent.addScoped('{', '}', () {
-      String? taskQueue;
-      if (taskQueueType != TaskQueueType.serial) {
-        taskQueue = 'taskQueue';
-        indent.writeln(
-            'val $taskQueue = binaryMessenger.makeBackgroundTaskQueue()');
-      }
-
       indent.write(
         'val channel = BasicMessageChannel<Any?>(binaryMessenger, "$channelName", codec',
       );
 
-      if (taskQueue != null) {
-        indent.addln(', $taskQueue)');
+      if (serialBackgroundQueue != null) {
+        indent.addln(', $serialBackgroundQueue)');
       } else {
         indent.addln(')');
       }

packages/pigeon/lib/src/objc/objc_generator.dart

@@ -6,7 +6,8 @@ import '../ast.dart';
 import '../functional.dart';
 import '../generator.dart';
 import '../generator_tools.dart';
-import '../pigeon_lib.dart' show Error, TaskQueueType;
+import '../pigeon_lib.dart' show Error;
+import '../types/task_queue.dart';
 
 /// Documentation comment open symbol.
 const String _docCommentPrefix = '///';
@@ -860,24 +861,33 @@ if (self.wrapped == nil) {
     indent.addScoped('{', '}', () {
       indent.writeln(
           'messageChannelSuffix = messageChannelSuffix.length > 0 ? [NSString stringWithFormat: @".%@", messageChannelSuffix] : @"";');
+      String? serialBackgroundQueue;
+      if (api.methods.any((Method m) =>
+          m.taskQueueType == TaskQueueType.serialBackgroundThread)) {
+        serialBackgroundQueue = 'taskQueue';
+        // See https://github.com/flutter/flutter/issues/162613 for why this
+        // is an ifdef instead of just a respondsToSelector: check.
+        indent.format('''
+#if TARGET_OS_IOS
+  NSObject<FlutterTaskQueue> *$serialBackgroundQueue = [binaryMessenger makeBackgroundTaskQueue];
+#else
+  NSObject<FlutterTaskQueue> *$serialBackgroundQueue = nil;
+#endif''');
+      }
       for (final Method func in api.methods) {
         addDocumentationComments(
             indent, func.documentationComments, _docCommentSpec);
 
         indent.writeScoped('{', '}', () {
-          String? taskQueue;
-          if (func.taskQueueType != TaskQueueType.serial) {
-            taskQueue = 'taskQueue';
-            indent.writeln(
-                'NSObject<FlutterTaskQueue> *$taskQueue = [binaryMessenger makeBackgroundTaskQueue];');
-          }
           _writeChannelAllocation(
             generatorOptions,
             indent,
             api,
             func,
             channelName,
-            taskQueue,
+            func.taskQueueType == TaskQueueType.serialBackgroundThread
+                ? serialBackgroundQueue
+                : null,
             dartPackageName: dartPackageName,
           );
           indent.write('if (api) ');
@@ -1112,7 +1122,15 @@ static FlutterError *createConnectionError(NSString *channelName) {
 
         if (taskQueue != null) {
           indent.newln();
-          indent.addln('taskQueue:$taskQueue];');
+          // See https://github.com/flutter/flutter/issues/162613 for why this
+          // is in an ifdef instead of just relying on the parameter being
+          // nullable.
+          indent.format('''
+#ifdef TARGET_OS_IOS
+taskQueue:$taskQueue
+#endif
+];
+''');
         } else {
           indent.addln('];');
         }

packages/pigeon/lib/src/pigeon_lib.dart

@@ -36,6 +36,9 @@ import 'java/java_generator.dart';
 import 'kotlin/kotlin_generator.dart';
 import 'objc/objc_generator.dart';
 import 'swift/swift_generator.dart';
+import 'types/task_queue.dart';
+
+export 'types/task_queue.dart' show TaskQueueType;
 
 class _Asynchronous {
   const _Asynchronous();
@@ -211,21 +214,6 @@ class SwiftClass {
   const SwiftClass();
 }
 
-/// Type of TaskQueue which determines how handlers are dispatched for
-/// HostApi's.
-enum TaskQueueType {
-  /// Handlers are invoked serially on the default thread. This is the value if
-  /// unspecified.
-  serial,
-
-  /// Handlers are invoked serially on a background thread.
-  serialBackgroundThread,
-
-  // TODO(gaaclarke): Add support for concurrent task queues.
-  // /// Handlers are invoked concurrently on a background thread.
-  // concurrentBackgroundThread,
-}
-
 /// Metadata annotation to control how handlers are dispatched for HostApi's.
 /// Note that the TaskQueue API might not be available on the target version of
 /// Flutter, see also:

packages/pigeon/lib/src/swift/swift_generator.dart

@@ -9,6 +9,7 @@ import '../ast.dart';
 import '../functional.dart';
 import '../generator.dart';
 import '../generator_tools.dart';
+import '../types/task_queue.dart';
 import 'templates.dart';
 
 /// Documentation comment open symbol.
@@ -736,6 +737,21 @@ if (wrapped == nil) {
       indent.addScoped('{', '}', () {
         indent.writeln(
             r'let channelSuffix = messageChannelSuffix.count > 0 ? ".\(messageChannelSuffix)" : ""');
+        String? serialBackgroundQueue;
+        if (api.methods.any((Method m) =>
+            m.taskQueueType == TaskQueueType.serialBackgroundThread)) {
+          serialBackgroundQueue = 'taskQueue';
+          // TODO(stuartmorgan): Remove the ? once macOS supports task queues
+          // and this is no longer an optional protocol method.
+          // See https://github.com/flutter/flutter/issues/162613 for why this
+          // is an ifdef instead of just relying on the optionality check.
+          indent.format('''
+#if os(iOS)
+  let $serialBackgroundQueue = binaryMessenger.makeBackgroundTaskQueue?()
+#else
+  let $serialBackgroundQueue: FlutterTaskQueue? = nil
+#endif''');
+        }
         for (final Method method in api.methods) {
           _writeHostMethodMessageHandler(
             indent,
@@ -747,6 +763,10 @@ if (wrapped == nil) {
             isAsynchronous: method.isAsynchronous,
             swiftFunction: method.swiftFunction,
             documentationComments: method.documentationComments,
+            serialBackgroundQueue:
+                method.taskQueueType == TaskQueueType.serialBackgroundThread
+                    ? serialBackgroundQueue
+                    : null,
           );
         }
       });
@@ -1404,7 +1424,7 @@ private func nilOrValue<T>(_ value: Any?) -> T? {
         func endOfStream() {
           sink(FlutterEndOfEventStream)
         }
-      
+
       }
       ''');
     }
@@ -1413,7 +1433,7 @@ private func nilOrValue<T>(_ value: Any?) -> T? {
     for (final Method func in api.methods) {
       indent.format('''
         class ${toUpperCamelCase(func.name)}StreamHandler: PigeonEventChannelWrapper<${_swiftTypeForDartType(func.returnType)}> {
-          static func register(with messenger: FlutterBinaryMessenger, 
+          static func register(with messenger: FlutterBinaryMessenger,
                               instanceName: String = "",
                               streamHandler: ${toUpperCamelCase(func.name)}StreamHandler) {
             var channelName = "${makeChannelName(api, func, dartPackageName)}"
@@ -1535,6 +1555,7 @@ private func nilOrValue<T>(_ value: Any?) -> T? {
     required TypeDeclaration returnType,
     required bool isAsynchronous,
     required String? swiftFunction,
+    String? serialBackgroundQueue,
     String setHandlerCondition = 'let api = api',
     List<String> documentationComments = const <String>[],
     String Function(List<String> safeArgNames, {required String apiVarName})?
@@ -1549,8 +1570,30 @@ private func nilOrValue<T>(_ value: Any?) -> T? {
 
     final String varChannelName = '${name}Channel';
     addDocumentationComments(indent, documentationComments, _docCommentSpec);
-    indent.writeln(
-        'let $varChannelName = FlutterBasicMessageChannel(name: "$channelName", binaryMessenger: binaryMessenger, codec: codec)');
+    final String baseArgs = 'name: "$channelName", '
+        'binaryMessenger: binaryMessenger, codec: codec';
+    // The version with taskQueue: is an optional protocol method that isn't
+    // implemented on macOS yet, so the call has to be conditionalized even
+    // though the taskQueue argument is nullable. The runtime branching can be
+    // removed once macOS supports task queues. The condition is on the task
+    // queue variable not being nil because the earlier code to set it will
+    // return nil on macOS where the optional parts of the protocol are not
+    // implemented.
+    final String channelCreationWithoutTaskQueue =
+        'FlutterBasicMessageChannel($baseArgs)';
+    if (serialBackgroundQueue == null) {
+      indent.writeln('let $varChannelName = $channelCreationWithoutTaskQueue');
+    } else {
+      final String channelCreationWithTaskQueue =
+          'FlutterBasicMessageChannel($baseArgs, taskQueue: $serialBackgroundQueue)';
+
+      indent.write('let $varChannelName = $serialBackgroundQueue == nil');
+      indent.addScoped('', '', () {
+        indent.writeln('? $channelCreationWithoutTaskQueue');
+        indent.writeln(': $channelCreationWithTaskQueue');
+      });
+    }
+
     indent.write('if $setHandlerCondition ');
     indent.addScoped('{', '}', () {
       indent.write('$varChannelName.setMessageHandler ');

packages/pigeon/lib/src/types/task_queue.dart

@@ -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.
+
+/// Type of TaskQueue which determines how handlers are dispatched for
+/// HostApi's.
+enum TaskQueueType {
+  /// Handlers are invoked serially on the default thread. This is the value if
+  /// unspecified.
+  serial,
+
+  /// Handlers are invoked serially on a background thread.
+  serialBackgroundThread,
+
+  // TODO(gaaclarke): Add support for concurrent task queues.
+  // /// Handlers are invoked concurrently on a background thread.
+  // concurrentBackgroundThread,
+}