Skip to content

Conversation

@gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Sep 16, 2022

Relands #111702 with conditional compilation to remove dart:isolatefrom the web target.

issue: #13937

WARNING: the added integration test doesn't run in presubmit.

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.

@flutter-dashboard flutter-dashboard bot added framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Sep 16, 2022
@gaaclarke gaaclarke force-pushed the reland-isolate-platform-channels-2 branch 3 times, most recently from 1372248 to 1af8300 Compare September 16, 2022 17:22
@gaaclarke gaaclarke force-pushed the reland-isolate-platform-channels-2 branch from 1af8300 to 9aef5cc Compare September 16, 2022 17:32
@gaaclarke gaaclarke marked this pull request as ready for review September 16, 2022 17:48
@flutter-dashboard

This comment was marked as off-topic.

// found in the LICENSE file.

import 'dart:async' show Completer;
import 'dart:isolate' show ReceivePort;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the important line, now isolate is only brought in through a conditional import.


import 'dart:async';
import 'dart:developer';
import 'dart:isolate' show ReceivePort;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: the removed import.

/// [RootIsolateToken] as its argument. The value `null` is returned when
/// executed from background isolates.
ui.RootIsolateToken? get rootIsolateToken => ui.RootIsolateToken.instance;
static ui.RootIsolateToken? get rootIsolateToken => ui.RootIsolateToken.instance;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I switched this to static since it doesn't make sense to have a ServiceBinding.instance to call it because we want to call it from background isolates where there will be no ServiceBinding.

@gaaclarke gaaclarke requested a review from dnfield September 16, 2022 17:52
@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 16, 2022
@auto-submit
Copy link
Contributor

auto-submit bot commented Sep 16, 2022

auto label is removed for flutter/flutter, pr: 111712, due to - The status or check suite Windows framework_tests_misc has failed. Please fix the issues identified (or deflake) before re-applying this label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants