Bug 229061

Summary: Web Inspector: refactor `WI.ImageUtilities.scratchCanvasContext2D` to not use a callback
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: NEW    
Severity: Normal CC: ews-watchlist, hi, inspector-bugzilla-changes, pangle, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Devin Rousso
Reported 2021-08-12 17:22:41 PDT
Having a callback makes it unclear as to whether the function is async (it's not). All of the callers expect it to be sync.
Attachments
Patch (7.03 KB, patch)
2021-08-12 17:29 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Devin Rousso
Comment 1 2021-08-12 17:29:37 PDT
Created attachment 435458 [details] Patch
Patrick Angle
Comment 2 2021-08-13 09:45:59 PDT
Comment on attachment 435458 [details] Patch I thought about this some more, and I'm concerned it trades one confusing construct for another, less common, confusing construct. The call sites at first glance now read as loops, which to me invokes the question "Why does this work need to be done more than once", a question that much like the sync/async situation can only really be answered by a trip to the definition of `WI.ImageUtilities.scratchCanvasContext2D`. I enjoy this solution from a technical perspective, but I'm not sure it meets the stated goal in the changelog of making these call sites clearer. It's just unclear in a different way.
Devin Rousso
Comment 3 2021-08-13 11:14:19 PDT
Yeah FWIW this is kinda a "hack" of sorts 😅 I personally found that the loop looked better than a callback, but that's just me.
Radar WebKit Bug Importer
Comment 4 2021-08-19 17:23:24 PDT
<rdar://problem/82148919>
Note You need to log in before you can comment on or make changes to this bug.