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

browser(firefox): follow-up with assorted simplifications #4066

Merged

Conversation

aslushnikov
Copy link
Contributor

This patch:

  • moves SimpleChannel to synchronously dispatch buffered commands
    instead of a await Promise.resolve() hack
  • moves dialog & screencast handling from PageHandler to
    TargetManager. This leaves PageHandler to be concerned solely about
    protocol.
  • removes attach and detach methods for worker channels: since
    channels are buffering messages until the namespace registers, there's
    no chance to loose any events.
  • slightly simplifies PageNetwork class: it's lifetime is now
    identical to the lifetime of the associated PageTarget, so a lot can
    be simplified later on.

References #3995

This patch:
- moves `SimpleChannel` to synchronously dispatch buffered commands
  instead of a `await Promise.resolve()` hack
- moves dialog & screencast handling from `PageHandler` to
  `TargetManager`. This leaves `PageHandler` to be concerned solely about
  protocol.
- removes `attach` and `detach` methods for worker channels: since
  channels are buffering messages until the namespace registers, there's
  no chance to loose any events.
- slightly simplifies `PageNetwork` class: it's lifetime is now
  identical to the lifetime of the associated `PageTarget`, so a lot can
  be simplified later on.

References microsoft#3995
@aslushnikov aslushnikov merged commit 4ab66a4 into microsoft:master Oct 6, 2020
@aslushnikov aslushnikov deleted the fff-various-simplifications branch October 6, 2020 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants