Bug 76141

Summary: Merge 'Getter'/'Setter' attributes into 'Accessor'
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: JavaScriptCoreAssignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal CC: ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fix
none
Fix oliver: review+

Description Gavin Barraclough 2012-01-11 19:41:44 PST
These are currently ambiguous (and used inconsistently).  It would logically appear that either being bit set implies that the corresponding type of accessor is present but (a) we don't correctly enforce this, and (b) this means the attributes would not be able to distinguish between a data descriptor and an accessor descriptor with neither a getter nor setter defined (which is a descriptor permissible under the spec).  This ambiguity would lead to unsafe property caching behavior (though this does not represent an actual current bug, since we are currently unable to create descriptors that have neither a getter nor setter, it just prevents us from doing so).
Comment 1 Gavin Barraclough 2012-01-11 19:44:05 PST
Created attachment 122159 [details]
Fix
Comment 2 WebKit Review Bot 2012-01-11 19:46:34 PST
Attachment 122159 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/runtime/JSObject.h:69:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Gavin Barraclough 2012-01-11 19:53:39 PST
Fixed in r104784
Comment 4 Csaba Osztrogonác 2012-01-12 03:19:08 PST
It caused an assertion on SL, GTK and Qt:

ASSERTION FAILED: value.isGetterSetter() == !!(attributes & Accessor)
../../../../Source/JavaScriptCore/runtime/PropertyDescriptor.cpp(90) : void JSC::PropertyDescriptor::setDescriptor(JSC::JSValue, unsigned int)
Comment 5 Csaba Osztrogonác 2012-01-12 03:21:36 PST
(In reply to comment #4)
> It caused an assertion on SL, GTK and Qt:
> 
> ASSERTION FAILED: value.isGetterSetter() == !!(attributes & Accessor)
> ../../../../Source/JavaScriptCore/runtime/PropertyDescriptor.cpp(90) : void JSC::PropertyDescriptor::setDescriptor(JSC::JSValue, unsigned int)

I forgot to mention the name of the test: fast/dom/getter-on-window-object2.html
Comment 6 Gavin Barraclough 2012-01-12 11:11:56 PST
Sorry I missed this ossy, investigating.
Comment 7 Gavin Barraclough 2012-01-12 15:46:17 PST
Created attachment 122325 [details]
Fix
Comment 8 Gavin Barraclough 2012-01-12 15:52:17 PST
Fixed the layout test in r104871