Bug 204542 - REGRESSION (r252792?): 6 inspector/canvas tests crashing
Summary: REGRESSION (r252792?): 6 inspector/canvas tests crashing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-22 17:52 PST by Aakash Jain
Modified: 2019-11-22 19:30 PST (History)
10 users (show)

See Also:


Attachments
Fixes the bug (1.60 KB, patch)
2019-11-22 18:24 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Fixes the bug (2.95 KB, patch)
2019-11-22 18:34 PST, Ryosuke Niwa
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2019-11-22 17:52:08 PST
Following tests started crashing recently:

inspector/canvas/recording-2d-frameCount.html
inspector/canvas/recording-2d-saves.html
inspector/canvas/recording-bitmaprenderer-frameCount.html
inspector/canvas/recording-webgl-frameCount.html
inspector/canvas/recording-webgl-snapshots.html
inspector/canvas/setRecordingAutoCaptureFrameCount.html


e.g.: https://ews-build.webkit.org/#/builders/17/builds/7186
Comment 2 Aakash Jain 2019-11-22 17:53:23 PST
Seems regression from either r252790 or r252792.
Comment 4 Jonathan Bedard 2019-11-22 17:55:15 PST
I'm pretty much positive it's r252792.

Look at what files changed: https://results.webkit.org/commits?before_id=252792&after_id=252789
Comment 5 Radar WebKit Bug Importer 2019-11-22 17:56:20 PST
<rdar://problem/57446993>
Comment 6 Jonathan Bedard 2019-11-22 17:57:19 PST
(In reply to Jonathan Bedard from comment #4)
> I'm pretty much positive it's r252792.
> 
> Look at what files changed:
> https://results.webkit.org/commits?before_id=252792&after_id=252789

Although I suppose that r252792 could have broken the canvas stuff and then r252790 independently broke the inspector stuff.
Comment 7 Ryosuke Niwa 2019-11-22 18:14:54 PST
Oops, I just landed a patch which would make reverting this harder. Let me see if I can figure something out...
Comment 8 Ryosuke Niwa 2019-11-22 18:24:11 PST
Created attachment 384220 [details]
Fixes the bug
Comment 9 Ryosuke Niwa 2019-11-22 18:34:33 PST
Created attachment 384222 [details]
Fixes the bug
Comment 10 Devin Rousso 2019-11-22 18:52:43 PST
Comment on attachment 384220 [details]
Fixes the bug

r=me, this actually should be enough as we shouldn’t be adding anything to `m_recordingCanvasIdentifiers` in this loop. If anything, it should already be empty at the end of this loop since each item gets removed inside `InspectorCanvasAgent::didFinishRecordingCanvasFrame`.

Thanks for the quick fix!
Comment 11 Ryosuke Niwa 2019-11-22 19:30:13 PST
Comment on attachment 384220 [details]
Fixes the bug

Clearing flags on attachment: 384220

Committed r252823: <https://trac.webkit.org/changeset/252823>
Comment 12 Ryosuke Niwa 2019-11-22 19:30:15 PST
All reviewed patches have been landed.  Closing bug.