[webkit-reviews] review canceled: [Bug 41769] [Qt] QScriptValue API should have a property flag accessor. : [Attachment 60733] Fix v1

[bugzilla-daemon at webkit.org]

Jędrzej Nowacki <jedrzej.nowacki at nokia.com> has canceled Jędrzej Nowacki
<jedrzej.nowacki at nokia.com>'s request for review:
Bug 41769: [Qt] QScriptValue API should have a property flag accessor.
https://bugs.webkit.org/show_bug.cgi?id=41769

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

------- Additional Comments from Jędrzej Nowacki <jedrzej.nowacki at nokia.com>
(In reply to comment #2)
> Jędrzej, I have a question: since the 'Object' property of the global object
can be modified and propertyFlags() could be called after that, isn't
"dangerous" to get the Object constructor at this point?
It is. There should be a "fixme" apparently it was lost somehow. Thanks for the
review.

> I had a similar situation for isArray(), and ended up extracting the Array
constructor and prototype just after the global context was created in
QScriptEnginePrivate (C++) ctor, storing them in member variables.
Yes I think keeping a reference to the prototype and to the constructor is a
good solution (and maybe only one as JSC doesn't keep original values).

The same issue is in the QSV::isError(), QSV::isRegExp() functions, I think it
will be for an other too. Moreover the QtScript API permits to change global
object and in the same time it use it a lot.

>From architectural point of view I believe that we should create separate
class/struct that represent Original Global Object, It should be created at
QScriptEnginePrivate creation time and it should be private for the QSEP.

I will repost the patch with the fixme comment.