Closed
Bug 1302037
Opened 8 years ago
Closed 8 years ago
StructuredClone should prohibit SAB in transfer map
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla53
People
(Reporter: lth, Assigned: lth)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(1 file)
(deleted),
patch
|
shu
:
review+
gchang
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
See bug #1302036 - This is the bug to prohibit a SAB from being in the transfer map. We will land this when the world is ready for it, but at the earliest one full release after the previous bug.
Assignee | ||
Comment 1•8 years ago
|
||
The change that allows the SAB to be omitted from the transfer map is in FF52 (and hopefully in FF51). So this one should land no sooner than FF54.
Assignee: nobody → lhansen
Priority: -- → P3
Assignee | ||
Updated•8 years ago
|
Assignee: lhansen → nobody
Updated•8 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•8 years ago
|
Blocks: shared-array-buffer
Assignee | ||
Comment 2•8 years ago
|
||
Every indication is that partner content has been updated. Tutorial materials and specs and test suites have been updated for sure. Emscripten shipped the change in October 2016.
So we want this change for FF53 and FF54. The spec calls for a DataCloneError to be thrown, which may require a new string (not sure - do we localize JS error messages?)
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
OS: Unspecified → All
Priority: P3 → P1
Hardware: Unspecified → All
Target Milestone: --- → mozilla53
Assignee | ||
Comment 3•8 years ago
|
||
Just convert the existing warning to an error.
I have verified that this throws a DOMException with name DataCloneError in the browser.
(There may be more test cases to adapt here, TBD.)
Attachment #8835929 -
Flags: review?(shu)
Assignee | ||
Comment 4•8 years ago
|
||
Updated•8 years ago
|
Attachment #8835929 -
Flags: review?(shu) → review+
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba8f78a8b0e58fa75939aaf4bf2470b0c29b74dd
Bug 1302037 - Don't allow SAB in transfer map. r=shu
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8835929 [details] [diff] [review]
bug1302037-no-sab-in-transfer.patch
Approval Request Comment
[Feature/Bug causing the regression]:
Shared memory for JS
[User impact if declined]:
Some incorrect programs will be accepted; compatibility risk.
[Is this code covered by automated tests?]:
Yes.
[Has the fix been verified in Nightly?]:
On inbound as I file this.
[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?]:
No.
[Why is the change risky/not risky?]:
There is little content on the web using this feature because no browsers have shipped it enabled-by-default yet; we control most of the test cases and tool chains, and they have been updated already.
[String changes made/needed]:
None.
Attachment #8835929 -
Flags: approval-mozilla-aurora?
Comment 7•8 years ago
|
||
bugherder |
Updated•8 years ago
|
Keywords: site-compat
Comment 8•8 years ago
|
||
SharedArrayBuffer has been disabled so far; no real site-compat effect.
Keywords: site-compat
Updated•8 years ago
|
status-firefox53:
--- → affected
Comment 9•8 years ago
|
||
Hi :lth,
Per comment #8, do we need to uplift this to Aurora53?
Flags: needinfo?(lhansen)
Assignee | ||
Comment 10•8 years ago
|
||
Yes, please. See comment #6. Comments 7 and 8 appear to be just noise.
Flags: needinfo?(lhansen)
Comment 11•8 years ago
|
||
Comment on attachment 8835929 [details] [diff] [review]
bug1302037-no-sab-in-transfer.patch
Reduce the compatibility risk. Aurora53+.
Attachment #8835929 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 12•8 years ago
|
||
bugherder uplift |
Comment 13•8 years ago
|
||
Updated
https://developer.mozilla.org/en-US/Firefox/Releases/53#JavaScript
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•