StructuredClone should prohibit SAB in transfer map

RESOLVED FIXED in Firefox 53

Status

()

defect
P1
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: lth, Assigned: lth)

Tracking

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

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed, firefox54 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

3 years ago
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

3 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

3 years ago
Assignee: lhansen → nobody
Assignee

Updated

2 years ago
Assignee

Comment 2

2 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

2 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)

Updated

2 years ago
Attachment #8835929 - Flags: review?(shu) → review+
Assignee

Comment 6

2 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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ba8f78a8b0e5
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
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)
Assignee

Comment 10

2 years ago
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.