Bug 223413

Summary: Add PropertyName parameter to custom setters to allow shared implementations to do late name lookup
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, ashvayka, beidson, benjamin, cdumez, cmarcelo, ews-watchlist, jsbell, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=224065
Attachments:
Description Flags
Patch
none
Patch
none
Patch ews-feeder: commit-queue-

Description Sam Weinig 2021-03-17 20:23:00 PDT
Add PropertyName parameter to custom setters to allow shared implementations to do late name lookup
Comment 1 Sam Weinig 2021-03-17 20:27:07 PDT
Created attachment 423556 [details]
Patch
Comment 2 Alexey Shvayka 2021-03-18 11:59:45 PDT
Comment on attachment 423556 [details]
Patch

Marvelous!
Saam confirmed we are OK with this.

Could you please remove this comment (https://github.com/WebKit/WebKit/blob/9c1958b2b1f097707106ad5b4096a68a0d8e88f5/Source/JavaScriptCore/runtime/Lookup.h#L46-L47) on signature? The second argument isn't necessarily an object, and PropertyName is now needed.

With proper DOMJIT support, we could optimize setters similarly to https://bugs.webkit.org/show_bug.cgi?id=171637 in future.
Comment 3 Chris Dumez 2021-03-18 12:01:05 PDT
Comment on attachment 423556 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:8
> +        [WIP]

Says [WIP] and r+. Something is fishy :)
Comment 4 Alexey Shvayka 2021-03-18 13:33:22 PDT
(In reply to Chris Dumez from comment #3)
> Says [WIP] and r+. Something is fishy :)

While there is no ChangeLog, the approach was discussed in length with Sam & JSC folks; the implementation is flawless, so I went ahead with review even though there was no formal r?

I am sure Sam will land with ChangeLog explaining why this change is needed, and why this approach was chosen among other options for CSSStyleDeclaration.
Comment 5 Sam Weinig 2021-03-19 09:17:06 PDT Comment hidden (obsolete)
Comment 6 Sam Weinig 2021-03-19 09:23:22 PDT
Created attachment 423736 [details]
Patch
Comment 7 EWS 2021-03-19 10:24:19 PDT
Committed r274724: <https://commits.webkit.org/r274724>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423736 [details].
Comment 8 Radar WebKit Bug Importer 2021-03-19 10:25:24 PDT
<rdar://problem/75625592>