Bug 41769

Summary: [Qt] QScriptValue API should have a property flag accessor.
Product: WebKit Reporter: Jędrzej Nowacki <jedrzej.nowacki>
Component: JavaScriptCoreAssignee: Jędrzej Nowacki <jedrzej.nowacki>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, hausmann, jedrzej.nowacki, kent.hansen
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 31863, 41680    
Attachments:
Description Flags
Fix v1
none
fix v1.01 hausmann: review+

Description Jędrzej Nowacki 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
Comment 1 Jędrzej Nowacki 2010-07-07 07:31:24 PDT
Created attachment 60733 [details]
Fix v1
Comment 2 Caio Marcelo de Oliveira Filho 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?
Comment 3 Jędrzej Nowacki 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.
Comment 4 Jędrzej Nowacki 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).
Comment 5 Jędrzej Nowacki 2010-07-08 01:27:13 PDT
Created attachment 60850 [details]
fix v1.01

Mostly the same patch, I have added the FIXME.
Comment 6 Kent Hansen 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.
Comment 7 Simon Hausmann 2010-07-09 02:21:06 PDT
Committed r62921: <http://trac.webkit.org/changeset/62921>