Bug 41769

Created attachment 60733 [details] Fix v1

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? 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. Maybe the same could be done to the Object constructor, or am I missing a difference between those two cases?

Comment on attachment 60733 [details] Fix v1 (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.

(In reply to comment #3) > 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've created a bug(41839).

Created attachment 60850 [details] fix v1.01 Mostly the same patch, I have added the FIXME.

(In reply to comment #5) > Created an attachment (id=60850) [details] > fix v1.01 > > Mostly the same patch, I have added the FIXME. The way to make it "safe" would be to store a reference to the original getOwnPropertyDescriptor() function at engine construction time (storing the Object constructor is not enough). The current approach is suboptimal anyway; as the patch says, there should be a C API for getting attributes. I've created https://bugs.webkit.org/show_bug.cgi?id=41937 for this. I think the patch is fine for now. Let's do it "properly" when the C API arrives.

Committed r62921: <http://trac.webkit.org/changeset/62921>