Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=haraken,tkent,ortuno
BUG=585328
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/bf1c0ddd52de2e9b656b8e73bdb7634eb3343fe3
Cr-Commit-Position: refs/heads/master@{#386347}

[sof, 2016-04-10 07:50:39]

Description was changed from

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable derived types on the Oilpan heap.

R=
BUG=
==========

to

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable derived types on the Oilpan heap.

R=
BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[sof, 2016-04-10 16:38:00]

Description was changed from

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable derived types on the Oilpan heap.

R=
BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=
BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[sof, 2016-04-10 16:38:24]

Description was changed from

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=
BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=
BUG=585328
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[sof, 2016-04-10 17:32:27]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org, tkent@chromium.org

[sof, 2016-04-10 17:32:28]

please take a look (tkent@ for the public/ changes.)

I realized that it isn't very satisfactory to ship these ref-counted objects
with Oilpan's version of ScriptWrappable.

All IDL interface types will be on the Oilpan heap following this, so removing
[GarbageCollected] and a few other simplifications in generated code ought to be
doable afterwards.

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
(right):

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp:194:
DOMArrayBuffer* arrayBuffer = m_loader.arrayBufferResult();
As FileReaderLoader re-creates a DOMArrayBuffer each time arrayBufferResult() is
called, it isn't very nice to call it repeatedly. Moreover, as
arrayBufferResult() is also called on the decoder thread, it is somewhat untidy
to adjust the underlying ref count of ArrayBufferBuilder's ArrayBuffer from
another thread.  Hence, arrange for the DOMArrayBuffer to be created on the main
thread and passed in to the operation on the decoder thread.

[ortuno, 2016-04-10 17:53:36]

ortuno@chromium.org changed reviewers:
+ ortuno@chromium.org

[ortuno, 2016-04-10 17:53:38]

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp
(right):

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp:118:
m_resolver->resolve(domDataView);
Just to make sure I understand this: Calling m_webCharacteristic->setValue()
with the pointer to the data view will make Oilpan increase the ref count for
the Data View. When we resolve that the promise with the pointer to the data
view, Oilpan will once again increase the ref count. If the characteristic
object gets GCed then Oilpan will decrease the ref count but it will keep the
object alive because of the reference we used when resolving the promise. Is all
of this correct?

I' just want to be sure that we won't run into use after delete if the
characteristic object is GCed but someone is still holding on to the pointer
used to resolve the promise.

[sof, 2016-04-10 18:12:51]

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp
(right):

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp:118:
m_resolver->resolve(domDataView);
On 2016/04/10 17:53:37, ortuno wrote:
> Just to make sure I understand this: Calling m_webCharacteristic->setValue()
> with the pointer to the data view will make Oilpan increase the ref count for
> the Data View. When we resolve that the promise with the pointer to the data
> view, Oilpan will once again increase the ref count. If the characteristic
> object gets GCed then Oilpan will decrease the ref count but it will keep the
> object alive because of the reference we used when resolving the promise. Is
all
> of this correct?
> 
> I' just want to be sure that we won't run into use after delete if the
> characteristic object is GCed but someone is still holding on to the pointer
> used to resolve the promise.

DOMDataView is on the Oilpan heap, so there's no ref counting going on for it.
That DOMDataView will only be eligible for GCing once it is no longer reachable,
i.e., when both this characteristic object and the promise have let go (or
themselves have become unreachable from the running renderer process.)

[ortuno, 2016-04-10 18:15:52]

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp
(right):

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp:118:
m_resolver->resolve(domDataView);
On 2016/04/10 at 18:12:51, sof wrote:
> On 2016/04/10 17:53:37, ortuno wrote:
> > Just to make sure I understand this: Calling m_webCharacteristic->setValue()
> > with the pointer to the data view will make Oilpan increase the ref count
for
> > the Data View. When we resolve that the promise with the pointer to the data
> > view, Oilpan will once again increase the ref count. If the characteristic
> > object gets GCed then Oilpan will decrease the ref count but it will keep
the
> > object alive because of the reference we used when resolving the promise. Is
all
> > of this correct?
> > 
> > I' just want to be sure that we won't run into use after delete if the
> > characteristic object is GCed but someone is still holding on to the pointer
> > used to resolve the promise.
> 
> DOMDataView is on the Oilpan heap, so there's no ref counting going on for it.
That DOMDataView will only be eligible for GCing once it is no longer reachable,
i.e., when both this characteristic object and the promise have let go (or
themselves have become unreachable from the running renderer process.)

Got it. Thanks! Bluetooth LGTM

[haraken, 2016-04-11 04:34:32]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-04-11 04:34:33]

LGTM!

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
(right):

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp:64:
m_keepAlive = this;

Isn't the |this| object kept alive by the persistent handle implicitly created
by threadSafeBind (so we don't need the m_keepAlive)?

Either way, you don't need to address it in this CL.

[tkent, 2016-04-11 04:47:45]

lgtm

[sof, 2016-04-11 05:14:22]

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
(right):

https://codereview.chromium.org/1878463002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp:64:
m_keepAlive = this;
On 2016/04/11 04:34:33, haraken wrote:
> 
> Isn't the |this| object kept alive by the persistent handle implicitly created
> by threadSafeBind (so we don't need the m_keepAlive)?
> 
> Either way, you don't need to address it in this CL.

I believe that's correct, but
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
gave me reasons to wonder if there's more to it. Will followup.

[sof, 2016-04-11 05:34:57]

Description was changed from

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=
BUG=585328
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=haraken,tkent,ortuno
BUG=585328
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[sof, 2016-04-11 05:35:31]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2016-04-11 05:35:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1878463002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1878463002/60001

[commit-bot: I haz the power, 2016-04-11 08:15:01]

Description was changed from

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=haraken,tkent,ortuno
BUG=585328
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=haraken,tkent,ortuno
BUG=585328
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-04-11 08:15:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-11 08:16:08]

Description was changed from

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=haraken,tkent,ortuno
BUG=585328
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Move DOMArrayBuffer, DOMArrayBufferViews and DataView to the heap.

Thereby having all ScriptWrappable-derived types on the Oilpan heap.

R=haraken,tkent,ortuno
BUG=585328
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/bf1c0ddd52de2e9b656b8e73bdb7634eb3343fe3
Cr-Commit-Position: refs/heads/master@{#386347}
==========

[commit-bot: I haz the power, 2016-04-11 08:16:10]

Patchset 4 (id:??) landed as
https://crrev.com/bf1c0ddd52de2e9b656b8e73bdb7634eb3343fe3
Cr-Commit-Position: refs/heads/master@{#386347}

[fmeawad, 2016-05-11 16:52:46]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1964183004/ by fmeawad@chromium.org.

The reason for reverting is: This CL causes a clankium crash on the perf
waterfall. Attempting a revert.

BUG=610015.