Bug 196875

Summary: REGRESSION (r244182): inspector/canvas/recording-webgl-snapshots.html became flaky on WK1
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: AnimationsAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, dino, ews-watchlist, hi, sroberts, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=177484
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews117 for mac-highsierra none

Said Abou-Hallawa
Reported 2019-04-12 12:48:27 PDT
This a followup bug for https://trac.webkit.org/changeset/244182 which caused this test to be flaky on WK1.
Attachments
Patch (3.23 KB, patch)
2019-05-27 13:49 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Patch (4.00 KB, patch)
2019-05-28 10:03 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews117 for mac-highsierra (3.03 MB, application/zip)
2019-05-28 11:30 PDT, EWS Watchlist 		no flags
Details
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-04-12 22:38:19 PDT
<rdar://problem/49873252>
Shawn Roberts
Comment 2 2019-04-15 10:12:51 PDT
Marked flaky in https://trac.webkit.org/changeset/244263/webkit while waiting for a fix.
Devin Rousso
Comment 3 2019-05-27 13:45:01 PDT
So here's how all of the inspector/canvas/recording-<type>.html tests work: 1. create a canvas and get the context for <type> 2. start recording the context - note that the started recording can have options, such as a `frameLimit` (controls how many "frames" (e.g. when the <canvas> paints) to record before _automatically_ stopping the recording from the backend) 3. perform some actions - depending on the test, the actions can span multiple `setTimeout` or `requestAnimationFrame` - based on `frameLimit`, some or all of those actions will be recorded 4. wait for either the inspector backend to _automatically_ stop the recording (from `frameLimit`) OR the test page itself to say "all done" (this is the `LastFrame` event) 5a. if `LastFrame` is dispatched _before_ the recording has ended (e.g. no `frameLimit`), the inspector frontend itself will then stop the recording 5b. if `LastFrame` is dispatched _after_ the recording has ended (e.g. a specified `frameLimit`), the inspector frontend won't attempt to "re-stop" the recording, as that would throw an error ("No active recording for canvas") 6. process the recording data and compare with the expected results The issue with this test looks like it's in step 5a. Due to the fact that all messages between the inspector frontend and inspector backend are async, it's possible for the inspector backend to automatically stop the recording and dispatch a message to the inspector frontend, only for it to be received _after_ `LastFrame`, meaning that the inspector frontend doesn't yet know about the fact that the recording had already been stopped. A solution to this is to just not set a `frameCount` for "shorter" recording tests like this, as the timing is too "close" when running in debug. This isn't an issue when using Web Inspector "normally" (e.g. not in a test), because the inspector frontend UI will "ignore" these types of errors, not to mention it would be MUCH harder for a person to get the timing just right to even encounter this situation.
Devin Rousso
Comment 4 2019-05-27 13:49:20 PDT
Created attachment 370706 [details] Patch
Said Abou-Hallawa
Comment 5 2019-05-28 09:28:20 PDT
Comment on attachment 370706 [details] Patch Can you please remove the test from LayoutTests/platform/mac/TestExpectations? It was marked flaky r244263.
Devin Rousso
Comment 6 2019-05-28 10:03:30 PDT
Created attachment 370754 [details] Patch
EWS Watchlist
Comment 7 2019-05-28 11:30:38 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 8 2019-05-28 11:30:39 PDT Comment hidden (obsolete)
WebKit Commit Bot
Comment 9 2019-05-28 12:10:31 PDT
Comment on attachment 370754 [details] Patch Clearing flags on attachment: 370754 Committed r245821: <https://trac.webkit.org/changeset/245821>
WebKit Commit Bot
Comment 10 2019-05-28 12:10:32 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.