Web Inspector: Canvas: a recording initiated by the user should be sh…

…own immediately on completion

https://bugs.webkit.org/show_bug.cgi?id=183647
<rdar://problem/38479187>

Reviewed by Matt Baker.

Source/WebInspectorUI:

When recordings are initiated via `console.record`, we don't want to automatically show the
recording after it's payload is sent to the frontend. We determine whether a recording came
from the console by comparing the recording's associated canvas with the current value of
`_recordingCanvas`. Previously, when stopping a recording, we would always null the value,
which meant that all non-single-frame recordings (single-frame recordings are stopped by
the agent after the first paint or tick after an action is performed) would be categorized
as coming from the console, since `_recordingCanvas` would be null by the time the frontend
recieved the payload.

This patch changes it so that the nulling of `_recordingCanvas` in `stopRecording` is only
done if the agent command errors. It was already the case that `_recordingCanvas` was nulled
in `stopRecording`, so this patch just prevents it from being nulled too early.

* UserInterface/Controllers/CanvasManager.js:
(WI.CanvasManager.prototype.stopRecording):

LayoutTests:

* inspector/canvas/recording-2d.html:
Reorder the agent calls so that the CanvasManager `_recordingCanvas` state variable has a
chance to properly update after a recording is stopped.


Canonical link: https://commits.webkit.org/199300@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229620 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,15 @@
+2018-03-14  Devin Rousso  <webkit@devinrousso.com>
+
+        Web Inspector: Canvas: a recording initiated by the user should be shown immediately on completion
+        https://bugs.webkit.org/show_bug.cgi?id=183647
+        <rdar://problem/38479187>
+
+        Reviewed by Matt Baker.
+
+        * inspector/canvas/recording-2d.html:
+        Reorder the agent calls so that the CanvasManager `_recordingCanvas` state variable has a
+        chance to properly update after a recording is stopped.
+
 2018-03-14  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r229613.

LayoutTests/inspector/canvas/recording-2d.html

@@ -508,9 +508,12 @@
                 InspectorTest.assert(!event.data.recording, "The recording payload should be null.");
 
                 ++eventCount;
-                if (eventCount == 1)
+                if (eventCount == 1) {
                     InspectorTest.pass("A recording should have been started and stopped once.");
-                else if (eventCount >= 2) {
+
+                    WI.canvasManager.startRecording(canvas);
+                    WI.canvasManager.stopRecording();
+                } else if (eventCount >= 2) {
                     InspectorTest.pass("A recording should have been started and stopped twice.");
 
                     WI.canvasManager.removeEventListener(handleRecordingStopped);
@@ -521,9 +524,6 @@
 
             WI.canvasManager.startRecording(canvas);
             WI.canvasManager.stopRecording();
-
-            WI.canvasManager.startRecording(canvas);
-            WI.canvasManager.stopRecording();
         },
     });
 

Source/WebInspectorUI/ChangeLog

@@ -1,3 +1,27 @@
+2018-03-14  Devin Rousso  <webkit@devinrousso.com>
+
+        Web Inspector: Canvas: a recording initiated by the user should be shown immediately on completion
+        https://bugs.webkit.org/show_bug.cgi?id=183647
+        <rdar://problem/38479187>
+
+        Reviewed by Matt Baker.
+
+        When recordings are initiated via `console.record`, we don't want to automatically show the
+        recording after it's payload is sent to the frontend. We determine whether a recording came
+        from the console by comparing the recording's associated canvas with the current value of
+        `_recordingCanvas`. Previously, when stopping a recording, we would always null the value,
+        which meant that all non-single-frame recordings (single-frame recordings are stopped by
+        the agent after the first paint or tick after an action is performed) would be categorized
+        as coming from the console, since `_recordingCanvas` would be null by the time the frontend
+        recieved the payload.
+
+        This patch changes it so that the nulling of `_recordingCanvas` in `stopRecording` is only
+        done if the agent command errors. It was already the case that `_recordingCanvas` was nulled
+        in `stopRecording`, so this patch just prevents it from being nulled too early.
+
+        * UserInterface/Controllers/CanvasManager.js:
+        (WI.CanvasManager.prototype.stopRecording):
+
 2018-03-12  Jon Davis  <jond@apple.com>
 
         Web Inspector: Remove redundant tooltips

Source/WebInspectorUI/UserInterface/Controllers/CanvasManager.js

@@ -110,13 +110,13 @@ WI.CanvasManager = class CanvasManager extends WI.Object
             return;
 
         let canvas = this._recordingCanvas;
-        this._recordingCanvas = null;
 
         CanvasAgent.stopRecording(canvas.identifier, (error) => {
             if (!error)
                 return;
 
             console.error(error);
+            this._recordingCanvas = null;
             this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingStopped, {canvas, recording: null});
         });
     }