Summary: Improve CanvasManager recording events. 1) Make CanvasManager interface consistent with TimelineManager: - Add RecordingStarted event - Rename RecordingFinished event to RecordingStopped 2) Remove hack where RecordingFinished was sent to signal that a recording failed to start.
Created attachment 322406 [details] Patch
Comment on attachment 322406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322406&action=review Other than the issue below, this looks fine to me. > Source/WebInspectorUI/UserInterface/Controllers/CanvasManager.js:89 > + this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingStopped, {canvas, recording: null}); This has the possibility of firing `WI.CanvasManager.Event.RecordingStopped` twice: once from this point here, and then another time once the recording object is received in the frontend. The tricky part, however, is that that won't always happen, as a recording with no actions will not call to `recordingFinished` (e.g. start and stop a recording without changing the canvas). Maybe we should keep `RecordingFinished`, but also add `RecordingStopped`?
Created attachment 322486 [details] Patch
Comment on attachment 322486 [details] Patch Attachment 322486 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4743747 New failing tests: inspector/canvas/recording.html inspector/canvas/recording-2d.html inspector/canvas/recording-webgl.html inspector/canvas/recording-webgl-snapshots.html
Created attachment 322566 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 322486 [details] Patch Attachment 322486 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4744096 New failing tests: inspector/canvas/recording.html inspector/canvas/recording-2d.html inspector/canvas/recording-webgl.html inspector/canvas/recording-webgl-snapshots.html
Created attachment 322571 [details] Archive of layout-test-results from ews102 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
I think you forgot to upload the changes to LayoutTests, specifically `WI.CanvasManager.Event.RecordingFinished` -> `WI.CanvasManager.Event.RecordingStopped` :P
Comment on attachment 322486 [details] Patch Attachment 322486 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4744197 New failing tests: inspector/canvas/recording.html inspector/canvas/recording-2d.html inspector/canvas/recording-webgl.html inspector/canvas/recording-webgl-snapshots.html
Created attachment 322572 [details] Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 322631 [details] Patch
Comment on attachment 322631 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322631&action=review r=me. Good refactor =D > Source/WebInspectorUI/ChangeLog:21 > + Make requests of CanvasManager only, defer updating the UI. Style: add newline after comments. > Source/WebInspectorUI/ChangeLog:24 > + Renamed to _recordingStarted. I think you meant "recordingStopped". > LayoutTests/inspector/canvas/recording-2d.html:453 > + let frames = recording.toJSON().frames; Why was the "toJSON" added? The idea is to test the swizzling of each action's parameters and, in this case, ensure that NaN is converted into null.
Created attachment 322695 [details] Patch for landing
Comment on attachment 322695 [details] Patch for landing Clearing flags on attachment: 322695 Committed r222888: <http://trac.webkit.org/changeset/222888>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34824278>