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

third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp

 /*
  * Copyright (C) 2009, 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 132 matching lines @@
     // Delete the accessor from the inner object.
     info.Holder()->Delete(isolate->GetCurrentContext(), v8AtomicString(isolate, "opener"));
 
     // Put property on the inner object.
     if (info.Holder()->IsObject()) {
         v8::Maybe<bool> unused = v8::Local<v8::Object>::Cast(info.Holder())->Set(isolate->GetCurrentContext(), v8AtomicString(isolate, "opener"), value);
         ALLOW_UNUSED_LOCAL(unused);
     }
 }
 
-static bool isLegacyTargetOriginDesignation(v8::Local<v8::Value> value)
-{
-    if (value->IsString() || value->IsStringObject())
-        return true;
-    return false;
-}
-
-
 void V8Window::postMessageMethodCustom(const v8::FunctionCallbackInfo<v8::Value>& info)
 {
     ExceptionState exceptionState(ExceptionState::ExecutionContext, "postMessage", "Window", info.Holder(), info.GetIsolate());
     if (UNLIKELY(info.Length() < 2)) {
         setMinimumArityTypeError(exceptionState, 2, info.Length());
         return;
     }
 
     // None of these need to be RefPtr because info and context are guaranteed
     // to hold on to them.
     DOMWindow* window = V8Window::toImpl(info.Holder());
     // TODO(yukishiino): The HTML spec specifies that we should use the
     // Incumbent Realm instead of the Current Realm, but currently we don't have
     // a way to retrieve the Incumbent Realm.  See also:
     // https://html.spec.whatwg.org/multipage/comms.html#dom-window-postmessage
     LocalDOMWindow* source = currentDOMWindow(info.GetIsolate());
 
     ASSERT(window);
     UseCounter::countIfNotPrivateScript(info.GetIsolate(), window->frame(), UseCounter::WindowPostMessage);
 
     // If called directly by WebCore we don't have a calling context.
     if (!source) {
         exceptionState.throwTypeError("No active calling context exists.");
         return;
     }
 
     // This function has variable arguments and can be:
-    // Per current spec:
     //   postMessage(message, targetOrigin)
     //   postMessage(message, targetOrigin, {sequence of transferrables})
-    // Legacy non-standard implementations in webkit allowed:
-    //   postMessage(message, {sequence of transferrables}, targetOrigin);
     Transferables transferables;
-    int targetOriginArgIndex = 1;
+    const int targetOriginArgIndex = 1;
     if (info.Length() > 2) {
-        int transferablesArgIndex = 2;
-        if (isLegacyTargetOriginDesignation(info[2])) {
-            Deprecation::countDeprecationIfNotPrivateScript(info.GetIsolate(), window->document(), UseCounter::WindowPostMessageWithLegacyTargetOriginArgument);
-            targetOriginArgIndex = 2;
-            transferablesArgIndex = 1;
-        }
+        const int transferablesArgIndex = 2;
         if (!SerializedScriptValue::extractTransferables(info.GetIsolate(), info[transferablesArgIndex], transferablesArgIndex, transferables, exceptionState)) {
             return;
         }
     }
     TOSTRING_VOID(V8StringResource<TreatNullAndUndefinedAsNullString>, targetOrigin, info[targetOriginArgIndex]);

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

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?)

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

Looks like transferables are processed first because they're then used to
serialize the message, in ScriptValueSerializer::serialize.
The TOSTRING_VOID macro does the return. But it
TreatNullAndUndefinedAsNullString still wrong, as is the order.
Added TODOs.

 
     RefPtr<SerializedScriptValue> message = SerializedScriptValue::serialize(info.GetIsolate(), info[0], &transferables, nullptr, exceptionState);
     if (exceptionState.hadException())
         return;
 
     window->postMessage(message.release(), transferables.messagePorts, targetOrigin, source, exceptionState);
 }
 
 void V8Window::openMethodCustom(const v8::FunctionCallbackInfo<v8::Value>& info)
 {
@@ skipping 77 matching lines @@
         if (items->hasExactlyOneItem()) {
             v8SetReturnValueFast(info, items->item(0), window);
             return;
         }
         v8SetReturnValueFast(info, items, window);
         return;
     }
 }
 
 } // namespace blink