Changeset 237401 in webkit

Timestamp:

Oct 24, 2018, 3:12:54 PM (6 years ago)

Author:

Devin Rousso

Message:

Web Inspector: popovers for function source code are malformed
​https://bugs.webkit.org/show_bug.cgi?id=190859

Reviewed by Joseph Pecoraro.

Previously, the WI.Popover would be drawn twice: once when the
WI.CodeMirrorTokenTrackingController determines that the user has hovered on a function
token, and once again when the source for that function is formatted (after being retrieved).
In the case that the formatter is able to return the prettified source within a frame (or
two), the WI.Popover is still in the middle of animating to its new size, meaning that the
changes made by the formatted update will be overridden on the next rAF (creates a flash).

• UserInterface/Views/Popover.js:

(WI.Popover.prototype):
(WI.Popover.prototype._animateFrame.drawBackground):
(WI.Popover.prototype._drawBackground):
Add a member variable to make sure that there is only ever one rAF firing at a time.
Drive-by: rework the background code to only use one canvas.

• UserInterface/Views/SourceCodeTextEditor.js:

(WI.SourceCodeTextEditor.prototype._showPopoverForFunction.didGetDetails):
Don't show the WI.Popover until the formatter has finished prettifying the function's
source code, as otherwise there is brief moment that the popover appears and is empty.

Location:

trunk/Source/WebInspectorUI

Files:

• ChangeLog (modified) (1 diff)
• UserInterface/Views/Popover.js (modified) (5 diffs)
• UserInterface/Views/SourceCodeTextEditor.js (modified) (2 diffs)

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2018-10-24  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: popovers for function source code are malformed
+        https://bugs.webkit.org/show_bug.cgi?id=190859
+
+        Reviewed by Joseph Pecoraro.
+
+        Previously, the `WI.Popover` would be drawn twice: once when the
+        `WI.CodeMirrorTokenTrackingController` determines that the user has hovered on a function
+        token, and once again when the source for that function is formatted (after being retrieved).
+        In the case that the formatter is able to return the prettified source within a frame (or
+        two), the `WI.Popover` is still in the middle of animating to its new size, meaning that the
+        changes made by the formatted `update` will be overridden on the next rAF (creates a flash).
+
+        * UserInterface/Views/Popover.js:
+        (WI.Popover.prototype):
+        (WI.Popover.prototype._animateFrame.drawBackground):
+        (WI.Popover.prototype._drawBackground):
+        Add a member variable to make sure that there is only ever one rAF firing at a time.
+        Drive-by: rework the background code to only use one canvas.
+
+        * UserInterface/Views/SourceCodeTextEditor.js:
+        (WI.SourceCodeTextEditor.prototype._showPopoverForFunction.didGetDetails):
+        Don't show the `WI.Popover` until the formatter has finished prettifying the function's
+        source code, as otherwise there is brief moment that the popover appears and is empty.
+
 2018-10-24  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebInspectorUI/UserInterface/Views/Popover.js

@@ -48 +48 @@
         this._container = this._element.appendChild(document.createElement("div"));
         this._container.className = "container";
+
+        this._drawBackgroundAnimationIdentifier = undefined;
     }
 
@@ -371 +373 @@
 
             if (progress < 1)
-                requestAnimationFrame(drawBackground.bind(this));
+                this._drawBackgroundAnimationIdentifier = requestAnimationFrame(drawBackground.bind(this));
         }
 
@@ -379 +381 @@
     _drawBackground()
     {
+        if (this._drawBackgroundAnimationIdentifier) {
+            cancelAnimationFrame(this._drawBackgroundAnimationIdentifier);
+            this._drawBackgroundAnimationIdentifier = undefined;
+        }
+
         let scaleFactor = window.devicePixelRatio;
 
@@ -385 +392 @@
         let scaledWidth = width * scaleFactor;
         let scaledHeight = height * scaleFactor;
-
-        // Create a scratch canvas so we can draw the popover that will later be drawn into
-        // the final context with a shadow.
-        let scratchCanvas = document.createElement("canvas");
-        scratchCanvas.width = scaledWidth;
-        scratchCanvas.height = scaledHeight;
-
-        let ctx = scratchCanvas.getContext("2d");
-        ctx.scale(scaleFactor, scaleFactor);
 
         // Bounds of the path don't take into account the arrow, but really only the tight bounding box
@@ -417 +415 @@
         let computedStyle = window.getComputedStyle(this._element, null);
 
+        let context = document.getCSSCanvasContext("2d", "popover", scaledWidth, scaledHeight);
+        context.clearRect(0, 0, scaledWidth, scaledHeight);
+        context.shadowOffsetX = 1;
+        context.shadowOffsetY = 1;
+        context.shadowBlur = 5;
+        context.shadowColor = computedStyle.getPropertyValue("--popover-shadow-color").trim();
+        context.resetTransform();
+        context.scale(scaleFactor, scaleFactor);
+
         // Clip the frame.
-        ctx.fillStyle = computedStyle.getPropertyValue("--popover-text-color").trim();
-        this._drawFrame(ctx, bounds, this._edge, this._anchorPoint);
-        ctx.clip();
+        context.fillStyle = computedStyle.getPropertyValue("--popover-text-color").trim();
+        this._drawFrame(context, bounds, this._edge, this._anchorPoint);
+        context.clip();
 
         // Panel background color fill.
-        ctx.fillStyle = computedStyle.getPropertyValue("--popover-background-color").trim();
-
-        ctx.fillRect(0, 0, width, height);
+        context.fillStyle = computedStyle.getPropertyValue("--popover-background-color").trim();
+
+        context.fillRect(0, 0, width, height);
 
         // Stroke.
-        ctx.strokeStyle = computedStyle.getPropertyValue("--popover-border-color").trim();
-        ctx.lineWidth = 2;
-        this._drawFrame(ctx, bounds, this._edge, this._anchorPoint);
-        ctx.stroke();
-
-        // Draw the popover into the final context with a drop shadow.
-        let finalContext = document.getCSSCanvasContext("2d", "popover", scaledWidth, scaledHeight);
-        finalContext.clearRect(0, 0, scaledWidth, scaledHeight);
-        finalContext.shadowOffsetX = 1;
-        finalContext.shadowOffsetY = 1;
-        finalContext.shadowBlur = 5;
-        finalContext.shadowColor = computedStyle.getPropertyValue("--popover-shadow-color").trim();
-        finalContext.drawImage(scratchCanvas, 0, 0, scaledWidth, scaledHeight);
+        context.strokeStyle = computedStyle.getPropertyValue("--popover-border-color").trim();
+        context.lineWidth = 2;
+        this._drawFrame(context, bounds, this._edge, this._anchorPoint);
+        context.stroke();
     }
 

trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js

@@ -1876 +1876 @@
                 readOnly: "nocursor",
             });
-            codeMirror.on("update", () => {
-                this._popover.update();
-            });
 
             const isModule = false;
@@ -1885 +1882 @@
             let workerProxy = WI.FormatterWorkerProxy.singleton();
             workerProxy.formatJavaScript(data.description, isModule, indentString, includeSourceMapData, ({formattedText}) => {
+                if (candidate !== this.tokenTrackingController.candidate)
+                    return;
+
+                this._showPopover(content);
                 codeMirror.setValue(formattedText || data.description);
+                this._popover.update();
             });
-
-            this._showPopover(content);
         }