Simplify handling of Transferable objects while (de)serializing.

Simplify handling of Transferable objects while (de)serializing.

Avoid unnecessary allocations and abstractions in the handling of
transferables. The Transferables now collates the different kinds of
objects that can be transferred via postMessage().

R=
BUG=haraken

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

[sof, 2016-04-17 20:03:01]

sigbjornf@opera.com changed reviewers:
+ haraken@chromium.org, jsbell@chromium.org

[sof, 2016-04-17 20:03:02]

please take a look.

[haraken, 2016-04-17 23:57:22]

LGTM. This is a great simplification!

https://codereview.chromium.org/1893983002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
(right):

https://codereview.chromium.org/1893983002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp:68: // used
in a context other then Worker's onmessage environment and the presence of

than

https://codereview.chromium.org/1893983002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/Transferables.cpp (right):

https://codereview.chromium.org/1893983002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/Transferables.cpp:13:
Transferables::Transferables()

Nit: Maybe we can inline these constructors/destructors.

[sof, 2016-04-18 06:28:35]

Description was changed from

==========
Simplify handling of Transferable objects while (de)serializing.

Avoid unnecessary allocations and abstractions in the handling of
transferables. The Transferables now collates the different kinds of
objects that can be transferred via postMessage().

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

to

==========
Simplify handling of Transferable objects while (de)serializing.

Avoid unnecessary allocations and abstractions in the handling of
transferables. The Transferables now collates the different kinds of
objects that can be transferred via postMessage().

R=
BUG=haraken
==========

[sof, 2016-04-18 06:41:56]

https://codereview.chromium.org/1893983002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
(right):

https://codereview.chromium.org/1893983002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp:68: // used
in a context other then Worker's onmessage environment and the presence of
On 2016/04/17 23:57:21, haraken wrote:
> 
> than

reworded

https://codereview.chromium.org/1893983002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/Transferables.cpp (right):

https://codereview.chromium.org/1893983002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/Transferables.cpp:13:
Transferables::Transferables()
On 2016/04/17 23:57:22, haraken wrote:
> 
> Nit: Maybe we can inline these constructors/destructors.

Yes, that works out -- MSVC gave me some trouble, which is what prompted
out-of-lining them. But that turns out to be due to the copy constructor that it
is more eager about instantiating/generating.

iow, done - removed Transferables.cpp

https://codereview.chromium.org/1893983002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/MessagePort.h (left):

https://codereview.chromium.org/1893983002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/MessagePort.h:52: // The overwhelmingly
common case is sending a single port, so handle that efficiently with an inline
buffer of size 1.
note: I removed the use of an inline buffer as I don't see transferring ports as
common with postMessage() outside of some initial MessageChannel setup uses, nor
that it should matter for overall performance of a JS-driven postMessage() call.

Should there be some unexpected perf fallout from this, will address - simpler
not to have the special case.

[sof, 2016-04-18 07:12:09]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2016-04-18 07:12:09]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1893983002/#ps20001
(title: "remove Transferable.cpp, not needed after all.")

[commit-bot: I haz the power, 2016-04-18 07:12:33]

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

[commit-bot: I haz the power, 2016-04-18 07:39:36]

Description was changed from

==========
Simplify handling of Transferable objects while (de)serializing.

Avoid unnecessary allocations and abstractions in the handling of
transferables. The Transferables now collates the different kinds of
objects that can be transferred via postMessage().

R=
BUG=haraken
==========

to

==========
Simplify handling of Transferable objects while (de)serializing.

Avoid unnecessary allocations and abstractions in the handling of
transferables. The Transferables now collates the different kinds of
objects that can be transferred via postMessage().

R=
BUG=haraken
==========

[commit-bot: I haz the power, 2016-04-18 07:39:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-18 07:41:20]

Description was changed from

==========
Simplify handling of Transferable objects while (de)serializing.

Avoid unnecessary allocations and abstractions in the handling of
transferables. The Transferables now collates the different kinds of
objects that can be transferred via postMessage().

R=
BUG=haraken
==========

to

==========
Simplify handling of Transferable objects while (de)serializing.

Avoid unnecessary allocations and abstractions in the handling of
transferables. The Transferables now collates the different kinds of
objects that can be transferred via postMessage().

R=
BUG=haraken

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

[commit-bot: I haz the power, 2016-04-18 07:41:22]

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

[xidachen, 2016-04-18 12:13:12]

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

[xidachen, 2016-04-18 12:13:13]

https://codereview.chromium.org/1893983002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/Transferables.h (right):

https://codereview.chromium.org/1893983002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/Transferables.h:22: class CORE_EXPORT
Transferables final {
Question:

With this class under core/ and marked as final, how would it work if I want to
make something under modules/ to be transferable?

[sof, 2016-04-18 18:28:02]

https://codereview.chromium.org/1893983002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/Transferables.h (right):

https://codereview.chromium.org/1893983002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/Transferables.h:22: class CORE_EXPORT
Transferables final {
On 2016/04/18 12:13:13, xidachen wrote:
> Question:
> 
> With this class under core/ and marked as final, how would it work if I want
to
> make something under modules/ to be transferable?

If you want to have such a thing, and it otherwise works out dependency-wise,
you can drop final and derive another STACK_ALLOCATED() transferable class. Or
just define the type in core/.