* Steps to reproduce: 1. Open WebInspector on a page with a 2D <canvas> 2. Select the 2D <canvas> in the Resources tab 3. Shift-click the recording navigation item to record a single frame 4. Wait for the recording tab to appear once the recording has finished 5. Switch back to the Resources tab => The recording navigation item still shows the grey "stop" box, even though the recording has finished
Created attachment 320744 [details] Patch
Comment on attachment 320744 [details] Patch r-, because we should be able to add test coverage for this. In recording2d.html whenever RecordingFinished is received, we should check that WI.CanvasManager.recordingCanvas is null. I should have added this test when introducing this change.
(In reply to Matt Baker from comment #2) > Comment on attachment 320744 [details] > Patch > > r-, because we should be able to add test coverage for this. In > recording2d.html whenever RecordingFinished is received, we should check > that WI.CanvasManager.recordingCanvas is null. > > I should have added this test when introducing this change. None of the tests use `CanvasManager.prototype.startRecording`. All the recording tests directly call `requestRecording`/`cancelRecording` on `CanvasAgent`. The issue is that the `CanvasManager` functions don't expose the `error` object or allow for a callback, both of which are needed in the tests.
(In reply to Devin Rousso from comment #3) > (In reply to Matt Baker from comment #2) > > Comment on attachment 320744 [details] > > Patch > > > > r-, because we should be able to add test coverage for this. In > > recording2d.html whenever RecordingFinished is received, we should check > > that WI.CanvasManager.recordingCanvas is null. > > > > I should have added this test when introducing this change. > > None of the tests use `CanvasManager.prototype.startRecording`. All the > recording tests directly call `requestRecording`/`cancelRecording` on > `CanvasAgent`. The issue is that the `CanvasManager` functions don't expose > the `error` object or allow for a callback, both of which are needed in the > tests. If CanvasManager isn't testable, then we should take steps to address that. Updating it to be promise-based would be one solution. We could also rely more of event dispatched from the manager. Currently, when requestRecording fails CanvasManager dispatches a RecordingFinished event with a null recording. This is enough information to write a test, as long as you don't care about the actual error message.
Comment on attachment 320744 [details] Patch r=me
Comment on attachment 320744 [details] Patch Clearing flags on attachment: 320744 Committed r222171: <http://trac.webkit.org/changeset/222171>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34693389>