RESOLVED FIXED223413
Add PropertyName parameter to custom setters to allow shared implementations to do late name lookup
https://bugs.webkit.org/show_bug.cgi?id=223413
Summary Add PropertyName parameter to custom setters to allow shared implementations ...
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
Patch (123.17 KB, patch)
2021-03-19 09:17 PDT, Sam Weinig
no flags
Patch (123.17 KB, patch)
2021-03-19 09:23 PDT, Sam Weinig
ews-feeder: commit-queue-
Sam Weinig
Comment 1 2021-03-17 20:27:07 PDT
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
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
Note You need to log in before you can comment on or make changes to this bug.