[webkit-reviews] review requested: [Bug 36594] [Qt] QtScript is missing toObject API : [Attachment 53860] Fix v2

[bugzilla-daemon at webkit.org]

Jędrzej Nowacki <jedrzej.nowacki at nokia.com> has asked  for review:
Bug 36594: [Qt] QtScript is missing toObject API
https://bugs.webkit.org/show_bug.cgi?id=36594

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

------- Additional Comments from Jędrzej Nowacki <jedrzej.nowacki at nokia.com>
> Something to be aware of: In Qt 4.5 and 4.6 we allow using
> QScriptEngine::toObject() on a value constructed in another engine. It's not
> tested in the autotests, but it works (granted, more by "luck" than by
design).
> 
> Ideally I'd like to have the check, but since it's a difference in behavior,
we
> should consider it. Should 4.6/4.7 be changed to do the check as well?
(Because
> nobody would ever rely on this behavior, right...? ;) ) Declare the
> lack-of-check a bug, that is? (I think it is, it's just that it's been that
way
> forever...)
Who can rely on this? Of course nobody :-) I think that if it works we should
keep it, but not support it. One of the main rules of QtScript is that
developers can't mix engines, values created in one engine couldn't be
transfered to an another and so on...  It make sens for me to add the qWarning
to current Qt without forcing the behavior.

> If the check remains, the qWarning message at least needs to be qualified,
e.g.
> "QScriptEngine::toObject: cannot convert value created in a different
engine";
> this also makes the overall wording consistent with similar warnings in other

> API.
Changed.

> It would be good if you could add QVERIFYs to the tests to ensure that the
type
> of the original value has not changed after the toObject() call (I've already

> added such checks to the tests in Qt).
Done.