Chromium Code Reviews
Help | Chromium Project | Sign in

Issue 2826263002: Make DOMArrayBuffer::Transfer neuter v8::ArrayBuffers

Can't Edit
Can't Publish+Mail
Start Review
4 days, 20 hours ago by binji
2 days, 20 hours ago
blink-reviews,,, chromium-reviews, dglazkov+blink, eae+blinkwatch, haraken, hongchan,, rwlbuis, Raymond Toy, sof
Target Ref:


Make DOMArrayBuffer::Transfer neuter v8::ArrayBuffers This behavior is currently done by ScriptValueSerializer, but it's more convenient if the necessary logic is moved to DOMArrayBuffer::Transfer directly; you'll never want to transfer without also calling Neuter on the V8 ArrayBuffer objects. BUG=711824

Patch Set 1 #

Total comments: 5

Patch Set 2 : merge HEAD and feedback #

Total comments: 1

Patch Set 3 : feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -56 lines) Patch
M third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp View 1 2 3 chunks +11 lines, -38 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMArrayBuffer.h View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMArrayBuffer.cpp View 1 2 1 chunk +48 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMArrayBufferBase.h View 1 chunk +0 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMSharedArrayBuffer.h View 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp View 1 2 2 chunks +13 lines, -12 lines 0 comments Download
Commit queue not available (can’t edit this change).


Total messages: 18 (14 generated)
PTAL It seems strange to pass v8::Isolate* to DOMArrayBuffer methods, but I'm not experienced enough ...
3 days, 22 hours ago (2017-04-20 18:44:02 UTC) #7
haraken File third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp (right): third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp:505: exception_state.ThrowDOMException( We're checking IsNeuterable twice. (Transfer calls another IsNeuterable.) ...
3 days, 20 hours ago (2017-04-20 20:07:01 UTC) #8
binji File third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp (right): third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp:505: exception_state.ThrowDOMException( On 2017/04/20 at 20:07:01, haraken wrote: > We're ...
3 days, 17 hours ago (2017-04-21 00:00:23 UTC) #9
3 days, 5 hours ago (2017-04-21 12:03:39 UTC) #14
File third_party/WebKit/Source/core/dom/DOMArrayBuffer.cpp (right):
third_party/WebKit/Source/core/dom/DOMArrayBuffer.cpp:14: static void
On 2017/04/21 00:00:23, binji wrote:
> On 2017/04/20 at 20:07:01, haraken wrote:
> > I forgot the context but would you help me understand why we need to check
> wrappers of all worlds? It seems like we're doing something strange here.
> I have to admit, I don't understand what a "world" is, and why it would have
> own copy of an ArrayBuffer. But if it shares the same backing store as the
> DOMArrayBuffer, it should be neutered on transfer.

Makes sense. Yes, those ArrayBuffer objects are sharing the same backing store.
If you need the semantics that all ArrayBuffers using the backing store must be
neutered, we need that logic.

> I went digging for where this code was originally added, and I was able to
> the basic idea back to the initial implementation of ArrayBuffer transfer:
File third_party/WebKit/Source/core/dom/DOMArrayBuffer.cpp (right):
third_party/WebKit/Source/core/dom/DOMArrayBuffer.cpp:74: }

Can we clean up the code as follows?

- Remove Transfer
- Rename TransferOrCopy to Transfer
- Make IsNeuterable a public method
- Make BaseAudioContext use IsNeuterable & Transfer
- Inline TransferNeuterable and NeuterArrayBuffersInAllWorlds
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46