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-

Sam Weinig
Reported 2021-03-17 20:23:00 PDT
Add PropertyName parameter to custom setters to allow shared implementations to do late name lookup
Attachments
Patch (196.79 KB, patch)
2021-03-17 20:27 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (123.17 KB, patch)
2021-03-19 09:17 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (123.17 KB, patch)
2021-03-19 09:23 PDT, Sam Weinig 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2021-03-17 20:27:07 PDT
Created attachment 423556 [details] Patch
Alexey Shvayka
Comment 2 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.
Chris Dumez
Comment 3 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 :)
Alexey Shvayka
Comment 4 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.
Sam Weinig
Comment 5 2021-03-19 09:17:06 PDT Comment hidden (obsolete)
Sam Weinig
Comment 6 2021-03-19 09:23:22 PDT
Created attachment 423736 [details] Patch
EWS
Comment 7 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].
Radar WebKit Bug Importer
Comment 8 2021-03-19 10:25:24 PDT
<rdar://problem/75625592>
Note You need to log in before you can comment on or make changes to this bug.