Web Inspector: Canvas: send a call stack with each action instead of an array of...

https://bugs.webkit.org/show_bug.cgi?id=191628

Reviewed by Dean Jackson.

Source/WebCore:

Updated existing test: inspector/model/recording.html

* inspector/InspectorCanvas.h:
* inspector/InspectorCanvas.cpp:
(WebCore::InspectorCanvas::indexForData):
(WebCore::InspectorCanvas::buildInitialState):
(WebCore::InspectorCanvas::buildAction):
Drive-by: prevent de-duplicated objects from being destroyed while recording.
Source/WebInspectorUI:

* UserInterface/Models/Recording.js:
(WI.Recording.prototype.async swizzle):

* UserInterface/Models/RecordingAction.js:
(WI.RecordingAction.fromPayload):
(WI.RecordingAction.prototype.async swizzle):

LayoutTests:

* inspector/model/recording.html:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 4dcc14b..dad2a80 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2018-11-14  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Canvas: send a call stack with each action instead of an array of call frames
+        https://bugs.webkit.org/show_bug.cgi?id=191628
+
+        Reviewed by Dean Jackson.
+
+        * inspector/model/recording.html:
+
 2018-11-14  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed test gardening, move a skip expectation to a more specific file.

diff --git a/LayoutTests/inspector/model/recording-expected.txt b/LayoutTests/inspector/model/recording-expected.txt
index 6ea720d..8ccec82 100644 (file)
--- a/LayoutTests/inspector/model/recording-expected.txt
+++ b/LayoutTests/inspector/model/recording-expected.txt
@@ -160,9 +160,7 @@ null
           [
             0
           ],
-          [
-            0
-          ]
+          0
         ]
       ],
       "duration": 1,

diff --git a/LayoutTests/inspector/model/recording.html b/LayoutTests/inspector/model/recording.html
index 6ee6970..5981232 100644 (file)
--- a/LayoutTests/inspector/model/recording.html
+++ b/LayoutTests/inspector/model/recording.html
@@ -168,7 +168,7 @@ function test()
                                 0,
                                 [0],
                                 [0],
-                                [0],
+                                0,
                             ],
                         ],
                         duration: 1,

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 5f01526..ef32b42 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2018-11-14  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Canvas: send a call stack with each action instead of an array of call frames
+        https://bugs.webkit.org/show_bug.cgi?id=191628
+
+        Reviewed by Dean Jackson.
+
+        Updated existing test: inspector/model/recording.html
+
+        * inspector/InspectorCanvas.h:
+        * inspector/InspectorCanvas.cpp:
+        (WebCore::InspectorCanvas::indexForData):
+        (WebCore::InspectorCanvas::buildInitialState):
+        (WebCore::InspectorCanvas::buildAction):
+        Drive-by: prevent de-duplicated objects from being destroyed while recording.
+
 2018-11-14  Stephan Szabo  <stephan.szabo@sony.com>
 
         [Win] Compile Service Worker support

diff --git a/Source/WebCore/inspector/InspectorCanvas.cpp b/Source/WebCore/inspector/InspectorCanvas.cpp
index c6517e6..ea710bc 100644 (file)
--- a/Source/WebCore/inspector/InspectorCanvas.cpp
+++ b/Source/WebCore/inspector/InspectorCanvas.cpp
@@ -67,10 +67,8 @@
 #include "WebMetalRenderingContext.h"
 #endif
 #include <JavaScriptCore/IdentifiersFactory.h>
-#include <JavaScriptCore/ScriptCallStack.h>
 #include <JavaScriptCore/ScriptCallStackFactory.h>
 
-
 namespace WebCore {
 
 using namespace Inspector;
@@ -343,7 +341,17 @@ String InspectorCanvas::getCanvasContentAsDataURL()
 
 int InspectorCanvas::indexForData(DuplicateDataVariant data)
 {
-    size_t index = m_indexedDuplicateData.find(data);
+    size_t index = m_indexedDuplicateData.findMatching([&] (auto item) {
+        if (data == item)
+            return true;
+
+        auto traceA = WTF::get_if<RefPtr<ScriptCallStack>>(data);
+        auto traceB = WTF::get_if<RefPtr<ScriptCallStack>>(item);
+        if (traceA && *traceA && traceB && *traceB)
+            return (*traceA)->isEqual((*traceB).get());
+
+        return false;
+    });
     if (index != notFound) {
         ASSERT(index < std::numeric_limits<int>::max());
         return static_cast<int>(index);
@@ -354,7 +362,7 @@ int InspectorCanvas::indexForData(DuplicateDataVariant data)
 
     RefPtr<JSON::Value> item;
     WTF::switchOn(data,
-        [&] (const HTMLImageElement* imageElement) {
+        [&] (const RefPtr<HTMLImageElement>& imageElement) {
             String dataURL = "data:,"_s;
 
             if (CachedImage* cachedImage = imageElement->cachedImage()) {
@@ -369,7 +377,7 @@ int InspectorCanvas::indexForData(DuplicateDataVariant data)
             index = indexForData(dataURL);
         },
 #if ENABLE(VIDEO)
-        [&] (HTMLVideoElement* videoElement) {
+        [&] (RefPtr<HTMLVideoElement>& videoElement) {
             String dataURL = "data:,"_s;
 
             unsigned videoWidth = videoElement->videoWidth();
@@ -383,7 +391,7 @@ int InspectorCanvas::indexForData(DuplicateDataVariant data)
             index = indexForData(dataURL);
         },
 #endif
-        [&] (HTMLCanvasElement* canvasElement) {
+        [&] (RefPtr<HTMLCanvasElement>& canvasElement) {
             String dataURL = "data:,"_s;
 
             ExceptionOr<UncachedString> result = canvasElement->toDataURL("image/png"_s);
@@ -392,12 +400,18 @@ int InspectorCanvas::indexForData(DuplicateDataVariant data)
 
             index = indexForData(dataURL);
         },
-        [&] (const CanvasGradient* canvasGradient) { item = buildArrayForCanvasGradient(*canvasGradient); },
-        [&] (const CanvasPattern* canvasPattern) { item = buildArrayForCanvasPattern(*canvasPattern); },
-        [&] (const ImageData* imageData) { item = buildArrayForImageData(*imageData); },
-        [&] (ImageBitmap* imageBitmap) {
+        [&] (const RefPtr<CanvasGradient>& canvasGradient) { item = buildArrayForCanvasGradient(*canvasGradient); },
+        [&] (const RefPtr<CanvasPattern>& canvasPattern) { item = buildArrayForCanvasPattern(*canvasPattern); },
+        [&] (const RefPtr<ImageData>& imageData) { item = buildArrayForImageData(*imageData); },
+        [&] (RefPtr<ImageBitmap>& imageBitmap) {
             index = indexForData(imageBitmap->buffer()->toDataURL("image/png"));
         },
+        [&] (const RefPtr<ScriptCallStack>& scriptCallStack) {
+            auto array = JSON::ArrayOf<double>::create();
+            for (size_t i = 0; i < scriptCallStack->size(); ++i)
+                array->addItem(indexForData(scriptCallStack->at(i)));
+            item = WTFMove(array);
+        },
         [&] (const ScriptCallFrame& scriptCallFrame) {
             auto array = JSON::ArrayOf<double>::create();
             array->addItem(indexForData(scriptCallFrame.functionName()));
@@ -489,18 +503,18 @@ Ref<Inspector::Protocol::Recording::InitialState> InspectorCanvas::buildInitialS
 
             int strokeStyleIndex;
             if (auto canvasGradient = state.strokeStyle.canvasGradient())
-                strokeStyleIndex = indexForData(canvasGradient.get());
+                strokeStyleIndex = indexForData(canvasGradient);
             else if (auto canvasPattern = state.strokeStyle.canvasPattern())
-                strokeStyleIndex = indexForData(canvasPattern.get());
+                strokeStyleIndex = indexForData(canvasPattern);
             else
                 strokeStyleIndex = indexForData(state.strokeStyle.color());
             statePayload->setInteger(stringIndexForKey("strokeStyle"_s), strokeStyleIndex);
 
             int fillStyleIndex;
             if (auto canvasGradient = state.fillStyle.canvasGradient())
-                fillStyleIndex = indexForData(canvasGradient.get());
+                fillStyleIndex = indexForData(canvasGradient);
             else if (auto canvasPattern = state.fillStyle.canvasPattern())
-                fillStyleIndex = indexForData(canvasPattern.get());
+                fillStyleIndex = indexForData(canvasPattern);
             else
                 fillStyleIndex = indexForData(state.fillStyle.color());
             statePayload->setInteger(stringIndexForKey("fillStyle"_s), fillStyleIndex);
@@ -598,16 +612,16 @@ Ref<JSON::ArrayOf<JSON::Value>> InspectorCanvas::buildAction(const String& name,
 #endif
             [&] (const RefPtr<ArrayBuffer>&) { addParameter(0, RecordingSwizzleTypes::TypedArray); },
             [&] (const RefPtr<ArrayBufferView>&) { addParameter(0, RecordingSwizzleTypes::TypedArray); },
-            [&] (const RefPtr<CanvasGradient>& value) { addParameter(indexForData(value.get()), RecordingSwizzleTypes::CanvasGradient); },
-            [&] (const RefPtr<CanvasPattern>& value) { addParameter(indexForData(value.get()), RecordingSwizzleTypes::CanvasPattern); },
+            [&] (const RefPtr<CanvasGradient>& value) { addParameter(indexForData(value), RecordingSwizzleTypes::CanvasGradient); },
+            [&] (const RefPtr<CanvasPattern>& value) { addParameter(indexForData(value), RecordingSwizzleTypes::CanvasPattern); },
             [&] (const RefPtr<Float32Array>&) { addParameter(0, RecordingSwizzleTypes::TypedArray); },
-            [&] (const RefPtr<HTMLCanvasElement>& value) { addParameter(indexForData(value.get()), RecordingSwizzleTypes::Image); },
-            [&] (const RefPtr<HTMLImageElement>& value) { addParameter(indexForData(value.get()), RecordingSwizzleTypes::Image); },
+            [&] (const RefPtr<HTMLCanvasElement>& value) { addParameter(indexForData(value), RecordingSwizzleTypes::Image); },
+            [&] (const RefPtr<HTMLImageElement>& value) { addParameter(indexForData(value), RecordingSwizzleTypes::Image); },
 #if ENABLE(VIDEO)
-            [&] (const RefPtr<HTMLVideoElement>& value) { addParameter(indexForData(value.get()), RecordingSwizzleTypes::Image); },
+            [&] (const RefPtr<HTMLVideoElement>& value) { addParameter(indexForData(value), RecordingSwizzleTypes::Image); },
 #endif
-            [&] (const RefPtr<ImageBitmap>& value) { addParameter(indexForData(value.get()), RecordingSwizzleTypes::ImageBitmap); },
-            [&] (const RefPtr<ImageData>& value) { addParameter(indexForData(value.get()), RecordingSwizzleTypes::ImageData); },
+            [&] (const RefPtr<ImageBitmap>& value) { addParameter(indexForData(value), RecordingSwizzleTypes::ImageBitmap); },
+            [&] (const RefPtr<ImageData>& value) { addParameter(indexForData(value), RecordingSwizzleTypes::ImageData); },
             [&] (const RefPtr<Int32Array>&) { addParameter(0, RecordingSwizzleTypes::TypedArray); },
             [&] (const Vector<float>& value) { addParameter(buildArrayForVector(value).ptr(), RecordingSwizzleTypes::Array); },
             [&] (const Vector<int>& value) { addParameter(buildArrayForVector(value).ptr(), RecordingSwizzleTypes::Array); },
@@ -625,11 +639,8 @@ Ref<JSON::ArrayOf<JSON::Value>> InspectorCanvas::buildAction(const String& name,
     action->addItem(WTFMove(parametersData));
     action->addItem(WTFMove(swizzleTypes));
 
-    auto trace = JSON::ArrayOf<double>::create();
-    auto stackTrace = Inspector::createScriptCallStack(JSExecState::currentState(), Inspector::ScriptCallStack::maxCallStackSizeToCapture);
-    for (size_t i = 0; i < stackTrace->size(); ++i)
-        trace->addItem(indexForData(stackTrace->at(i)));
-    action->addItem(WTFMove(trace));
+    RefPtr<ScriptCallStack> trace = Inspector::createScriptCallStack(JSExecState::currentState(), Inspector::ScriptCallStack::maxCallStackSizeToCapture);
+    action->addItem(indexForData(WTFMove(trace)));
 
     return action;
 }

diff --git a/Source/WebCore/inspector/InspectorCanvas.h b/Source/WebCore/inspector/InspectorCanvas.h
index 2abc63f..dbab643 100644 (file)
--- a/Source/WebCore/inspector/InspectorCanvas.h
+++ b/Source/WebCore/inspector/InspectorCanvas.h
@@ -28,6 +28,7 @@
 #include "CallTracerTypes.h"
 #include <JavaScriptCore/InspectorProtocolObjects.h>
 #include <JavaScriptCore/ScriptCallFrame.h>
+#include <JavaScriptCore/ScriptCallStack.h>
 #include <wtf/Variant.h>
 #include <wtf/Vector.h>
 #include <wtf/text/WTFString.h>
@@ -82,15 +83,16 @@ private:
     String getCanvasContentAsDataURL();
 
     using DuplicateDataVariant = Variant<
-        CanvasGradient*,
-        CanvasPattern*,
-        HTMLCanvasElement*,
-        HTMLImageElement*,
+        RefPtr<CanvasGradient>,
+        RefPtr<CanvasPattern>,
+        RefPtr<HTMLCanvasElement>,
+        RefPtr<HTMLImageElement>,
 #if ENABLE(VIDEO)
-        HTMLVideoElement*,
+        RefPtr<HTMLVideoElement>,
 #endif
-        ImageData*,
-        ImageBitmap*,
+        RefPtr<ImageData>,
+        RefPtr<ImageBitmap>,
+        RefPtr<Inspector::ScriptCallStack>,
         Inspector::ScriptCallFrame,
         String
     >;

diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
index 4c019e3..be4e9bb 100644 (file)
--- a/Source/WebInspectorUI/ChangeLog
+++ b/Source/WebInspectorUI/ChangeLog
@@ -1,5 +1,19 @@
 2018-11-14  Devin Rousso  <drousso@apple.com>
 
+        Web Inspector: Canvas: send a call stack with each action instead of an array of call frames
+        https://bugs.webkit.org/show_bug.cgi?id=191628
+
+        Reviewed by Dean Jackson.
+
+        * UserInterface/Models/Recording.js:
+        (WI.Recording.prototype.async swizzle):
+
+        * UserInterface/Models/RecordingAction.js:
+        (WI.RecordingAction.fromPayload):
+        (WI.RecordingAction.prototype.async swizzle):
+
+2018-11-14  Devin Rousso  <drousso@apple.com>
+
         Web Inspector: add drag+drop for importing Audits and Recordings
         https://bugs.webkit.org/show_bug.cgi?id=191566
 

diff --git a/Source/WebInspectorUI/UserInterface/Models/Recording.js b/Source/WebInspectorUI/UserInterface/Models/Recording.js
index 96e8d06..d05f104 100644 (file)
--- a/Source/WebInspectorUI/UserInterface/Models/Recording.js
+++ b/Source/WebInspectorUI/UserInterface/Models/Recording.js
@@ -312,6 +312,27 @@ WI.Recording = class Recording extends WI.Object
                     var image = await this.swizzle(index, WI.Recording.Swizzle.Image);
                     this._swizzle[index][type] = await createImageBitmap(image);
                     break;
+
+                case WI.Recording.Swizzle.CallStack: {
+                    let array = await this.swizzle(data, WI.Recording.Swizzle.Array);
+                    this._swizzle[index][type] = await Promise.all(array.map((item) => this.swizzle(item, WI.Recording.Swizzle.CallFrame)));
+                    break;
+                }
+
+                case WI.Recording.Swizzle.CallFrame: {
+                    let array = await this.swizzle(data, WI.Recording.Swizzle.Array);
+                    let [functionName, url] = await Promise.all([
+                        this.swizzle(array[0], WI.Recording.Swizzle.String),
+                        this.swizzle(array[1], WI.Recording.Swizzle.String),
+                    ]);
+                    this._swizzle[index][type] = WI.CallFrame.fromPayload(WI.assumingMainTarget(), {
+                        functionName,
+                        url,
+                        lineNumber: array[2],
+                        columnNumber: array[3],
+                    });
+                    break;
+                }
                 }
             } catch { }
         }
@@ -488,4 +509,8 @@ WI.Recording.Swizzle = {
     WebGLProgram: 17,
     WebGLUniformLocation: 18,
     ImageBitmap: 19,
+
+    // Special frontend-only swizzle types.
+    CallStack: Symbol("CallStack"),
+    CallFrame: Symbol("CallFrame"),
 };

diff --git a/Source/WebInspectorUI/UserInterface/Models/RecordingAction.js b/Source/WebInspectorUI/UserInterface/Models/RecordingAction.js
index 8010209..e83a710 100644 (file)
--- a/Source/WebInspectorUI/UserInterface/Models/RecordingAction.js
+++ b/Source/WebInspectorUI/UserInterface/Models/RecordingAction.js
@@ -57,7 +57,7 @@ WI.RecordingAction = class RecordingAction extends WI.Object
 
     // Static
 
-    // Payload format: [name, parameters, swizzleTypes, trace, [snapshot]]
+    // Payload format: (name, parameters, swizzleTypes, [trace, [snapshot]])
     static fromPayload(payload)
     {
         if (!Array.isArray(payload))
@@ -72,8 +72,11 @@ WI.RecordingAction = class RecordingAction extends WI.Object
         if (!Array.isArray(payload[2]))
             payload[2] = [];
 
-        if (!Array.isArray(payload[3]))
-            payload[3] = [];
+        if (isNaN(payload[3]) || (!payload[3] && payload[3] !== 0)) {
+            // COMPATIBILITY (iOS 12.1): "trace" was sent as an array of call frames instead of a single call stack
+            if (!Array.isArray(payload[3]))
+                payload[3] = [];
+        }
 
         if (payload.length >= 5 && isNaN(payload[4]))
             payload[4] = -1;
@@ -262,25 +265,18 @@ WI.RecordingAction = class RecordingAction extends WI.Object
             return recording.swizzle(item, this._payloadSwizzleTypes[index]);
         };
 
-        let swizzleCallFrame = async (item, index) => {
-            let array = await recording.swizzle(item, WI.Recording.Swizzle.None);
-            let [functionName, url] = await Promise.all([
-                recording.swizzle(array[0], WI.Recording.Swizzle.String),
-                recording.swizzle(array[1], WI.Recording.Swizzle.String),
-            ]);
-            return WI.CallFrame.fromPayload(WI.mainTarget, {
-                functionName,
-                url,
-                lineNumber: array[2],
-                columnNumber: array[3],
-            });
-        };
-
         let swizzlePromises = [
             recording.swizzle(this._payloadName, WI.Recording.Swizzle.String),
             Promise.all(this._payloadParameters.map(swizzleParameter)),
-            Promise.all(this._payloadTrace.map(swizzleCallFrame)),
         ];
+
+        if (!isNaN(this._payloadTrace))
+            swizzlePromises.push(recording.swizzle(this._payloadTrace, WI.Recording.Swizzle.CallStack))
+        else {
+            // COMPATIBILITY (iOS 12.1): "trace" was sent as an array of call frames instead of a single call stack
+            swizzlePromises.push(Promise.all(this._payloadTrace.map((item) => recording.swizzle(item, WI.Recording.Swizzle.CallFrame))));
+        }
+
         if (this._payloadSnapshot >= 0)
             swizzlePromises.push(recording.swizzle(this._payloadSnapshot, WI.Recording.Swizzle.String));