Web Inspector: Debugger: virtualize the list of variables in the Scope sidebar
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Mar 2019 21:49:13 +0000 (21:49 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Mar 2019 21:49:13 +0000 (21:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192648
<rdar://problem/46800949>

Reviewed by Joseph Pecoraro.

Source/WebInspectorUI:

* UserInterface/Views/ScopeChainDetailsSidebarPanel.js:
(WI.ScopeChainDetailsSidebarPanel.prototype._generateCallFramesSection):

* UserInterface/Views/TreeElement.js:
(WI.TreeElement.prototype.set hidden):
(WI.TreeElement.prototype._attach):
(WI.TreeElement.prototype._detach):
(WI.TreeElement.prototype.collapse):
(WI.TreeElement.prototype.expand):
Move `updateVirtualizedElements` calls to the owner `WI.TreeOutline` to ensure that they get
called. Make the remaining calls use rAF debouncing to better coalesce updates.

* UserInterface/Views/TreeOutline.js:
(WI.TreeOutline.prototype._rememberTreeElement):
(WI.TreeOutline.prototype._forgetTreeElement):
(WI.TreeOutline.prototype.registerScrollVirtualizer):
(WI.TreeOutline.prototype._updateVirtualizedElements.calculateOffsetFromContainer): Added.
(WI.TreeOutline.prototype._updateVirtualizedElements):
(WI.TreeOutline.prototype._calculateVirtualizedValues): Deleted.
Calculate the `WI.TreeOutline`'s top offset within the scroll container so that it will only
update when it's within the visual area.

* UserInterface/Views/Utilities.js:
(Array.prototype.remove):
Return whether the item was actually removed from the array.

LayoutTests:

* inspector/unit-tests/array-utilities.html:
* inspector/unit-tests/array-utilities-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/inspector/unit-tests/array-utilities-expected.txt
LayoutTests/inspector/unit-tests/array-utilities.html
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/Utilities.js
Source/WebInspectorUI/UserInterface/Views/ScopeChainDetailsSidebarPanel.js
Source/WebInspectorUI/UserInterface/Views/TreeElement.js
Source/WebInspectorUI/UserInterface/Views/TreeOutline.js

index dcc3d13..9fabecf 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-20  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Debugger: virtualize the list of variables in the Scope sidebar
+        https://bugs.webkit.org/show_bug.cgi?id=192648
+        <rdar://problem/46800949>
+
+        Reviewed by Joseph Pecoraro.
+
+        * inspector/unit-tests/array-utilities.html:
+        * inspector/unit-tests/array-utilities-expected.txt:
+
 2019-03-20  Dean Jackson  <dino@apple.com>
 
         [iOS] Enable fast clicking everywhere
index e196911..f52c69c 100644 (file)
@@ -96,8 +96,14 @@ PASS: lastValue of an empty array should be undefined.
 [1,2,3,4] => [[1,2],[2,3],[3,4]]
 
 -- Running test case: Array.prototype.remove
+PASS: remove should return true when removing a value that exists.
 PASS: remove should only remove the first matching value.
+PASS: remove should return true when removing a value that exists.
+PASS: remove should return false when removing a value that does not exist.
 PASS: remove should only remove values that strictly match.
+PASS: remove should return false when removing a value that does not exist.
+PASS: remove should return false when removing a value that does not exist.
+PASS: remove should not affect the array if the value does not exist.
 
 -- Running test case: Array.prototype.removeAll
 PASS: removeAll should remove all matching values.
index 01d8de1..d15afec 100644 (file)
@@ -213,15 +213,18 @@ function test()
         name: "Array.prototype.remove",
         test() {
             let arr1 = [1, 2, 3, 1];
-            arr1.remove(1);
+            InspectorTest.expectThat(arr1.remove(1), "remove should return true when removing a value that exists.");
             InspectorTest.expectShallowEqual(arr1, [2, 3, 1], "remove should only remove the first matching value.");
 
             let arr2 = ["1", "2", 3, 1];
-            arr2.remove("1");
-            arr2.remove(2);
+            InspectorTest.expectThat(arr2.remove("1"), "remove should return true when removing a value that exists.");
+            InspectorTest.expectFalse(arr2.remove(2), "remove should return false when removing a value that does not exist.");
             InspectorTest.expectShallowEqual(arr2, ["2", 3, 1], "remove should only remove values that strictly match.");
 
-            return true;
+            let arr3 = [1, 2, 3];
+            InspectorTest.expectFalse(arr3.remove("1"), "remove should return false when removing a value that does not exist.");
+            InspectorTest.expectFalse(arr3.remove(4), "remove should return false when removing a value that does not exist.");
+            InspectorTest.expectShallowEqual(arr3, [1, 2, 3], "remove should not affect the array if the value does not exist.");
         }
     });
 
index 9ecbe62..128a594 100644 (file)
@@ -1,3 +1,37 @@
+2019-03-20  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Debugger: virtualize the list of variables in the Scope sidebar
+        https://bugs.webkit.org/show_bug.cgi?id=192648
+        <rdar://problem/46800949>
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Views/ScopeChainDetailsSidebarPanel.js:
+        (WI.ScopeChainDetailsSidebarPanel.prototype._generateCallFramesSection):
+
+        * UserInterface/Views/TreeElement.js:
+        (WI.TreeElement.prototype.set hidden):
+        (WI.TreeElement.prototype._attach):
+        (WI.TreeElement.prototype._detach):
+        (WI.TreeElement.prototype.collapse):
+        (WI.TreeElement.prototype.expand):
+        Move `updateVirtualizedElements` calls to the owner `WI.TreeOutline` to ensure that they get
+        called. Make the remaining calls use rAF debouncing to better coalesce updates.
+
+        * UserInterface/Views/TreeOutline.js:
+        (WI.TreeOutline.prototype._rememberTreeElement):
+        (WI.TreeOutline.prototype._forgetTreeElement):
+        (WI.TreeOutline.prototype.registerScrollVirtualizer):
+        (WI.TreeOutline.prototype._updateVirtualizedElements.calculateOffsetFromContainer): Added.
+        (WI.TreeOutline.prototype._updateVirtualizedElements):
+        (WI.TreeOutline.prototype._calculateVirtualizedValues): Deleted.
+        Calculate the `WI.TreeOutline`'s top offset within the scroll container so that it will only
+        update when it's within the visual area.
+
+        * UserInterface/Views/Utilities.js:
+        (Array.prototype.remove):
+        Return whether the item was actually removed from the array.
+
 2019-03-20  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Timelines - Export fails for MediaTimelineRecords with originator DOM Node - Cannot serialize cyclic structure
index 434b68a..174faab 100644 (file)
@@ -585,9 +585,10 @@ Object.defineProperty(Array.prototype, "remove",
         for (let i = 0; i < this.length; ++i) {
             if (this[i] === value) {
                 this.splice(i, 1);
-                return;
+                return true;
             }
         }
+        return false;
     }
 });
 
index bb4f0d6..226a269 100644 (file)
@@ -261,6 +261,7 @@ WI.ScopeChainDetailsSidebarPanel = class ScopeChainDetailsSidebarPanel extends W
                 }
 
                 let treeOutline = objectTree.treeOutline;
+                treeOutline.registerScrollVirtualizer(this.contentView.element, 16);
                 treeOutline.addEventListener(WI.TreeOutline.Event.ElementAdded, this._treeElementAdded.bind(this, detailsSectionIdentifier), this);
                 treeOutline.addEventListener(WI.TreeOutline.Event.ElementDisclosureDidChanged, this._treeElementDisclosureDidChange.bind(this, detailsSectionIdentifier), this);
 
index eff98e8..88a6c4d 100644 (file)
@@ -165,12 +165,8 @@ WI.TreeElement = class TreeElement extends WI.Object
             this._childrenListNode.hidden = this._hidden;
 
         if (this.treeOutline) {
-            if (this.treeOutline.virtualized) {
-                let focusedTreeElement = null;
-                if (!this._hidden && this.selected)
-                    focusedTreeElement = this;
-                this.treeOutline.updateVirtualizedElementsDebouncer.delayForTime(0, focusedTreeElement);
-            }
+            if (this.treeOutline.virtualized)
+                this.treeOutline.updateVirtualizedElementsDebouncer.delayForFrame();
 
             this.treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementVisibilityDidChange, {element: this});
         }
@@ -269,9 +265,6 @@ WI.TreeElement = class TreeElement extends WI.Object
                 this.parent._childrenListNode.insertBefore(this._childrenListNode, this._listItemNode.nextSibling);
         }
 
-        if (this.treeOutline && this.treeOutline.virtualized)
-            this.treeOutline.updateVirtualizedElementsDebouncer.delayForTime(0);
-
         if (this.selected)
             this.select();
         if (this.expanded)
@@ -286,9 +279,6 @@ WI.TreeElement = class TreeElement extends WI.Object
             this._listItemNode.parentNode.removeChild(this._listItemNode);
         if (this._childrenListNode && this._childrenListNode.parentNode)
             this._childrenListNode.parentNode.removeChild(this._childrenListNode);
-
-        if (this.treeOutline && this.treeOutline.virtualized)
-            this.treeOutline.updateVirtualizedElementsDebouncer.delayForTime(0);
     }
 
     static treeElementToggled(event)
@@ -354,8 +344,9 @@ WI.TreeElement = class TreeElement extends WI.Object
         if (this.oncollapse)
             this.oncollapse(this);
 
-        if (this.treeOutline && this.treeOutline.virtualized) {
-            this.treeOutline.updateVirtualizedElementsDebouncer.delayForTime(0, this);
+        if (this.treeOutline) {
+            if (this.treeOutline.virtualized)
+                this.treeOutline.updateVirtualizedElementsDebouncer.delayForFrame();
 
             this.treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementDisclosureDidChanged, {element: this});
         }
@@ -421,8 +412,9 @@ WI.TreeElement = class TreeElement extends WI.Object
         if (this.onexpand)
             this.onexpand(this);
 
-        if (this.treeOutline && this.treeOutline.virtualized) {
-            this.treeOutline.updateVirtualizedElementsDebouncer.delayForTime(0, this);
+        if (this.treeOutline) {
+            if (this.treeOutline.virtualized)
+                this.treeOutline.updateVirtualizedElementsDebouncer.delayForFrame();
 
             this.treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementDisclosureDidChanged, {element: this});
         }
index d91355a..ccc9115 100644 (file)
@@ -442,14 +442,14 @@ WI.TreeOutline = class TreeOutline extends WI.Object
         if (!this._knownTreeElements[element.identifier])
             this._knownTreeElements[element.identifier] = [];
 
-        // check if the element is already known
         var elements = this._knownTreeElements[element.identifier];
-        if (elements.indexOf(element) !== -1)
-            return;
+        if (!elements.includes(element)) {
+            elements.push(element);
+            this._cachedNumberOfDescendents++;
+        }
 
-        // add the element
-        elements.push(element);
-        this._cachedNumberOfDescendents++;
+        if (this.virtualized)
+            this._virtualizedDebouncer.delayForFrame();
     }
 
     _forgetTreeElement(element)
@@ -458,10 +458,14 @@ WI.TreeOutline = class TreeOutline extends WI.Object
             element.deselect(true);
             this.selectedTreeElement = null;
         }
+
         if (this._knownTreeElements[element.identifier]) {
-            this._knownTreeElements[element.identifier].remove(element, true);
-            this._cachedNumberOfDescendents--;
+            if (this._knownTreeElements[element.identifier].remove(element))
+                this._cachedNumberOfDescendents--;
         }
+
+        if (this.virtualized)
+            this._virtualizedDebouncer.delayForFrame();
     }
 
     _forgetChildrenRecursive(parentElement)
@@ -727,7 +731,9 @@ WI.TreeOutline = class TreeOutline extends WI.Object
 
     registerScrollVirtualizer(scrollContainer, treeItemHeight)
     {
+        console.assert(scrollContainer);
         console.assert(!isNaN(treeItemHeight));
+        console.assert(!this.virtualized);
 
         let boundUpdateVirtualizedElements = (focusedTreeElement) => {
             this._updateVirtualizedElements(focusedTreeElement);
@@ -745,6 +751,8 @@ WI.TreeOutline = class TreeOutline extends WI.Object
         this._virtualizedScrollContainer.addEventListener("scroll", (event) => {
             throttler.fire();
         });
+
+        this._updateVirtualizedElements();
     }
 
     get updateVirtualizedElementsDebouncer()
@@ -963,24 +971,12 @@ WI.TreeOutline = class TreeOutline extends WI.Object
         document.head.appendChild(WI.TreeOutline._styleElement);
     }
 
-    _calculateVirtualizedValues()
-    {
-        let numberVisible = Math.ceil(this._virtualizedScrollContainer.offsetHeight / this._virtualizedTreeItemHeight);
-        let extraRows = Math.max(numberVisible * 5, 50);
-        let firstItem = Math.floor(this._virtualizedScrollContainer.scrollTop / this._virtualizedTreeItemHeight) - extraRows;
-        let lastItem = firstItem + numberVisible + (extraRows * 2);
-        return {
-            numberVisible,
-            extraRows,
-            firstItem,
-            lastItem,
-        };
-    }
-
     _updateVirtualizedElements(focusedTreeElement)
     {
         console.assert(this.virtualized);
 
+        this._virtualizedDebouncer.cancel();
+
         function walk(parent, callback, count = 0) {
             let shouldReturn = false;
             for (let child of parent.children) {
@@ -1002,7 +998,22 @@ WI.TreeOutline = class TreeOutline extends WI.Object
             return {count, shouldReturn};
         }
 
-        let {numberVisible, extraRows, firstItem, lastItem} = this._calculateVirtualizedValues();
+        function calculateOffsetFromContainer(node, target) {
+            let top = 0;
+            while (node !== target) {
+                top += node.offsetTop;
+                node = node.offsetParent;
+                if (!node)
+                    return 0;
+            }
+            return top;
+        }
+
+        let offsetFromContainer = calculateOffsetFromContainer(this._virtualizedTopSpacer.parentNode ? this._virtualizedTopSpacer : this.element, this._virtualizedScrollContainer);
+        let numberVisible = Math.ceil(Math.max(0, this._virtualizedScrollContainer.offsetHeight - offsetFromContainer) / this._virtualizedTreeItemHeight);
+        let extraRows = Math.max(numberVisible * 5, 50);
+        let firstItem = Math.floor((this._virtualizedScrollContainer.scrollTop - offsetFromContainer) / this._virtualizedTreeItemHeight) - extraRows;
+        let lastItem = firstItem + numberVisible + (extraRows * 2);
 
         let shouldScroll = false;
         if (focusedTreeElement && focusedTreeElement.revealed(false)) {
@@ -1030,7 +1041,7 @@ WI.TreeOutline = class TreeOutline extends WI.Object
                 treeElementsToAttach.add(treeElement);
                 if (count >= firstItem + extraRows && count <= lastItem - extraRows)
                     visibleTreeElements.add(treeElement);
-            } else if (treeElement.element.parentNode)
+            } else if (treeElement._listItemNode.parentNode)
                 treeElementsToDetach.add(treeElement);
 
             return false;
@@ -1050,24 +1061,24 @@ WI.TreeOutline = class TreeOutline extends WI.Object
         this._virtualizedAttachedTreeElements = treeElementsToAttach;
 
         for (let treeElement of treeElementsToDetach)
-            treeElement.element.remove();
+            treeElement._listItemNode.remove();
 
         for (let treeElement of treeElementsToAttach) {
-            treeElement.parent._childrenListNode.appendChild(treeElement.element);
+            treeElement.parent._childrenListNode.appendChild(treeElement._listItemNode);
             if (treeElement._childrenListNode)
                 treeElement.parent._childrenListNode.appendChild(treeElement._childrenListNode);
         }
 
-        this._virtualizedTopSpacer.style.height = (Math.max(firstItem, 0) * this._virtualizedTreeItemHeight) + "px";
+        this._virtualizedTopSpacer.style.height = (Number.constrain(firstItem, 0, totalItems) * this._virtualizedTreeItemHeight) + "px";
         if (this.element.previousElementSibling !== this._virtualizedTopSpacer)
             this.element.parentNode.insertBefore(this._virtualizedTopSpacer, this.element);
 
-        this._virtualizedBottomSpacer.style.height = (Math.max(totalItems - lastItem, 0) * this._virtualizedTreeItemHeight) + "px";
+        this._virtualizedBottomSpacer.style.height = (Number.constrain(totalItems - lastItem, 0, totalItems) * this._virtualizedTreeItemHeight) + "px";
         if (this.element.nextElementSibling !== this._virtualizedBottomSpacer)
             this.element.parentNode.insertBefore(this._virtualizedBottomSpacer, this.element.nextElementSibling);
 
         if (shouldScroll)
-            this._virtualizedScrollContainer.scrollTop = (firstItem + extraRows) * this._virtualizedTreeItemHeight;
+            this._virtualizedScrollContainer.scrollTop = offsetFromContainer + ((firstItem + extraRows) * this._virtualizedTreeItemHeight);
     }
 
     _handleContextmenu(event)