Bug 71336 – Implement __lookupGetter__/__lookupSetter__ in terms of getPropertyDescriptor

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Implement __lookupGetter__/__lookupSetter__ in terms of getPropertyDescriptor

[Sam Weinig]

Created attachment 113250 [details]
Patch

[Darin Adler]

Comment on attachment 113250 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113250&action=review

What’s the performance impact?

> LayoutTests/ChangeLog:10
> +        Update results now that __lookupGetter__/__lookupSetter__ match getOwnPropertyDescriptor.
> +        The failures are tracked by https://bugs.webkit.org/show_bug.cgi?id=71333.

Doesn’t seem great to create new failures.

[Sam Weinig]

(In reply to comment #2)
> (From update of attachment 113250 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113250&action=review
> 
> What’s the performance impact?
>
> 
> > LayoutTests/ChangeLog:10
> > +        Update results now that __lookupGetter__/__lookupSetter__ match getOwnPropertyDescriptor.
> > +        The failures are tracked by https://bugs.webkit.org/show_bug.cgi?id=71333.
> 
> Doesn’t seem great to create new failures.

I discussed both of these issues with Gavin, and we believe that this implementation will be a bit slower than the last one (though not much, or algorithmaclly) but it is a very infrequently used function and speeding up getPropertyDescriptor would be the preferable thing to spend out time speeding up.  

The failure is also not cool, but I plan to fix it soon, I just wanted to de-virtualize these two functions to make Mark Hahnenberg's life a bit easier.

[Sam Weinig]

Committed r99034: <http://trac.webkit.org/changeset/99034>

[Ryosuke Niwa]

It appears that this patch caused platform/mac/accessibility/select-element-selection-with-optgroups.html to hit an assertion on SL:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=accessibility%2Fdeleting-iframe-destroys-axcache.html%2Cplatform%2Fmac%2Faccessibility%2Fselect-element-selection-with-optgroups.html

blame list:
http://trac.webkit.org/log/?verbose=on&rev=99037&stop_rev=99025

stack trace:
http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r99135%20(2818)/platform/mac/accessibility/select-element-selection-with-optgroups-crash-log.txt

[Sam Weinig]

(In reply to comment #5)
> It appears that this patch caused platform/mac/accessibility/select-element-selection-with-optgroups.html to hit an assertion on SL:
> 
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=accessibility%2Fdeleting-iframe-destroys-axcache.html%2Cplatform%2Fmac%2Faccessibility%2Fselect-element-selection-with-optgroups.html
> 
> blame list:
> http://trac.webkit.org/log/?verbose=on&rev=99037&stop_rev=99025
> 
> stack trace:
> http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r99135%20(2818)/platform/mac/accessibility/select-element-selection-with-optgroups-crash-log.txt

This seems pretty unlikely.