Started handling messages from background isolates. by gaaclarke · Pull Request #109005 · flutter/flutter

gaaclarke · 2022-08-04T23:27:47Z

companion engine pr: flutter/engine#35174
design doc: https://docs.google.com/document/d/1yAFw-6kBefuurXWTur9jdEUAckWiWJVukP1Iay8ehyU/edit#
issue: #13937

example

main.dart

import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'dart:isolate';

void isolateMain(BackgroundIsolateBinding binding) async {
  BackgroundIsolateBinding.initializeBackgroundIsolate(binding);
  const BasicMessageChannel channel =
      BasicMessageChannel('dev.flutter.aaclarke.echo', StandardMessageCodec());
  final String result = await channel.send('hello');
  print(result);
}

void main() {
  final BackgroundIsolateBinding binding =
      BackgroundIsolateBinding.initializeRootIsolate();
  Isolate.spawn(isolateMain, binding);
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: const MyHomePage(title: 'Isolate Platform Channels'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});
  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: Container(),
    );
  }
}

AppDelegate.swift

import UIKit
import Flutter
import os.log

@UIApplicationMain
@objc class AppDelegate: FlutterAppDelegate {
  override func application(
    _ application: UIApplication,
    didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?
  ) -> Bool {
    let registrar = self.registrar(forPlugin: "echo");
    let channel = FlutterBasicMessageChannel(name: "dev.flutter.aaclarke.echo", binaryMessenger: registrar!.messenger());
    channel.setMessageHandler { (input, reply) in
      if let input_string = input as? String? {
        os_log("%@", input_string ?? "null");
      } else {
        os_log("unrecognized message");
      }
      reply(input)
    }
    GeneratedPluginRegistrant.register(with: self)
    return super.application(application, didFinishLaunchingWithOptions: launchOptions)
  }
}

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

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

Hixie · 2022-08-15T21:49:34Z

The overall idea seems fine. I'm not sure I understand the specifics of the implementation on the Dart side. Maybe we can go over the PR together on VC?

Hixie · 2022-08-18T19:44:58Z

@gaaclarke here's a proposed reworking of the API as discussed in our chat:
master...Hixie:flutter:isolate-platform-channels

It's semantically equivalent to the original code, just presented differently.

gaaclarke · 2022-08-19T01:05:55Z

@gaaclarke here's a proposed reworking of the API as discussed in our chat: master...Hixie:flutter:isolate-platform-channels

It's semantically equivalent to the original code, just presented differently.

Thanks, I've incorporated most of it now to see how it worked out. I still need to clean it up and incorporate your docs. I made 2 changes:

Not requiring the binary messenger in the channel constructor as we discussed in 04194fd#commitcomment-81568761
I put the logic for acquiring the the RootIsolateToken on that class and started exporting it from services/platform_channels.dart. I wanted to make sure that importing services was sufficient to allowing one to use platform channels on background isolates.

packages/flutter/lib/src/services/platform_channel.dart

Hixie · 2022-08-19T03:24:58Z

packages/flutter/lib/src/services/platform_channel.dart

i would advise using the pattern i had on my branch to make this a static method rather than a closure. That'll avoid accidentally closing over things we don't want to close over, but also generally the code will be easier to reason about (and it makes the satisfying pattern of having both the send and the receive logic be parallel).

packages/flutter/lib/src/services/platform_channel.dart

gaaclarke · 2022-09-09T17:38:40Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

False positive, there are integration tests.

dnfield

LGTM

gaaclarke · 2022-09-09T23:19:52Z

Over the course of responding to feedback I broke the integration test which apparently doesn't run as a presubmit. Reverting to open the tree.

This reverts commit 0d19d46.

…flutter#109005)

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Aug 4, 2022

gaaclarke mentioned this pull request Aug 4, 2022

Started handling messages from background isolates for iOS flutter/engine#35174

Merged

8 tasks

gaaclarke force-pushed the isolate-platform-channels branch 6 times, most recently from 563a5e3 to 8cce92e Compare August 12, 2022 17:26

gaaclarke force-pushed the isolate-platform-channels branch 3 times, most recently from a18df55 to 7c5cc9a Compare August 12, 2022 22:20

gaaclarke force-pushed the isolate-platform-channels branch from f516ee3 to c7dbcde Compare August 19, 2022 01:00

Hixie reviewed Aug 19, 2022

View reviewed changes