Bug 204494 - replaceUsingStringSearch() should not use CachedCall with host functions.
Summary: replaceUsingStringSearch() should not use CachedCall with host functions.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-21 23:48 PST by Mark Lam
Modified: 2019-11-22 01:40 PST (History)
10 users (show)

See Also:

Attachments
proposed patch. (3.64 KB, patch)
2019-11-21 23:54 PST, Mark Lam 		ross.kirsling: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2019-11-21 23:48:26 PST 
<rdar://problem/57421078>
Comment 1 Mark Lam 2019-11-21 23:54:02 PST 
Created attachment 384128 [details]
proposed patch.
Comment 2 Ross Kirsling 2019-11-22 00:39:27 PST 
Comment on attachment 384128 [details]
proposed patch.

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

> Source/JavaScriptCore/runtime/StringPrototype.cpp:812
> -            if (cachedCall) {
> +            if (callType == CallType::JS) {

Might be clearer to leave this line -- then the branch reads as "if we have a cached call then call it".
Comment 3 Mark Lam 2019-11-22 01:34:09 PST 
Comment on attachment 384128 [details]
proposed patch.

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

Thanks for the review.

>> Source/JavaScriptCore/runtime/StringPrototype.cpp:812
>> +            if (callType == CallType::JS) {
> 
> Might be clearer to leave this line -- then the branch reads as "if we have a cached call then call it".

Good point.  I'll change it.
Comment 4 Mark Lam 2019-11-22 01:40:09 PST 
Landed in r252766: <http://trac.webkit.org/r252766>.