Bug 210862

Summary:

Remove revoked Proxy checks from ProxyCreate

Product:

WebKit

Reporter:

Alexey Shvayka <ashvayka>

Component:

JavaScriptCore

Assignee:

Alexey Shvayka <ashvayka>

Status:

RESOLVED FIXED

Severity:

Minor

CC:

ews-watchlist, keith_miller, mark.lam, msaboff, ross.kirsling, saam, tzagallo, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

See Also:

https://bugs.webkit.org/show_bug.cgi?id=198755

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Alexey Shvayka

Reported 2020-04-22 09:08:04 PDT

Remove revoked Proxy checks from ProxyCreate

Attachments
Patch (5.83 KB, patch) 2020-04-22 09:26 PDT, Alexey Shvayka	no flags	Details Formatted Diff Diff
Patch (12.29 KB, patch) 2020-04-22 10:36 PDT, Alexey Shvayka	no flags	Details Formatted Diff Diff
Patch (13.37 KB, patch) 2020-04-23 21:55 PDT, Alexey Shvayka	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Alexey Shvayka

Comment 1 2020-04-22 09:26:35 PDT

Created attachment 397205 [details] Patch

Alexey Shvayka

Comment 2 2020-04-22 10:36:46 PDT

Created attachment 397222 [details] Patch Adjust tests.

Ross Kirsling

Comment 3 2020-04-22 12:50:10 PDT

Comment on attachment 397222 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397222&action=review > JSTests/ChakraCore.yaml:1849 > - cmd: runChakra :skip, "NoException", "arraywithproxy.baseline", [] > + cmd: runChakra :skipDueToOutdatedOrBadTest, "NoException", "", [] Whoooa, I didn't know this setting existed. Pretty sure I could have used it in the past! :D > Source/JavaScriptCore/ChangeLog:11 > + Also cleans up ProxyObject creation by using isFunction() instead of > + isCallable(), which are identical. Hmm, I wonder if this should be cleaned up for redundancy (in another patch of course)... It's odd that isCallable still checks getCallData for JSFunctionType, when it doesn't look like this could ever be CallType::None.

Alexey Shvayka

Comment 4 2020-04-23 11:29:18 PDT

Comment on attachment 397222 [details] Patch (In reply to Ross Kirsling from comment #3) Thank you for review! > Whoooa, I didn't know this setting existed. Pretty sure I could have used it > in the past! :D I've discovered it by accident: it was used for /test/es6/proxyenumbug.js, which is totally outdated. JSC made some significant progress on spec compliance since ChakraCore tests were last updated, and I believe tests are up to date with ECMA-262 and don't have print('key' + Symbol()) anymore. It would be great to re-sync them. > It's odd that isCallable still checks getCallData for JSFunctionType, when > it doesn't look like this could ever be CallType::None. This would be a great improvement! With this patch, isCallable() has 1-2 call sites as isFunction() seems to be preferred. Also there are a few usages of getCallData() which ignore the CallData. Do you mind if I land this after test262 sync?

Alexey Shvayka

Comment 5 2020-04-23 21:55:45 PDT

Created attachment 397425 [details] Patch Set reviewer and rebase patch.

EWS

Comment 6 2020-04-23 22:43:37 PDT

Committed r260621: <https://trac.webkit.org/changeset/260621> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397425 [details].

Radar WebKit Bug Importer

Comment 7 2020-04-23 22:44:23 PDT

<rdar://problem/62282842>

Note You need to log in before you can comment on or make changes to this bug.