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+

Gavin Barraclough
Reported 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).
Attachments
Fix (16.79 KB, patch)
2012-01-11 19:44 PST, Gavin Barraclough 		no flags
DetailsFormatted DiffDiff
Fix (6.19 KB, patch)
2012-01-12 15:46 PST, Gavin Barraclough 		oliver: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Gavin Barraclough
Comment 1 2012-01-11 19:44:05 PST
Created attachment 122159 [details] Fix
WebKit Review Bot
Comment 2 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.
Gavin Barraclough
Comment 3 2012-01-11 19:53:39 PST
Fixed in r104784
Csaba Osztrogonác
Comment 4 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)
Csaba Osztrogonác
Comment 5 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
Gavin Barraclough
Comment 6 2012-01-12 11:11:56 PST
Sorry I missed this ossy, investigating.
Gavin Barraclough
Comment 7 2012-01-12 15:46:17 PST
Created attachment 122325 [details] Fix
Gavin Barraclough
Comment 8 2012-01-12 15:52:17 PST
Fixed the layout test in r104871
Note You need to log in before you can comment on or make changes to this bug.