[webkit-reviews] review denied: [Bug 180509] Web Inspector: Canvas: Improve feedback after stopping a recording : [Attachment 328684] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has denied Matt Baker <mattbaker at apple.com>'s
request for review:
Bug 180509: Web Inspector: Canvas: Improve feedback after stopping a recording
https://bugs.webkit.org/show_bug.cgi?id=180509

Attachment 328684: Patch

https://bugs.webkit.org/attachment.cgi?id=328684&action=review




--- Comment #11 from Brian Burg <bburg at apple.com> ---
Comment on attachment 328684
  --> https://bugs.webkit.org/attachment.cgi?id=328684
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328684&action=review

The approach in this patch is prone to getting	out of sync when recordings are
started and stopped via the Console API. For that reason, I think you should
revise this patch.

I would prefer that we expose a 'state' property on the recording object and
fire events when the state changes. This mean we can test the expected state
transitions in a layout test rather than the canvas view trying to track model
state. I think the states you need are Capturing, Loading, Ready.

> LayoutTests/inspector/canvas/recording-2d.html:505
> +		       WI.canvasManager.startRecording(canvas);

Nit: this test would be a lot simpler to follow if these methods returned
promises and the test case turned into an async function.