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

[CP][stable channel][beta channel] Mitigate unspecified behavior in DDC with Chrome 86 #43885

Closed
sigmundch opened this issue Oct 22, 2020 · 8 comments
Assignees
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta merge-to-stable

Comments

@sigmundch
Copy link
Member

commit(s) to merge: 34b485c

merge instructions: clean merge

What is the issue: Chrome 86 has several breaking changes that may affect DDC. Some affect DDC during load time (like the one we fixed and cherry-picked 2 months ago: #43287) and some only if certain browser APIs are used (like getGamepads: #43750).

The reason these changes broke DDC is that DDC adds interceptors to browser APIs by updating the prototype of the underlying browser object. If that prototype is removed (which is what happened with MemoryInfo and GamepadList), DDC accidentally updates Object.prototype, which may affect all objects in the program.

It's hard to predict what kind of errors follows: with MemoryInfo, invariants about dart2js were broken. With GamepadList all dynamic calls in the program were broken in DDC.

What is the fix: 34b485c mitigates the problem, but wont fix individual APIs. It makes sure that when adding interceptors to browser APIs, they are added on a valid prototype and not on Object's prototype.

Why cherrypick: Developers that use the APIs changed by Chrome 86 see a very bizarre error far removed from the issue, almost as a memory corruption error would look. For example, #43750 shows that all dynamic calls break.

Risk: low. Change is pretty minimal and already rolled to internal users.

Link to original issue(s): #43750 #43193

/cc @kevmoo @mit-mit @whesse @athomas @vsmenon @franklinyow @Markzipan @srujzs

@sigmundch sigmundch added merge-to-stable cherry-pick-review Issue that need cherry pick triage to approve labels Oct 22, 2020
@sigmundch
Copy link
Member Author

note: this request is for the stable channel, but it may be worth cherry-picking into the beta channel as well.

@franklinyow
Copy link
Contributor

@pcsosinski

@franklinyow
Copy link
Contributor

Approved + merge to beta

@franklinyow franklinyow added the cherry-pick-approved Label for approved cherrypick request label Oct 22, 2020
@franklinyow franklinyow changed the title [CP][stable channel] Mitigate unspecified behavior in DDC with Chrome 86 [CP][stable channel][beta channel] Mitigate unspecified behavior in DDC with Chrome 86 Oct 22, 2020
@pcsosinski
Copy link

what's the timeline for this being in the wild? our beta build is targeted to be re-cut from master in the beginning of Nov.

@sigmundch
Copy link
Member Author

Chrome 86 reached the stable channel on Oct 6, so it is already out in the wild.

On the earlier part of the scrum meeting we were debating whether to include the beta or not. We thought that the new beta cut in Nov wont be released until at least mid November, we feared that it would be a while until it reaches flutter web users, some of whom have seen related issues (like in dart-lang/webdev#1133)

@athomas
Copy link
Member

athomas commented Oct 27, 2020

Merged to stable for 2.10.3 (ecf9ce8).

@franklinyow Do we want to make a beta release for this as well (the label indicates it)? This would be the only issue to merge to beta.

@franklinyow
Copy link
Contributor

@athomas
Yes, please release to beta as well. Thanks

@devoncarew devoncarew added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Oct 28, 2020
@athomas
Copy link
Member

athomas commented Oct 29, 2020

Merged to beta for 2.11.0-213.5.beta (d93fe00).

@athomas athomas closed this as completed Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta merge-to-stable
Projects
None yet
Development

No branches or pull requests

5 participants