1496378 - Make ArrayBufferViewObject a base class of TypedArrayObject and DataViewObject

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Jan de Mooij [:jandem]]

DataViewObject and TypedArrayObject have the same object layout. In fact, DataViewObject uses the TypedArrayObject::FOO_SLOT constants but this dependency isn't obvious.

For bug 1065894 it would be nice to move these constants (and some methods that are currently duplicated) into a shared base class. We actually already have a class for this, ArrayBufferViewObject, but it's not a base class of DataViewObject/TypedArrayObject.

Comment 1

[Jan de Mooij [:jandem]]

One issue is that ArrayBufferViewObject is also used for typed objects sometimes, even though obj->is<ArrayBufferViewObject>() only returns true for TypedArrays and DataViews (and we have code relying on that). We're using static_casts.

(Also: ArrayBufferViewObject inherits from NativeObject and typed objects aren't native.)

I'm not sure yet what to do about this - we could split ArrayBufferViewObject in ArrayBufferViewObject and NativeArrayBufferViewObject but that's becoming pretty verbose.

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #1)
> One issue is that ArrayBufferViewObject is also used for typed objects
> sometimes, even though obj->is<ArrayBufferViewObject>() only returns true
> for TypedArrays and DataViews (and we have code relying on that). We're
> using static_casts.

I think the right thing to do for now is to use JSObject instead of the (invalid) static_cast hackery for typed objects.

I'll post some patches so we can see what people think.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1496378 part 1 - Make ArrayBufferViewObject a base class of TypedArrayObject and DataViewObject. r?jwalden

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1496378 part 2 - Move data pointer accessors from DataViewObject/TypedArrayObject to ArrayBufferViewObject. r?jwalden

Note that DataViewObject had dataPointer* methods but TypedArrayObject used
viewData* for this. I used dataPointer* for consistency with ArrayBufferObject
(and I like it more).

Depends on D7721

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1496378 part 3 - Move buffer methods from DataViewObject/TypedArrayObject to ArrayBufferViewObject. r?jwalden

Depends on D7722

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1496378 part 4 - Move ArrayBufferViewObject and related APIs into vm/ArrayBufferViewObject.{h,cpp}. r?jwalden

Depends on D7723

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1496378 part 5 - Clean up ArrayBufferViewObject::trace. r?jwalden

Depends on D7724

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1496378 part 6 - Remove TypedObject to ArrayBufferViewObject static_cast hackery. r?jwalden

Depends on D7727

Comment 9

[Jan de Mooij [:jandem]]

With these patches we have:

* ArrayBufferViewObject is a base class of TypedArrayObject and DataViewObject.

* ArrayBufferObject's view list stores JSObjects (ArrayBufferViewObject or TypedObject).

I'm not thrilled about ArrayBufferViewObject not being used for *all* views (although one could argue that for TypedObjects the buffer is more of an implementation detail), but more refactoring/renaming will be easier to do on top of these patches.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/87071cf93bcc
part 1 - Make ArrayBufferViewObject a base class of TypedArrayObject and DataViewObject. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/1f8af7db9d96
part 2 - Move data pointer accessors from DataViewObject/TypedArrayObject to ArrayBufferViewObject. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/e6c5e6c15848
part 3 - Move buffer methods from DataViewObject/TypedArrayObject to ArrayBufferViewObject. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/992c0bd2f327
part 4 - Move ArrayBufferViewObject and related APIs into vm/ArrayBufferViewObject.{h,cpp}. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/b449c4991dc4
part 5 - Clean up ArrayBufferViewObject::trace. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/8886fad423dc
part 6 - Remove TypedObject to ArrayBufferViewObject static_cast hackery. r=jwalden

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/87071cf93bcc
https://hg.mozilla.org/mozilla-central/rev/1f8af7db9d96
https://hg.mozilla.org/mozilla-central/rev/e6c5e6c15848
https://hg.mozilla.org/mozilla-central/rev/992c0bd2f327
https://hg.mozilla.org/mozilla-central/rev/b449c4991dc4
https://hg.mozilla.org/mozilla-central/rev/8886fad423dc