789594 - Implement structured cloning of DataViews

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

The current implementation does not allow cloning DataViews. Bug 789593 should be implemented before this bug, though.

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Implement DataView cloning

Does what it says on the tin.

Comment 4

[Sebastian Markbåge]

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

[Jeff Walden [:Waldo]]

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

Comment 6

[Jeff Walden [:Waldo]]

(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 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/65b1a0d77304

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/65b1a0d77304