Closed
Bug 1342060
Opened 7 years ago
Closed 7 years ago
wasm: enable by default
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: luke, Assigned: luke)
References
Details
(Keywords: dev-doc-complete)
Attachments
(5 files, 1 obsolete file)
19.98 KB,
patch
|
luke
:
review+
jorendorff
:
superreview+
|
Details | Diff | Splinter Review |
17.42 KB,
patch
|
luke
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
14.57 KB,
patch
|
luke
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
7.13 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
7.31 KB,
patch
|
Details | Diff | Splinter Review |
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.
Comment 1•7 years ago
|
||
\o/
Updated•7 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 2•7 years ago
|
||
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 3•7 years ago
|
||
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•7 years 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•7 years ago
|
||
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+
Updated•7 years ago
|
Attachment #8840510 -
Flags: superreview?(jorendorff) → superreview+
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•7 years ago
|
||
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•7 years ago
|
||
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 9•7 years ago
|
||
Comment on attachment 8840545 [details] [diff] [review] enable-by-default Enable all the wasms!
Attachment #8840545 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•7 years ago
|
||
Comment on attachment 8840543 [details] [diff] [review] aurora patch Aurora wasm incoming!
Attachment #8840543 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 11•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/94916848475d
status-firefox53:
--- → fixed
Flags: in-testsuite+
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/65887172cead
status-firefox52:
--- → fixed
Comment hidden (admin-reviewed, advocacy) |
Assignee | ||
Comment 14•7 years ago
|
||
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 15•7 years ago
|
||
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•7 years 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•7 years 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•7 years ago
|
||
(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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/391047948db4 https://hg.mozilla.org/mozilla-central/rev/0187aa7969c7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 22•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/65887172cead https://hg.mozilla.org/releases/mozilla-esr52/rev/c8288de78a8d
status-firefox-esr52:
--- → fixed
Comment 23•7 years ago
|
||
This was re-disabled for ESR52 in bug 1342440.
Comment 24•7 years ago
|
||
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.
Description
•