Bug 76141 - Merge 'Getter'/'Setter' attributes into 'Accessor'
Summary: Merge 'Getter'/'Setter' attributes into 'Accessor'
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-11 19:41 PST by Gavin Barraclough
Modified: 2012-01-12 15:52 PST (History)
2 users (show)

See Also:


Attachments
Fix (16.79 KB, patch)
2012-01-11 19:44 PST, Gavin Barraclough
no flags Details | Formatted Diff | Diff
Fix (6.19 KB, patch)
2012-01-12 15:46 PST, Gavin Barraclough
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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