RESOLVED FIXED 41769
[Qt] QScriptValue API should have a property flag accessor.
https://bugs.webkit.org/show_bug.cgi?id=41769
Summary [Qt] QScriptValue API should have a property flag accessor.
Jędrzej Nowacki
Reported 2010-07-07 07:17:42 PDT
QtScript should provide an API for checking a JS object's properties flags see http://doc.trolltech.com/4.6/qscriptvalue.html#propertyFlags
Attachments
Fix v1 (10.99 KB, patch)
2010-07-07 07:31 PDT, Jędrzej Nowacki
no flags
fix v1.01 (11.06 KB, patch)
2010-07-08 01:27 PDT, Jędrzej Nowacki
hausmann: review+
Jędrzej Nowacki
Comment 1 2010-07-07 07:31:24 PDT
Caio Marcelo de Oliveira Filho
Comment 2 2010-07-07 15:52:17 PDT
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?
Jędrzej Nowacki
Comment 3 2010-07-08 00:49:31 PDT
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.
Jędrzej Nowacki
Comment 4 2010-07-08 01:20:29 PDT
(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).
Jędrzej Nowacki
Comment 5 2010-07-08 01:27:13 PDT
Created attachment 60850 [details] fix v1.01 Mostly the same patch, I have added the FIXME.
Kent Hansen
Comment 6 2010-07-09 00:57:18 PDT
(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.
Simon Hausmann
Comment 7 2010-07-09 02:21:06 PDT
Note You need to log in before you can comment on or make changes to this bug.