Closed
Bug 789594
Opened 12 years ago
Closed 9 years ago
Implement structured cloning of DataViews
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: sfink, Assigned: sfink)
References
Details
(Whiteboard: [js:t])
Attachments
(1 file)
(deleted),
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
The current implementation does not allow cloning DataViews. Bug 789593 should be implemented before this bug, though.
Updated•12 years ago
|
Whiteboard: [js:t]
Updated•10 years ago
|
Assignee: general → nobody
Assignee | ||
Comment 3•9 years ago
|
||
Does what it says on the tin.
Attachment #8640255 -
Flags: review?(jwalden+bmo)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → sphink
Status: NEW → ASSIGNED
Comment 4•9 years ago
|
||
To be clear, it should also allow the underlying buffer to be transferred.
var worker = new Worker('test.js');
var buffer = new ArrayBuffer(10);
var dataView = new DataView(buffer);
worker.postMessage(dataView, [buffer]);
Comment 5•9 years ago
|
||
Comment on attachment 8640255 [details] [diff] [review]
Implement DataView cloning
Review of attachment 8640255 [details] [diff] [review]:
-----------------------------------------------------------------
Come one, come all, get your cargo-cult reviews here! Nothing but the finest in hackish comparisons against similar cases in existing code, backed only by handwavy understanding of the approaches being implemented, without deep grokking sufficient to be fully confident of correctness! The greatest farce in SpiderMonkey!
...okay, not quite that bad, but really all I did was compare what you did to what's done now for typed arrays, without real familiarity with that code or deep understanding of the structured clone implementation techniques. :-)
::: js/src/gdb/mozilla/prettyprinters.py
@@ +173,5 @@
> self.void_t = gdb.lookup_type('void')
> self.void_ptr_t = self.void_t.pointer()
> try:
> self.JSString_ptr_t = gdb.lookup_type('JSString').pointer()
> + # self.JSSymbol_ptr_t = gdb.lookup_type('JS::Symbol').pointer()
This seems unrelated?
::: js/src/vm/StructuredClone.cpp
@@ +1446,5 @@
> + // Read byteOffset.
> + uint64_t n;
> + if (!in.read(&n))
> + return false;
> + uint32_t byteOffset = n;
Use mozilla::AssertedCast<uint32_t>(n) from #include "mozilla/Casting.h" here. (Bonus points for a patch doing that everywhere in this file, actually.)
Attachment #8640255 -
Flags: review?(jwalden+bmo) → review+
Comment 6•9 years ago
|
||
(In reply to Sebastian Markbåge from comment #4)
> To be clear, it should also allow the underlying buffer to be transferred.
Assuming the existing typed array structured cloning code does that -- and I'm 99.99999% sure it does -- the cargo-culted code in this patch should do so as well, never fear.
Comment 8•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•