Changeset 276170 in webkit

Timestamp:

Apr 16, 2021, 2:22:00 PM (5 years ago)

Author:

Devin Rousso

Message:

Web Inspector: REGRESSION(?): Graphics: dropping a recording leaves behind a drop zone view
​https://bugs.webkit.org/show_bug.cgi?id=224648

Reviewed by BJ Burg.

• UserInterface/Views/GraphicsTabContentView.js:

(WI.GraphicsTabContentView.prototype.initialLayout):
It appears that it's possible for re-entrancy issues in the WI.View system since the
_didInitialLayout flag isn't set until _after_ initialLayout returns, meaning that if
the logic inside initialLayout triggers a synchronous layout then that second layout
won't know that it's already in the middle of an initialLayout. In this case, showing the
WI.GraphicsOverviewContentView causes the navigation sidebar to be shown, which forces a
synchronous layout from handling WI.Sidebar.Event.WidthDidChange. For now, there's no
"rush" to show the WI.GraphicsOverviewContentView so we delay it by one event loop turn.

• UserInterface/Views/CanvasContentView.js:

(WI.CanvasContentView.prototype.initialLayout):
(WI.CanvasContentView.prototype.dropZoneShouldAppearForDragEvent): Deleted.
(WI.CanvasContentView.prototype.dropZoneHandleDrop): Deleted.
There's no reason to have another WI.DropZoneView here since there's already one that
covers the entire tab.

Location:

trunk/Source/WebInspectorUI

Files:

• ChangeLog (modified) (1 diff)
• UserInterface/Views/CanvasContentView.js (modified) (2 diffs)
• UserInterface/Views/GraphicsTabContentView.js (modified) (1 diff)

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2021-04-16  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: REGRESSION(?): Graphics: dropping a recording leaves behind a drop zone view
+        https://bugs.webkit.org/show_bug.cgi?id=224648
+
+        Reviewed by BJ Burg.
+
+        * UserInterface/Views/GraphicsTabContentView.js:
+        (WI.GraphicsTabContentView.prototype.initialLayout):
+        It appears that it's possible for re-entrancy issues in the `WI.View` system since the
+        `_didInitialLayout` flag isn't set until _after_ `initialLayout` returns, meaning that if
+        the logic inside `initialLayout` triggers a synchronous `layout` then that second `layout`
+        won't know that it's already in the middle of an `initialLayout`. In this case, showing the
+        `WI.GraphicsOverviewContentView` causes the navigation sidebar to be shown, which forces a
+        synchronous `layout` from handling `WI.Sidebar.Event.WidthDidChange`. For now, there's no
+        "rush" to show the `WI.GraphicsOverviewContentView` so we delay it by one event loop turn.
+
+        * UserInterface/Views/CanvasContentView.js:
+        (WI.CanvasContentView.prototype.initialLayout):
+        (WI.CanvasContentView.prototype.dropZoneShouldAppearForDragEvent): Deleted.
+        (WI.CanvasContentView.prototype.dropZoneHandleDrop): Deleted.
+        There's no reason to have another `WI.DropZoneView` here since there's already one that
+        covers the entire tab.
+
 2021-04-16  Devin Rousso  <drousso@apple.com>
 

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

@@ -83 +83 @@
     }
 
-    // DropZoneView delegate
-
-    dropZoneShouldAppearForDragEvent(dropZone, event)
-    {
-        return event.dataTransfer.types.includes("Files");
-    }
-
-    dropZoneHandleDrop(dropZone, event)
-    {
-        let files = event.dataTransfer.files;
-        if (files.length !== 1) {
-            InspectorFrontendHost.beep();
-            return;
-        }
-
-        WI.FileUtilities.readJSON(files, (result) => WI.canvasManager.processJSON(result));
-    }
-
     // Protected
 
@@ -184 +166 @@
         if (isCard)
             this._refreshPixelSize();
-
-        if (!isCard) {
-            let dropZoneView = new WI.DropZoneView(this);
-            dropZoneView.text = WI.UIString("Import Recording");
-            dropZoneView.targetElement = this.element;
-            this.addSubview(dropZoneView);
-        }
     }
 

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

@@ -204 +204 @@
 
         this._overviewContentView = new WI.GraphicsOverviewContentView;
-        this.contentBrowser.showContentView(this._overviewContentView);
+
+        // FIXME: <https://webkit.org/b/224650> (Web Inspector: audit for re-entrancy issues with `initialLayout` and `layout`)
+        setTimeout(() => {
+            this.contentBrowser.showContentView(this._overviewContentView);
+        });
 
         let dropZoneView = new WI.DropZoneView(this);