Bug 205669 – Reflect.construct() with a the optional newTarget parameter does not work with DOM constructors

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Bindings

[Sam Weinig]

Created attachment 386567 [details]
sub.html

Reflect.construct() with a the optional newTarget parameter does not work with DOM constructors, such as DOMParser.

For example, given a script with two access to two global objects, mainWindow and childWindow:

Whereas this works:
var value1 = Reflect.construct(window.String, [], childWindow.String);
value.constructor == childWindow.String;

this does not:

var value1 = Reflect.construct(window.DOMParser, [], childWindow.DOMParser);
value.constructor == childWindow.DOMParser;

[Sam Weinig]

Created attachment 386568 [details]
main.html

[Sam Weinig]

Created attachment 386569 [details]
main.html

[Sam Weinig]

main.html demonstrates the issue.

[Sam Weinig]

For reference, DOMParser's binding constructor is generated as JSDOMParser.cpp and some relevant lines are:

using JSDOMParserConstructor = JSDOMConstructor<JSDOMParser>;

template<> EncodedJSValue JSC_HOST_CALL JSDOMParserConstructor::construct(JSGlobalObject* lexicalGlobalObject, CallFrame* callFrame)
{
    VM& vm = lexicalGlobalObject->vm();
    auto throwScope = DECLARE_THROW_SCOPE(vm);
    UNUSED_PARAM(throwScope);
    auto* castedThis = jsCast<JSDOMParserConstructor*>(callFrame->jsCallee());
    ASSERT(castedThis);
    auto* context = castedThis->scriptExecutionContext();
    if (UNLIKELY(!context))
        return throwConstructorScriptExecutionContextUnavailableError(*lexicalGlobalObject, throwScope, "DOMParser");
    ASSERT(context->isDocument());
    auto& document = downcast<Document>(*context);
    auto object = DOMParser::create(document);
    return JSValue::encode(toJSNewlyCreated<IDLInterface<DOMParser>>(*lexicalGlobalObject, *castedThis->globalObject(), WTFMove(object)));
}

[Sam Weinig]

My guess, though it has been a while since I did too much in here, is that we should probably be using callFrame->newTarget() in some fashion, probably in place of callFrame->jsCallee() in most places. 

We'll need to do a bit more digging in the WebIDL spec to figure out which document the DOMParser should be created with, though I suspect it is the document of newTarget.

We can test what other browsers do by looking at document.url of a document produced by the constructed DOMParser, as it will match it's owner's url.

[Sam Weinig]

Created attachment 386571 [details]
main.html

Updated main.html

[Sam Weinig]

Looks like Firefox uses the document of the main window. I don't have Chrome installed at the moment to check what it does.

[Sam Weinig]

This causes failures in http://wpt.live/WebIDL/ecmascript-binding/constructors.html

[Alexey Shvayka]

(In reply to Sam Weinig from comment #8)
> This causes failures in
> http://wpt.live/WebIDL/ecmascript-binding/constructors.html

Most of the test cases (except for bound/proxied NewTarget values) are now fixed by https://bugs.webkit.org/show_bug.cgi?id=174313.

[Alexey Shvayka]

(In reply to Alexey Shvayka from comment #9)
> Most of the test cases (except for bound/proxied NewTarget values) are now
> fixed by https://bugs.webkit.org/show_bug.cgi?id=174313.

Please follow https://webkit.org/b/202599: it fixes remaining failures.

*** This bug has been marked as a duplicate of bug 202599 ***