[webkit-reviews] review requested: [Bug 42174] [Qt] Implement QScriptEngine::newFunction() parts that doesn't depend on QScriptContext : [Attachment 62488] Patch v2

[bugzilla-daemon at webkit.org]

Caio Marcelo de Oliveira Filho <caio.oliveira at openbossa.org> has asked	for
review:
Bug 42174: [Qt] Implement QScriptEngine::newFunction() parts that doesn't
depend on QScriptContext
https://bugs.webkit.org/show_bug.cgi?id=42174

Attachment 62488: Patch v2
https://bugs.webkit.org/attachment.cgi?id=62488&action=review

------- Additional Comments from Caio Marcelo de Oliveira Filho
<caio.oliveira at openbossa.org>
Thanks Jezdrej and Kent for the reviews. This patch fixes the issues pointed
out, add suggested tests and added a new internal function to share code
between the two newFunction() implementations. I'll be more careful with code
style when importing the comments and test cases from Qt's upstream from now on
;-)

Some comments on specific issues:

- QEXPECT_FAIL was added for the commented tests. If I understood the issues
right, the two cases of checking property flags fail due to different reasons:

The property "constructor" of the newly created prototype fails because of bug
40631 (can't change the property flags).
On the other hand, when checking the flags of the property "prototype" of the
function object, it enters the if-statement in
JSCallbackObject::getOwnPropertyDescriptor, which returns hardcoded values. If
I understood correctly, this issue is related to the bug 33946 and the bug
41937.

- JSClassRef can be shared by different contexts, but after discussion with
Jezdrej, I changed to instantiate it for each QScriptEngine, so we can properly
release it. We can implementing the sharing of this later if necessary.