[go: nahoru, domu]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor platform message logic #22181

Merged
merged 1 commit into from
Nov 5, 2020

Conversation

Hixie
Copy link
Contributor
@Hixie Hixie commented Oct 29, 2020

This laids the groundwork for sending messages through ChannelBuffers with channel-specific callbacks rather than a single onPlatformMessage callback.

This allows us to remove the logic from the framework that puts data back into the channel buffers. Right now (before this PR) the logic for messages from plugins to the framework is bidirectional:

                  **                 *
Plugins -> Engine -> ChannelBuffers <- Framework <---+-.
               |              |                      | |
               |              '------> via drain ----' |
               |                                       |
               '----------------- onPlatformMessage ---'

* = when the listener is null on the framework side
** = when onPlatformMessage is null

This ends up with weird race conditions and is generally less than completely clear. With this PR, we lay the groundwork for eventually reaching this model:

Plugins -> Engine -> ChannelBuffers -> Framework

...which is significantly simpler.

@google-cla google-cla bot added the cla: yes label Oct 29, 2020
@Hixie Hixie force-pushed the ChannelBuffers2 branch 4 times, most recently from 9e9bb48 to 12e38a7 Compare October 29, 2020 23:26
@Hixie
Copy link
Contributor Author
Hixie commented Oct 29, 2020

Closes flutter/flutter#66699

This laids the groundwork for sending messages through ChannelBuffers with channel-specific callbacks rather than a single onPlatformMessage callback.

This allows us to remove the logic from the framework that puts data back into the channel buffers. Right now (before this PR) the logic for messages from plugins to the framework is bidirectional:

```
                  **                 *
Plugins -> Engine -> ChannelBuffers <- Framework <---+-.
               |              |                      | |
               |              '------> via drain ----' |
               |                                       |
               '----------------- onPlatformMessage ---'

* = when the listener is null on the framework side
** = when onPlatformMessage is null
```

This ends up with weird race conditions and is generally less than completely clear. With this PR, we lay the groundwork for eventually reaching this model:

```
Plugins -> Engine -> ChannelBuffers -> Framework
```

...which is significantly simpler.
@Hixie Hixie marked this pull request as ready for review October 30, 2020 02:01
@Hixie
Copy link
Contributor Author
Hixie commented Nov 2, 2020

This is ready to go if someone LGTMs it (see #21572 for past reviews).

Copy link
Contributor
@nturgut nturgut left a comment

Choose a reason for hiding this comment

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

LGTM for the changes effecting web_ui platform channel calls.

Optional: Is there an issue/issues for TODO items that we can add instead of developer ldap?

Copy link
Contributor
@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

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

LGTM

@chinmaygarde chinmaygarde added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Nov 5, 2020
@fluttergithubbot fluttergithubbot merged commit ad79d23 into flutter:master Nov 5, 2020
@Hixie Hixie deleted the ChannelBuffers2 branch November 5, 2020 23:56
@Hixie
Copy link
Contributor Author
Hixie commented Nov 5, 2020

@nturgut I'll be taking care of them in the near future.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2020
chaselatta pushed a commit to chaselatta/engine that referenced this pull request Nov 30, 2020
auto-submit bot pushed a commit that referenced this pull request Jun 15, 2023
Fixes old TODOs originally added in #22181.
The framework appears to be fully migrated off these.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
5 participants