Web Inspector: Improve CanvasManager recording events
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Oct 2017 23:51:47 +0000 (23:51 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Oct 2017 23:51:47 +0000 (23:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177762

Reviewed by Devin Rousso.

Source/JavaScriptCore:

* inspector/protocol/Canvas.json:
Renamed events for clarity and consistency; made recording data optional.

Source/WebCore:

Dispatch Canvas.recordingFinished regardless of whether any actions were
recorded. Without this extra guarantee, the frontend has to keep track
of additional state to determine whether a recording is in progress.

* inspector/InspectorCanvasAgent.cpp:
(WebCore::InspectorCanvasAgent::startRecording):
(WebCore::InspectorCanvasAgent::stopRecording):
(WebCore::InspectorCanvasAgent::didFinishRecordingCanvasFrame):
(WebCore::InspectorCanvasAgent::requestRecording): Deleted.
(WebCore::InspectorCanvasAgent::cancelRecording): Deleted.
* inspector/InspectorCanvasAgent.h:

Source/WebInspectorUI:

* UserInterface/Controllers/CanvasManager.js:
(WI.CanvasManager.prototype.startRecording):
(WI.CanvasManager.prototype.stopRecording):
(WI.CanvasManager.prototype.recordingFinished):
Replace the RecordingFinished event with a pair of events. RecordingStarted
is sent when CanvasAgent.startRecording succeeds. RecordingStopped is
sent when a recordingFinished event is received from the backend, or
when a call to CanvasAgent.stopRecording fails.

* UserInterface/Views/CanvasContentView.js:
(WI.CanvasContentView.prototype.initialLayout):
(WI.CanvasContentView.prototype._toggleRecording):
(WI.CanvasContentView.prototype._recordingStarted):
(WI.CanvasContentView.prototype._recordingFinished): Deleted.
Update recording status when CanvasManager fires recording events,
instead of immediately after clicking the record button.

LayoutTests:

Update tests for renamed CanvasManager event.

* inspector/canvas/recording-2d.html:
* inspector/canvas/recording-expected.txt:
* inspector/canvas/recording-webgl-snapshots.html:
* inspector/canvas/recording-webgl.html:
* inspector/canvas/recording.html:
* inspector/canvas/resources/recording-utilities.js:
(TestPage.registerInitializer):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222888 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/ChangeLog
LayoutTests/inspector/canvas/recording-2d.html
LayoutTests/inspector/canvas/recording-expected.txt
LayoutTests/inspector/canvas/recording-webgl-snapshots.html
LayoutTests/inspector/canvas/recording-webgl.html
LayoutTests/inspector/canvas/recording.html
LayoutTests/inspector/canvas/resources/recording-utilities.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/inspector/protocol/Canvas.json
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorCanvasAgent.cpp
Source/WebCore/inspector/InspectorCanvasAgent.h
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/CanvasManager.js
Source/WebInspectorUI/UserInterface/Views/CanvasContentView.js

index 8a38bdd..8d33a0b 100644 (file)
@@ -1,3 +1,20 @@
+2017-10-04  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Improve CanvasManager recording events
+        https://bugs.webkit.org/show_bug.cgi?id=177762
+
+        Reviewed by Devin Rousso.
+
+        Update tests for renamed CanvasManager event.
+
+        * inspector/canvas/recording-2d.html:
+        * inspector/canvas/recording-expected.txt:
+        * inspector/canvas/recording-webgl-snapshots.html:
+        * inspector/canvas/recording-webgl.html:
+        * inspector/canvas/recording.html:
+        * inspector/canvas/resources/recording-utilities.js:
+        (TestPage.registerInitializer):
+
 2017-10-04  Nan Wang  <n_wang@apple.com>
 
         AX: Make video objects accessible on iOS
index 77ca2a4..b67a634 100644 (file)
@@ -406,7 +406,7 @@ function test() {
         name: "Canvas.recording2D.singleFrame",
         description: "Check that the recording is stopped after a single frame.",
         test(resolve, reject) {
-            requestRecording(WI.Canvas.ContextType.Canvas2D, resolve, reject, {singleFrame: true});
+            startRecording(WI.Canvas.ContextType.Canvas2D, resolve, reject, {singleFrame: true});
         },
     });
 
@@ -414,10 +414,10 @@ function test() {
         name: "Canvas.recording2D.multipleFrames",
         description: "Check that recording data is serialized correctly for multiple frames.",
         test(resolve, reject) {
-            let canvas = requestRecording(WI.Canvas.ContextType.Canvas2D, resolve, reject, {singleFrame: false});
+            let canvas = startRecording(WI.Canvas.ContextType.Canvas2D, resolve, reject, {singleFrame: false});
 
             InspectorTest.singleFireEventListener("LastFrame", () => {
-                CanvasAgent.cancelRecording(canvas.identifier, (error) => {
+                CanvasAgent.stopRecording(canvas.identifier, (error) => {
                     if (error) {
                         reject(error);
                         return;
@@ -431,7 +431,7 @@ function test() {
         name: "Canvas.recording2D.memoryLimit",
         description: "Check that the recording is stopped when it reaches the memory limit.",
         test(resolve, reject) {
-            requestRecording(WI.Canvas.ContextType.Canvas2D, resolve, reject, {memoryLimit: 10});
+            startRecording(WI.Canvas.ContextType.Canvas2D, resolve, reject, {memoryLimit: 10});
         },
     });
 
@@ -445,7 +445,7 @@ function test() {
                 return;
             }
 
-            WI.canvasManager.awaitEvent(WI.CanvasManager.Event.RecordingFinished)
+            WI.canvasManager.awaitEvent(WI.CanvasManager.Event.RecordingStopped)
             .then((event) => {
                 let recording = event.data.recording.toJSON();
 
@@ -460,7 +460,7 @@ function test() {
             .then(resolve, reject);
 
             const singleFrame = true;
-            CanvasAgent.requestRecording(canvas.identifier, singleFrame, (error) => {
+            CanvasAgent.startRecording(canvas.identifier, singleFrame, (error) => {
                 if (error) {
                     reject(error);
                     return;
index 4a72c6f..28d09cd 100644 (file)
@@ -2,11 +2,11 @@ Test general cases of CanvasAgent recording calls.
 
 
 == Running test suite: Canvas.recording
--- Running test case: Canvas.requestRecording.InvalidCanvasId
+-- Running test case: Canvas.startRecording.InvalidCanvasId
 PASS: Should produce an error.
 Error: No canvas for given identifier.
 
--- Running test case: Canvas.cancelRecording.InvalidCanvasId
+-- Running test case: Canvas.stopRecording.InvalidCanvasId
 PASS: Should produce an error.
 Error: No canvas for given identifier.
 
index f4d590d..13d7330 100644 (file)
@@ -96,7 +96,7 @@ function test() {
         name: "Canvas.recordingWebGL.snapshots",
         description: "Check that the snapshot taken after each visual action is different.",
         test(resolve, reject) {
-            requestRecording(WI.Canvas.ContextType.WebGL, resolve, reject, {singleFrame: true});
+            startRecording(WI.Canvas.ContextType.WebGL, resolve, reject, {singleFrame: true});
         },
     });
 
index c1dc917..54c102a 100644 (file)
@@ -503,7 +503,7 @@ function test() {
         name: "Canvas.recordingWebGL.singleFrame",
         description: "Check that the recording is stopped after a single frame.",
         test(resolve, reject) {
-            requestRecording(WI.Canvas.ContextType.WebGL, resolve, reject, {singleFrame: true});
+            startRecording(WI.Canvas.ContextType.WebGL, resolve, reject, {singleFrame: true});
         },
     });
 
@@ -511,10 +511,10 @@ function test() {
         name: "Canvas.recordingWebGL.multipleFrames",
         description: "Check that recording data is serialized correctly for multiple frames.",
         test(resolve, reject) {
-            let canvas = requestRecording(WI.Canvas.ContextType.WebGL, resolve, reject, {singleFrame: false});
+            let canvas = startRecording(WI.Canvas.ContextType.WebGL, resolve, reject, {singleFrame: false});
 
             InspectorTest.singleFireEventListener("LastFrame", () => {
-                CanvasAgent.cancelRecording(canvas.identifier, (error) => {
+                CanvasAgent.stopRecording(canvas.identifier, (error) => {
                     if (error) {
                         reject(error);
                         return;
@@ -528,7 +528,7 @@ function test() {
         name: "Canvas.recordingWebGL.memoryLimit",
         description: "Check that the recording is stopped when it reaches the memory limit.",
         test(resolve, reject) {
-            requestRecording(WI.Canvas.ContextType.WebGL, resolve, reject, {memoryLimit: 10});
+            startRecording(WI.Canvas.ContextType.WebGL, resolve, reject, {memoryLimit: 10});
         },
     });
 
index cf5d5fc..8cf00fe 100644 (file)
@@ -7,11 +7,11 @@ function test() {
     let suite = InspectorTest.createAsyncSuite("Canvas.recording");
 
     suite.addTestCase({
-        name: "Canvas.requestRecording.InvalidCanvasId",
+        name: "Canvas.startRecording.InvalidCanvasId",
         description: "Invalid canvas identifiers should cause an error.",
         test(resolve, reject) {
             const canvasId = "DOES_NOT_EXIST";
-            CanvasAgent.requestRecording(canvasId, (error) => {
+            CanvasAgent.startRecording(canvasId, (error) => {
                 InspectorTest.expectThat(error, "Should produce an error.");
                 InspectorTest.log("Error: " + error);
                 resolve();
@@ -20,11 +20,11 @@ function test() {
     });
 
     suite.addTestCase({
-        name: "Canvas.cancelRecording.InvalidCanvasId",
+        name: "Canvas.stopRecording.InvalidCanvasId",
         description: "Invalid canvas identifiers should cause an error.",
         test(resolve, reject) {
             const canvasId = "DOES_NOT_EXIST";
-            CanvasAgent.cancelRecording(canvasId, (error) => {
+            CanvasAgent.stopRecording(canvasId, (error) => {
                 InspectorTest.expectThat(error, "Should produce an error.");
                 InspectorTest.log("Error: " + error);
                 resolve();
index 917f403..1a932a1 100644 (file)
@@ -93,14 +93,14 @@ TestPage.registerInitializer(() => {
         return canvases[0];
     };
 
-    window.requestRecording = function(type, resolve, reject, {singleFrame, memoryLimit} = {}) {
+    window.startRecording = function(type, resolve, reject, {singleFrame, memoryLimit} = {}) {
         let canvas = getCanvas(type);
         if (!canvas) {
             reject(`Missing canvas with type "${type}".`);
             return;
         }
 
-        WI.canvasManager.awaitEvent(WI.CanvasManager.Event.RecordingFinished).then((event) => {
+        WI.canvasManager.awaitEvent(WI.CanvasManager.Event.RecordingStopped).then((event) => {
             InspectorTest.evaluateInPage(`cancelActions()`);
 
             let recording = event.data.recording;
@@ -111,7 +111,7 @@ TestPage.registerInitializer(() => {
             });
         }).then(resolve, reject);
 
-        CanvasAgent.requestRecording(canvas.identifier, singleFrame, memoryLimit, (error) => {
+        CanvasAgent.startRecording(canvas.identifier, singleFrame, memoryLimit, (error) => {
             if (error) {
                 reject(error);
                 return;
index f02b6aa..7530ca3 100644 (file)
@@ -1,3 +1,13 @@
+2017-10-04  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Improve CanvasManager recording events
+        https://bugs.webkit.org/show_bug.cgi?id=177762
+
+        Reviewed by Devin Rousso.
+
+        * inspector/protocol/Canvas.json:
+        Renamed events for clarity and consistency; made recording data optional.
+
 2017-10-04  JF Bastien  <jfbastien@apple.com>
 
         WTF: Update std::expected to match current proposal
index 64c310a..c1bf08b 100644 (file)
             ]
         },
         {
-            "name": "requestRecording",
-            "description": "Requests that the next frame or up to the given number of bytes of data be recorded for the given canvas.",
+            "name": "startRecording",
+            "description": "Record the next frame, or up to the given number of bytes of data, for the given canvas.",
             "parameters": [
                 { "name": "canvasId", "$ref": "CanvasId" },
                 { "name": "singleFrame", "type": "boolean", "optional": true, "description": "Whether to record a single frame or until the memory limit is reached." },
             ]
         },
         {
-            "name": "cancelRecording",
-            "description": "Cancels a requested recording for the given canvas.",
+            "name": "stopRecording",
+            "description": "Stop recording the given canvas.",
             "parameters": [
                 { "name": "canvasId", "$ref": "CanvasId" }
             ]
             "name": "recordingFinished",
             "parameters": [
                 { "name": "canvasId", "$ref": "CanvasId" },
-                { "name": "recording", "$ref": "Recording.Recording" }
+                { "name": "recording", "$ref": "Recording.Recording", "optional": true }
             ]
         },
         {
index 4e93462..f0bc1b1 100644 (file)
@@ -1,3 +1,22 @@
+2017-10-04  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Improve CanvasManager recording events
+        https://bugs.webkit.org/show_bug.cgi?id=177762
+
+        Reviewed by Devin Rousso.
+
+        Dispatch Canvas.recordingFinished regardless of whether any actions were
+        recorded. Without this extra guarantee, the frontend has to keep track
+        of additional state to determine whether a recording is in progress.
+
+        * inspector/InspectorCanvasAgent.cpp:
+        (WebCore::InspectorCanvasAgent::startRecording):
+        (WebCore::InspectorCanvasAgent::stopRecording):
+        (WebCore::InspectorCanvasAgent::didFinishRecordingCanvasFrame):
+        (WebCore::InspectorCanvasAgent::requestRecording): Deleted.
+        (WebCore::InspectorCanvasAgent::cancelRecording): Deleted.
+        * inspector/InspectorCanvasAgent.h:
+
 2017-10-04  Nan Wang  <n_wang@apple.com>
 
         AX: Make video objects accessible on iOS
index 1ea12d0..afa71ff 100644 (file)
@@ -237,7 +237,7 @@ void InspectorCanvasAgent::resolveCanvasContext(ErrorString& errorString, const
     result = injectedScript.wrapObject(value, objectGroupName);
 }
 
-void InspectorCanvasAgent::requestRecording(ErrorString& errorString, const String& canvasId, const bool* const singleFrame, const int* const memoryLimit)
+void InspectorCanvasAgent::startRecording(ErrorString& errorString, const String& canvasId, const bool* const singleFrame, const int* const memoryLimit)
 {
     auto* inspectorCanvas = assertInspectorCanvas(errorString, canvasId);
     if (!inspectorCanvas)
@@ -257,7 +257,7 @@ void InspectorCanvasAgent::requestRecording(ErrorString& errorString, const Stri
     inspectorCanvas->canvas().renderingContext()->setCallTracingActive(true);
 }
 
-void InspectorCanvasAgent::cancelRecording(ErrorString& errorString, const String& canvasId)
+void InspectorCanvasAgent::stopRecording(ErrorString& errorString, const String& canvasId)
 {
     auto* inspectorCanvas = assertInspectorCanvas(errorString, canvasId);
     if (!inspectorCanvas)
@@ -460,8 +460,11 @@ void InspectorCanvasAgent::didFinishRecordingCanvasFrame(HTMLCanvasElement& canv
     if (!canvasRenderingContext->callTracingActive())
         return;
 
-    if (!inspectorCanvas->hasRecordingData())
+    if (!inspectorCanvas->hasRecordingData()) {
+        if (forceDispatch)
+            m_frontendDispatcher->recordingFinished(inspectorCanvas->identifier(), nullptr);
         return;
+    }
 
     if (!forceDispatch && !inspectorCanvas->singleFrame()) {
         inspectorCanvas->markNewFrame();
index 427d243..4f79da7 100644 (file)
@@ -73,8 +73,8 @@ public:
     void requestContent(ErrorString&, const String& canvasId, String* content) override;
     void requestCSSCanvasClientNodes(ErrorString&, const String& canvasId, RefPtr<Inspector::Protocol::Array<int>>&) override;
     void resolveCanvasContext(ErrorString&, const String& canvasId, const String* const objectGroup, RefPtr<Inspector::Protocol::Runtime::RemoteObject>&) override;
-    void requestRecording(ErrorString&, const String& canvasId, const bool* const singleFrame, const int* const memoryLimit) override;
-    void cancelRecording(ErrorString&, const String& canvasId) override;
+    void startRecording(ErrorString&, const String& canvasId, const bool* const singleFrame, const int* const memoryLimit) override;
+    void stopRecording(ErrorString&, const String& canvasId) override;
     void requestShaderSource(ErrorString&, const String& programId, const String& shaderType, String*) override;
     void updateShader(ErrorString&, const String& programId, const String& shaderType, const String& source) override;
     void setShaderProgramDisabled(ErrorString&, const String& programId, bool disabled) override;
index dcdd219..0481bb5 100644 (file)
@@ -1,3 +1,27 @@
+2017-10-04  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Improve CanvasManager recording events
+        https://bugs.webkit.org/show_bug.cgi?id=177762
+
+        Reviewed by Devin Rousso.
+
+        * UserInterface/Controllers/CanvasManager.js:
+        (WI.CanvasManager.prototype.startRecording):
+        (WI.CanvasManager.prototype.stopRecording):
+        (WI.CanvasManager.prototype.recordingFinished):
+        Replace the RecordingFinished event with a pair of events. RecordingStarted
+        is sent when CanvasAgent.startRecording succeeds. RecordingStopped is
+        sent when a recordingFinished event is received from the backend, or
+        when a call to CanvasAgent.stopRecording fails.
+
+        * UserInterface/Views/CanvasContentView.js:
+        (WI.CanvasContentView.prototype.initialLayout):
+        (WI.CanvasContentView.prototype._toggleRecording):
+        (WI.CanvasContentView.prototype._recordingStarted):
+        (WI.CanvasContentView.prototype._recordingFinished): Deleted.
+        Update recording status when CanvasManager fires recording events,
+        instead of immediately after clicking the record button.
+
 2017-10-04  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Detail Views for resources in Network Tab
index 7765cab..1856a05 100644 (file)
@@ -61,14 +61,14 @@ WI.CanvasManager = class CanvasManager extends WI.Object
 
         this._recordingCanvas = canvas;
 
-        CanvasAgent.requestRecording(canvas.identifier, singleFrame, (error) => {
-            if (!error)
+        CanvasAgent.startRecording(canvas.identifier, singleFrame, (error) => {
+            if (error) {
+                console.error(error);
+                this._recordingCanvas = null;
                 return;
+            }
 
-            console.error(error);
-            this._recordingCanvas = null;
-
-            this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingFinished, {canvas, recording: null});
+            this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingStarted, {canvas});
         });
     }
 
@@ -78,12 +78,15 @@ WI.CanvasManager = class CanvasManager extends WI.Object
         if (!this._recordingCanvas)
             return;
 
-        let canvasIdentifier = this._recordingCanvas.identifier;
+        let canvas = this._recordingCanvas;
         this._recordingCanvas = null;
 
-        CanvasAgent.cancelRecording(canvasIdentifier, (error) => {
-            if (error)
-                console.error(error);
+        CanvasAgent.stopRecording(canvas.identifier, (error) => {
+            if (!error)
+                return;
+
+            console.error(error);
+            this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingStopped, {canvas, recording: null});
         });
     }
 
@@ -155,10 +158,11 @@ WI.CanvasManager = class CanvasManager extends WI.Object
         if (!canvas)
             return;
 
-        let recording = WI.Recording.fromPayload(recordingPayload);
-        recording.source = canvas;
+        let recording = recordingPayload ? WI.Recording.fromPayload(recordingPayload) : null
+        if (recording)
+            recording.source = canvas;
 
-        this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingFinished, {canvas, recording});
+        this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingStopped, {canvas, recording});
     }
 
     programCreated(canvasIdentifier, programIdentifier)
@@ -222,7 +226,8 @@ WI.CanvasManager.Event = {
     Cleared: "canvas-manager-cleared",
     CanvasWasAdded: "canvas-manager-canvas-was-added",
     CanvasWasRemoved: "canvas-manager-canvas-was-removed",
-    RecordingFinished: "canvas-managger-recording-finished",
+    RecordingStarted: "canvas-manager-recording-started",
+    RecordingStopped: "canvas-manager-recording-stopped",
     ShaderProgramAdded: "canvas-manager-shader-program-added",
     ShaderProgramRemoved: "canvas-manager-shader-program-removed",
 };
index 5c7f8c0..9ff1b90 100644 (file)
@@ -69,7 +69,8 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
     {
         super.initialLayout();
 
-        WI.canvasManager.addEventListener(WI.CanvasManager.Event.RecordingFinished, this._recordingFinished, this);
+        WI.canvasManager.addEventListener(WI.CanvasManager.Event.RecordingStarted, this._recordingStarted, this);
+        WI.canvasManager.addEventListener(WI.CanvasManager.Event.RecordingStopped, this._recordingStopped, this);
     }
 
     shown()
@@ -106,11 +107,14 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
             let singleFrame = event.data.nativeEvent.shiftKey;
             WI.canvasManager.startRecording(this.representedObject, singleFrame);
         }
+    }
 
+    _recordingStarted(event)
+    {
         this._updateRecordNavigationItem();
     }
 
-    _recordingFinished(event)
+    _recordingStopped(event)
     {
         this._updateRecordNavigationItem();