Closed Bug 1302037 Opened 4 years ago Closed 3 years ago

StructuredClone should prohibit SAB in transfer map

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: lth, Assigned: lth)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

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.
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: lhansen → nobody
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
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)
Attachment #8835929 - Flags: review?(shu) → review+
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?
https://hg.mozilla.org/mozilla-central/rev/ba8f78a8b0e5
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Keywords: site-compat
SharedArrayBuffer has been disabled so far; no real site-compat effect.
Keywords: site-compat
Hi :lth,
Per comment #8, do we need to uplift this to Aurora53?
Flags: needinfo?(lhansen)
Yes, please.  See comment #6.  Comments 7 and 8 appear to be just noise.
Flags: needinfo?(lhansen)
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+
You need to log in before you can comment on or make changes to this bug.