Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshGBwAJ

BUG=425896, 640825
R=sigbjornf@opera.com,haraken@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/77a58a92f789f4fd7099b2b4ff87cad27eea3067
Cr-Commit-Position: refs/heads/master@{#417066}

[foolip, 2016-08-30 16:37:17]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-30 16:37:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2016-08-30 16:40:59]

Description was changed from

==========
Drop the legacy

Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshG...

BUG=425896
R=sigbjornf@opera.com,haraken@chromium.org
==========

to

==========
Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshG...

BUG=425896
R=sigbjornf@opera.com,haraken@chromium.org
==========

[foolip, 2016-08-30 16:41:14]

Pardon for the botched subject, this is "Remove window.postMessage(message,
transferables, targetOrigin) legacy overload"

[foolip, 2016-08-30 16:42:24]

On 2016/08/30 16:41:14, foolip wrote:
> Pardon for the botched subject, this is "Remove window.postMessage(message,
> transferables, targetOrigin) legacy overload"

Also, reason I did this was that sshruthi@ looked over pending removals and it
looked like sof was on vacation and/or busy.

[jsbell, 2016-08-30 17:01:36]

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

[jsbell, 2016-08-30 17:01:36]

drive by comment...

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

https://codereview.chromium.org/2295863002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:190:
TOSTRING_VOID(V8StringResource<TreatNullAndUndefinedAsNullString>, targetOrigin,
info[targetOriginArgIndex]);
We process the arguments out of order: 3rd, 2nd, 1st. This is observable from
script...

The processing of transferables first is probably covered by
"StructuredCloneWithTransfer(message, transfer, targetRealm)" in spec land, not
going to dig into that since we do that in every postMessage() impl.

But maybe the string conversion should be moved up. Also, if it fails (e.g.
passing {toString: () => { throw Error(); }}) do we have an early exit?

(These are out of scope for this CL, sorry. Maybe some TODOs?)

[commit-bot: I haz the power, 2016-08-30 17:43:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 17:43:07]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[foolip, 2016-09-01 10:21:40]

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

https://codereview.chromium.org/2295863002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:190:
TOSTRING_VOID(V8StringResource<TreatNullAndUndefinedAsNullString>, targetOrigin,
info[targetOriginArgIndex]);
On 2016/08/30 17:01:36, jsbell wrote:
> We process the arguments out of order: 3rd, 2nd, 1st. This is observable from
> script...
> 
> The processing of transferables first is probably covered by
> "StructuredCloneWithTransfer(message, transfer, targetRealm)" in spec land,
not
> going to dig into that since we do that in every postMessage() impl.

Looks like transferables are processed first because they're then used to
serialize the message, in ScriptValueSerializer::serialize.

> But maybe the string conversion should be moved up. Also, if it fails (e.g.
> passing {toString: () => { throw Error(); }}) do we have an early exit?

The TOSTRING_VOID macro does the return. But it
TreatNullAndUndefinedAsNullString still wrong, as is the order.

> (These are out of scope for this CL, sorry. Maybe some TODOs?)

Added TODOs.

[foolip, 2016-09-01 10:24:11]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[foolip, 2016-09-01 10:24:19]

Description was changed from

==========
Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshG...

BUG=425896
R=sigbjornf@opera.com,haraken@chromium.org
==========

to

==========
Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshG...

BUG=425896,640825
R=sigbjornf@opera.com,haraken@chromium.org
==========

[commit-bot: I haz the power, 2016-09-01 10:24:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 11:30:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-01 11:30:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[foolip, 2016-09-01 11:46:33]

Description was changed from

==========
Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshG...

BUG=425896,640825
R=sigbjornf@opera.com,haraken@chromium.org
==========

to

==========
Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshG...

BUG=425896,640825
R=sigbjornf@opera.com,haraken@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[foolip, 2016-09-01 11:47:20]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-01 11:47:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 14:09:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-01 14:09:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[foolip, 2016-09-01 14:22:08]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-01 14:22:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 15:59:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-01 15:59:20]

Dry run: This issue passed the CQ dry run.

[foolip, 2016-09-01 19:44:33]

Ping reviewers, I think it's all in order now.

[haraken, 2016-09-01 21:12:27]

LGTM on my side

[jsbell, 2016-09-01 21:36:05]

non-owner LGTM since I'm here anyway :)

[foolip, 2016-09-02 09:46:27]

foolip@chromium.org changed reviewers:
+ aboxhall@chromium.org, dmazzoni@chromium.org

[foolip, 2016-09-02 09:46:28]

aboxhall@, dmazzoni@, could either of you check the chromevox changes?
https://code.google.com/archive/p/google-axs-chrome/ is now read only, so I'm
not sure how I should have handled the third-party change. Thanks!

[dmazzoni, 2016-09-07 16:39:45]

lgtm for chromevox

Don't worry about the upstream version, it's deprecated

[foolip, 2016-09-07 19:20:09]

The CQ bit was checked by foolip@chromium.org

[commit-bot: I haz the power, 2016-09-07 19:20:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 21:57:10]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-07 22:00:09]

Description was changed from

==========
Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshG...

BUG=425896,640825
R=sigbjornf@opera.com,haraken@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Remove window.postMessage(message, transferables, targetOrigin) legacy overload

Intent to Deprecate and Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/h4ooaB_Y9JE/nh7vXshG...

BUG=425896,640825
R=sigbjornf@opera.com,haraken@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/77a58a92f789f4fd7099b2b4ff87cad27eea3067
Cr-Commit-Position: refs/heads/master@{#417066}
==========

[commit-bot: I haz the power, 2016-09-07 22:00:11]

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