Bug 238955

Summary: Broaden TypedArray API fix to all apps not just Bleacher Report
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, darin, ews-watchlist, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch saam: review+

Description Keith Miller 2022-04-07 13:12:03 PDT
Broaden TypedArray API fix to all apps not just Bleacher Report
Comment 1 Keith Miller 2022-04-07 13:14:02 PDT
Created attachment 456960 [details]
Patch
Comment 2 Saam Barati 2022-04-07 13:17:59 PDT
Comment on attachment 456960 [details]
Patch

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

r=me

> Source/JavaScriptCore/API/JSTypedArray.cpp:369
> +inline static bool isLinkedBeforeQuirk()

let's call this "isLinkedBeforeTypedArrayLengthQuirk"?
Comment 3 Keith Miller 2022-04-08 09:41:16 PDT
Committed r292609 (?): <https://commits.webkit.org/r292609>
Comment 4 Radar WebKit Bug Importer 2022-04-08 09:42:17 PDT
<rdar://problem/91487064>
Comment 5 Darin Adler 2022-04-08 10:07:14 PDT
Comment on attachment 456960 [details]
Patch

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

>> Source/JavaScriptCore/API/JSTypedArray.cpp:369
>> +inline static bool isLinkedBeforeQuirk()
> 
> let's call this "isLinkedBeforeTypedArrayLengthQuirk"?

Typically in the past I have named such functions needsTypedArrayAPIQuirk() rather than specifying how the functions do their jobs. I think it’s easier to read at the call site.
Comment 6 Saam Barati 2022-04-08 16:06:54 PDT
(In reply to Darin Adler from comment #5)
> Comment on attachment 456960 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=456960&action=review
> 
> >> Source/JavaScriptCore/API/JSTypedArray.cpp:369
> >> +inline static bool isLinkedBeforeQuirk()
> > 
> > let's call this "isLinkedBeforeTypedArrayLengthQuirk"?
> 
> Typically in the past I have named such functions needsTypedArrayAPIQuirk()
> rather than specifying how the functions do their jobs. I think it’s easier
> to read at the call site.

I agree this is a better name and aids readability.