wasm: enable by default

RESOLVED FIXED in Firefox 52

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: luke, Assigned: luke)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

unspecified
mozilla54
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 fixed, firefox53 fixed, firefox54 fixed, firefox-esr52 disabled)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Assignee)

Description

3 months ago
Chrome has already enabled wasm by default on all channels, the binary format and API are stable, and there's no blockers I'm aware of for enabling starting with 52.
\o/
Keywords: dev-doc-needed
(Assignee)

Comment 2

3 months ago
Created attachment 8840486 [details] [diff] [review]
enable-by-default

This patch both enables wasm and stops accepting the experimental 0xd version (only 0x1 is accepted now).  Because of this, a number of tests had to be updated that baked in 0xd.
Attachment #8840486 - Flags: review?(bbouvier)
Comment on attachment 8840486 [details] [diff] [review]
enable-by-default

Review of attachment 8840486 [details] [diff] [review]:
-----------------------------------------------------------------

The technical parts look great to me.
Attachment #8840486 - Flags: review?(bbouvier) → review+
(Assignee)

Comment 4

3 months ago
Comment on attachment 8840486 [details] [diff] [review]
enable-by-default

Requesting sr for enabling this feature in SM.
Attachment #8840486 - Flags: superreview?(jorendorff)
(Assignee)

Comment 5

3 months ago
Created attachment 8840510 [details] [diff] [review]
enable-by-default

Oops, try server found some extra tests that needed to be modified.
Attachment #8840486 - Attachment is obsolete: true
Attachment #8840486 - Flags: superreview?(jorendorff)
Attachment #8840510 - Flags: superreview?(jorendorff)
Attachment #8840510 - Flags: review+
Attachment #8840510 - Flags: superreview?(jorendorff) → superreview+

Comment 6

3 months ago
Pushed by lwagner@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/391047948db4
wasm: enable by default (r=bbouvier,sr=jorendorff)
(Assignee)

Comment 7

3 months ago
Created attachment 8840543 [details] [diff] [review]
aurora patch

Approval Request Comment
[Feature/Bug causing the regression]: enabling wasm
[User impact if declined]: no wasm
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not with the patch itself; shipping a new feature is inherently risky, of course, but this is what we've been addressing with several forms of fuzzing, large compiled codebases, shared test suites, all of which has applied to what's in aurora/beta)
Attachment #8840543 - Flags: review+
Attachment #8840543 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 8

3 months ago
Created attachment 8840545 [details] [diff] [review]
enable-by-default

Approval Request Comment
[Feature/Bug causing the regression]: enabling wasm on beta
[User impact if declined]: no wasm
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: see comment 7
[Why is the change risky/not risky?]: see comment 7
Attachment #8840545 - Flags: review+
Attachment #8840545 - Flags: approval-mozilla-beta?
Comment on attachment 8840545 [details] [diff] [review]
enable-by-default

Enable all the wasms!
Attachment #8840545 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8840543 [details] [diff] [review]
aurora patch

Aurora wasm incoming!
Attachment #8840543 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 11

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/94916848475d
status-firefox53: --- → fixed
Flags: in-testsuite+

Comment 12

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/65887172cead
status-firefox52: --- → fixed
Comment hidden (admin-reviewed, advocacy)
(Assignee)

Comment 14

3 months ago
Created attachment 8840636 [details] [diff] [review]
fix-android-tests

Oops, I missed in the try results that on some of the old android versions we test on, wasm compilation is disabled and so the global WebAssembly object is not exposed on those platforms.  This patch fixes the window/worker/service-worker cases.  In a window, we can use SpecialPowers to ensure that the reason for a missing WebAssembly object is because it's not supported by the platform.  But in workers, there are no SpecialPowers so the best I could think of was to just mark it as "optional".
Attachment #8840636 - Flags: review?(jorendorff)
Comment on attachment 8840636 [details] [diff] [review]
fix-android-tests

Review of attachment 8840636 [details] [diff] [review]:
-----------------------------------------------------------------

OK.
Attachment #8840636 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 16

3 months ago
(In reply to Douglas Crosher [:dougc] from comment #13)
Your dispute is noted.  I understand that there are many things that you would like to have changed in wasm.  At this point, though, browsers have converged on a common, stable core that can serve as a first shippable unit and that is already enabled by default in Chrome.  Going forward, there's a lot of room for incremental improvement and it will be much easier to make these complex value judgements once we see a lot more kinds of usage of wasm which we can use to evaluate future options.

Comment 17

3 months ago
Pushed by lwagner@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0187aa7969c7
Followup to 391047948db4 to fix Android tests (r=jorendorff)
(Assignee)

Comment 18

3 months ago
Created attachment 8840652 [details] [diff] [review]
fix-android-tests branch patch

(The test code fixed by the follow-up patch moved between m-i and m-a.)
Comment hidden (advocacy, off-topic)
Uplifted the followup patch:
https://hg.mozilla.org/releases/mozilla-aurora/rev/166b520f6e4cd78b8cdbbb49febc740bca491b20
https://hg.mozilla.org/releases/mozilla-beta/rev/c8288de78a8d7fd44ea7487521d69a51444700eb

Comment 21

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/391047948db4
https://hg.mozilla.org/mozilla-central/rev/0187aa7969c7
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54

Comment 22

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/65887172cead
https://hg.mozilla.org/releases/mozilla-esr52/rev/c8288de78a8d
status-firefox-esr52: --- → fixed
This was re-disabled for ESR52 in bug 1342440.
status-firefox-esr52: fixed → disabled
https://developer.mozilla.org/en-US/docs/WebAssembly#Browser_compatibility
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.