DevHeads.net

Review Request 108929: khtml/kjs: Implement typed-arrays

Review request for kdelibs.

Description
khtml/kjs: Implement typed-arrays

ArrayBuffer, Int8Array, Uint8Array, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array

I tested many configuration under valgrind, to find all possible errors.
It seems other browsers don't set a limit, so you can eat all memory with these arrays (danger), so khtml/kjs also doesn't set a limit. If you feel like we really need one, please tell.

Diffs
khtml/CMakeLists.txt 99034cc
khtml/ecma/kjs_arraybuffer.h PRE-CREATION
khtml/ecma/kjs_arraybuffer.cpp PRE-CREATION
khtml/ecma/kjs_arraybufferview.h PRE-CREATION
khtml/ecma/kjs_arraybufferview.cpp PRE-CREATION
khtml/ecma/kjs_window.h 1b70be1
khtml/ecma/kjs_window.cpp 927d251

Diff: <a href="http://git.reviewboard.kde.org/r/108929/diff/" title="http://git.reviewboard.kde.org/r/108929/diff/">http://git.reviewboard.kde.org/r/108929/diff/</a>

Testing

Thanks,

Bernd Buschinski

Comments

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Bernd Buschinski at 02/12/2013 - 18:51

(Updated Feb. 12, 2013, 10:51 p.m.)

Review request for kdelibs.

Changes
- unsigned long -> size_t
- long -> ssize_t
- more c++ static_cast
- correct (c) + year
- minor comment fix

Description
khtml/kjs: Implement typed-arrays

ArrayBuffer, Int8Array, Uint8Array, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array

I tested many configuration under valgrind, to find all possible errors.
It seems other browsers don't set a limit, so you can eat all memory with these arrays (danger), so khtml/kjs also doesn't set a limit. If you feel like we really need one, please tell.

Diffs (updated)
khtml/CMakeLists.txt 99034cc
khtml/ecma/kjs_arraybuffer.h PRE-CREATION
khtml/ecma/kjs_arraybuffer.cpp PRE-CREATION
khtml/ecma/kjs_arraybufferview.h PRE-CREATION
khtml/ecma/kjs_arraybufferview.cpp PRE-CREATION
khtml/ecma/kjs_window.h 1b70be1
khtml/ecma/kjs_window.cpp 927d251

Diff: <a href="http://git.reviewboard.kde.org/r/108929/diff/" title="http://git.reviewboard.kde.org/r/108929/diff/">http://git.reviewboard.kde.org/r/108929/diff/</a>

Testing

Thanks,

Bernd Buschinski

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Andrea Iacovitti at 02/13/2013 - 05:13

khtml/ecma/kjs_arraybufferview.h
<http://git.reviewboard.kde.org/r/108929/#comment20578>

Is this include really necessary or just an accident? (Noticed because a compilation error)

- Andrea Iacovitti

On Feb. 12, 2013, 10:51 p.m., Bernd Buschinski wrote:

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Bernd Buschinski at 02/18/2013 - 20:03

Heh, yes kdevelop :)

- Bernd

On Feb. 19, 2013, 12:03 a.m., Bernd Buschinski wrote:

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Milian Wolff at 02/14/2013 - 11:07

Do you use KDevelop? ;-) Second report I hear today of accidental includes sneaking in...

- Milian

On Feb. 13, 2013, 8:18 p.m., Bernd Buschinski wrote:

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Bernd Buschinski at 02/13/2013 - 09:39

ha, good catch, no clue how this sneaked in

- Bernd

On Feb. 12, 2013, 10:51 p.m., Bernd Buschinski wrote:

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Bernd Buschinski at 02/13/2013 - 09:39

(Updated Feb. 13, 2013, 1:39 p.m.)

Review request for kdelibs.

Changes
- remove unneeded tiff.h include

Description
khtml/kjs: Implement typed-arrays

ArrayBuffer, Int8Array, Uint8Array, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array

I tested many configuration under valgrind, to find all possible errors.
It seems other browsers don't set a limit, so you can eat all memory with these arrays (danger), so khtml/kjs also doesn't set a limit. If you feel like we really need one, please tell.

Diffs (updated)
khtml/ecma/kjs_arraybufferview.h PRE-CREATION
khtml/ecma/kjs_arraybuffer.cpp PRE-CREATION
khtml/ecma/kjs_arraybuffer.h PRE-CREATION
khtml/CMakeLists.txt 99034cc
khtml/ecma/kjs_arraybufferview.cpp PRE-CREATION
khtml/ecma/kjs_window.h 1b70be1
khtml/ecma/kjs_window.cpp 927d251

Diff: <a href="http://git.reviewboard.kde.org/r/108929/diff/" title="http://git.reviewboard.kde.org/r/108929/diff/">http://git.reviewboard.kde.org/r/108929/diff/</a>

Testing

Thanks,

Bernd Buschinski

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Bernd Buschinski at 02/13/2013 - 16:19

(Updated Feb. 13, 2013, 8:18 p.m.)

Review request for kdelibs.

Changes
- micro optimize, remember bufferStart instead of calculating it for every access
- use PropertySlot::setCustomIndex to remember the index of parsing it again

Description
khtml/kjs: Implement typed-arrays

ArrayBuffer, Int8Array, Uint8Array, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array

I tested many configuration under valgrind, to find all possible errors.
It seems other browsers don't set a limit, so you can eat all memory with these arrays (danger), so khtml/kjs also doesn't set a limit. If you feel like we really need one, please tell.

Diffs (updated)
khtml/CMakeLists.txt 99034cc
khtml/ecma/kjs_arraybuffer.h PRE-CREATION
khtml/ecma/kjs_arraybuffer.cpp PRE-CREATION
khtml/ecma/kjs_arraybufferview.h PRE-CREATION
khtml/ecma/kjs_arraybufferview.cpp PRE-CREATION
khtml/ecma/kjs_window.h 1b70be1
khtml/ecma/kjs_window.cpp 927d251

Diff: <a href="http://git.reviewboard.kde.org/r/108929/diff/" title="http://git.reviewboard.kde.org/r/108929/diff/">http://git.reviewboard.kde.org/r/108929/diff/</a>

Testing

Thanks,

Bernd Buschinski

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Bernd Buschinski at 02/18/2013 - 20:03

(Updated Feb. 19, 2013, 12:03 a.m.)

Review request for kdelibs.

Changes
- fixed ssize_t -> size_t where appropriate
- Reimplemented ArrayBuffer::Slice and ArrayBufferView::Subarray,
they are supposed to handle negative start/end, "it refers to an index from the end of the array"

Description
khtml/kjs: Implement typed-arrays

ArrayBuffer, Int8Array, Uint8Array, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array

I tested many configuration under valgrind, to find all possible errors.
It seems other browsers don't set a limit, so you can eat all memory with these arrays (danger), so khtml/kjs also doesn't set a limit. If you feel like we really need one, please tell.

Diffs (updated)
khtml/ecma/kjs_window.cpp 927d251
khtml/ecma/kjs_window.h 1b70be1
khtml/ecma/kjs_arraybufferview.h PRE-CREATION
khtml/ecma/kjs_arraybufferview.cpp PRE-CREATION
khtml/ecma/kjs_arraybuffer.cpp PRE-CREATION
khtml/CMakeLists.txt 99034cc
khtml/ecma/kjs_arraybuffer.h PRE-CREATION

Diff: <a href="http://git.reviewboard.kde.org/r/108929/diff/" title="http://git.reviewboard.kde.org/r/108929/diff/">http://git.reviewboard.kde.org/r/108929/diff/</a>

Testing

Thanks,

Bernd Buschinski

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Commit Hook at 03/30/2013 - 11:14

(Updated March 30, 2013, 3:14 p.m.)

Status
This change has been marked as submitted.

Review request for kdelibs.

Description
khtml/kjs: Implement typed-arrays

ArrayBuffer, Int8Array, Uint8Array, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array

I tested many configuration under valgrind, to find all possible errors.
It seems other browsers don't set a limit, so you can eat all memory with these arrays (danger), so khtml/kjs also doesn't set a limit. If you feel like we really need one, please tell.

Diffs
khtml/ecma/kjs_window.cpp 927d251
khtml/ecma/kjs_window.h 1b70be1
khtml/ecma/kjs_arraybufferview.h PRE-CREATION
khtml/ecma/kjs_arraybufferview.cpp PRE-CREATION
khtml/ecma/kjs_arraybuffer.cpp PRE-CREATION
khtml/CMakeLists.txt 99034cc
khtml/ecma/kjs_arraybuffer.h PRE-CREATION

Diff: <a href="http://git.reviewboard.kde.org/r/108929/diff/" title="http://git.reviewboard.kde.org/r/108929/diff/">http://git.reviewboard.kde.org/r/108929/diff/</a>

Testing

Thanks,

Bernd Buschinski

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Commit Hook at 03/30/2013 - 11:12

This review has been submitted with commit e769fee13b4c1b363cdc4674d292748126899162 by Bernd Buschinski to branch master.

- Commit Hook

On Feb. 19, 2013, 12:03 a.m., Bernd Buschinski wrote:

Re: Review Request 108929: khtml/kjs: Implement typed-arrays

By Rolf Eike Beer at 02/16/2013 - 13:37

khtml/ecma/kjs_arraybuffer.cpp
<http://git.reviewboard.kde.org/r/108929/#comment20674>

Those can be size_t, you check that the passed value is > 0 before assignment anyway.

khtml/ecma/kjs_arraybuffer.cpp
<http://git.reviewboard.kde.org/r/108929/#comment20675>

This doesn't need to be ssize_t either, you also check for is being always positive. This allows to get rid of the cast in the second if.

khtml/ecma/kjs_arraybufferview.h
<http://git.reviewboard.kde.org/r/108929/#comment20676>

Can be size_t, for the same reason as above.

khtml/ecma/kjs_arraybufferview.h
<http://git.reviewboard.kde.org/r/108929/#comment20677>

should be static_cast<size_t>, the result is of type size_t.

khtml/ecma/kjs_arraybufferview.h
<http://git.reviewboard.kde.org/r/108929/#comment20678>

Is it guaranteed that this doesn't wraparound? That would be one of the few cases where ssize_t may actually be needed.

khtml/ecma/kjs_arraybufferview.h
<http://git.reviewboard.kde.org/r/108929/#comment20679>

size_t

- Rolf Eike Beer

On Feb. 13, 2013, 8:18 p.m., Bernd Buschinski wrote: