[webkit-reviews] review granted: [Bug 177762] Web Inspector: Improve CanvasManager recording events : [Attachment 322631] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <webkit at devinrousso.com> has granted Matt Baker
<mattbaker at apple.com>'s request for review:
Bug 177762: Web Inspector: Improve CanvasManager recording events
https://bugs.webkit.org/show_bug.cgi?id=177762

Attachment 322631: Patch

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




--- Comment #12 from Devin Rousso <webkit at devinrousso.com> ---
Comment on attachment 322631
  --> https://bugs.webkit.org/attachment.cgi?id=322631
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.