Closed Bug 1462466 Opened 2 years ago Closed 2 years ago

fix ServiceWorker::PostMessage() when e10s pref is flipped

Categories

(Core :: DOM: Service Workers, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

(Whiteboard: SW-MUST)

Attachments

(4 files, 8 obsolete files)

7.84 KB, patch
baku
: review+
Details | Diff | Splinter Review
13.83 KB, patch
baku
: review+
Details | Diff | Splinter Review
1.03 KB, patch
baku
: review+
Details | Diff | Splinter Review
5.03 KB, patch
baku
: review+
Details | Diff | Splinter Review
Bug 1459209 implements ServiceWorker::PostMessage(), but testing shows that it hits assertions and does not actually work.  I'll fix this in a separate bug here since the changes will be a bit intrusive and I don't want to stop the overall reviews going on in the other bug.  Since its off behind a pref it can be broken for a bit.
Depends on: 1462467
Let's make sure this doesn't break anything with the e10s pref disabled:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e15eb41dfdffbc5020093cf0e0336dbfb06b16e
Andrea, this patch adds a new helper ServiceWorkerCloneData class.  Its a ref-counted StructuredCloneData implementation.  It also proxy releases the SharedJSAllocatedData on the correct source thread.  Please see the header comment for more information.
Attachment #8976707 - Attachment is obsolete: true
Attachment #8977032 - Flags: review?(amarchesini)
This patch makes us use the new ServiceWorkerCloneData class for the ServiceWorker::PostMessage() path.  This nicely avoids having to do the re-packing in ServiceWorkerPrivate.  This patch updates both the legacy and new e10s pref'd path added in bug 1459209.
Attachment #8976708 - Attachment is obsolete: true
Attachment #8977035 - Flags: review?(amarchesini)
This fixes a typo in StructuredCloneData::CopyFromCloneMessageDataForBackgroundParent().  We need this to work properly for the e10s pref'd path.
Attachment #8976709 - Attachment is obsolete: true
Comment on attachment 8977036 [details] [diff] [review]
P3 Fix StructuredCloneData::CopyFromCloneMessageDataForBackgroundParent() to use CopyMemory instead of BorrowMemory. r=baku

See comment 8.
Attachment #8977036 - Flags: review?(amarchesini)
We don't need the sandbox creation code in ServiceWorkerPrivate any more.
Attachment #8977028 - Attachment is obsolete: true
Attachment #8977039 - Flags: review?(amarchesini)
Priority: -- → P2
Attachment #8977032 - Flags: review?(amarchesini)
Attachment #8977035 - Flags: review?(amarchesini)
Attachment #8977036 - Flags: review?(amarchesini)
Attachment #8977039 - Flags: review?(amarchesini)
I'm going to land this before the IPC code to make things simpler.
Blocks: 1459209
No longer depends on: 1459209, 1462467
Comment on attachment 8987105 [details] [diff] [review]
P1 Add a ServiceWorkerCloneData type with threadsafe ref-counting. r=baku

Andrea, this patch adds a new helper ServiceWorkerCloneData class.  Its a ref-counted StructuredCloneData implementation.  It also proxy releases the SharedJSAllocatedData on the correct source thread.  Please see the header comment for more information.
Attachment #8987105 - Flags: review?(amarchesini)
Comment on attachment 8987106 [details] [diff] [review]
P2 Make ServiceWorker::PostMessage() code use the ServiceWorkerCloneData class. r=baku

This patch makes us use the new ServiceWorkerCloneData class for the ServiceWorker::PostMessage() path.  This nicely avoids having to do the re-packing in ServiceWorkerPrivate.
Attachment #8987106 - Flags: review?(amarchesini)
Comment on attachment 8987107 [details] [diff] [review]
P3 Fix StructuredCloneData::CopyFromCloneMessageDataForBackgroundParent() to use CopyMemory instead of BorrowMemory. r=baku

This fixes a typo in StructuredCloneData::CopyFromCloneMessageDataForBackgroundParent().
Attachment #8987107 - Flags: review?(amarchesini)
Comment on attachment 8987108 [details] [diff] [review]
P4 Remove stale sandbox code from ServiceWorkerPrivate. r=baku

We don't need the sandbox creation code in ServiceWorkerPrivate any more.
Attachment #8987108 - Flags: review?(amarchesini)
Whiteboard: SW-MUST
Attachment #8987105 - Flags: review?(amarchesini) → review+
Attachment #8987106 - Flags: review?(amarchesini) → review+
Attachment #8987107 - Flags: review?(amarchesini) → review+
Attachment #8987108 - Flags: review?(amarchesini) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/24cbb43eea80
P1 Add a ServiceWorkerCloneData type with threadsafe ref-counting. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/948ef44381f3
P2 Make ServiceWorker::PostMessage() code use the ServiceWorkerCloneData class. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/e71c74469159
P3 Fix StructuredCloneData::CopyFromCloneMessageDataForBackgroundParent() to use CopyMemory instead of BorrowMemory. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/101fb5a63031
P4 Remove stale sandbox code from ServiceWorkerPrivate. r=baku
You need to log in before you can comment on or make changes to this bug.