Attachment 358179 Details for Bug 192648 – Patch

[patch] Patch

bug-192648-20190101222006.patch (text/plain), 15.40 KB, created by Devin Rousso on 2019-01-01 21:20:07 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-01-01 21:20:07 PST

Size: 15.40 KB

patch

obsolete

>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index d0d8c6c2cb99a6a96941aac00f7af2287933ae7c..0b1f4cec2efbc8ff4482b40645f82e5a7ee33766 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,35 @@
>+2019-01-01  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 NOBODY (OOPS!).
>+
>+        * 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):
>+        (WI.TreeElement.prototype.reveal):
>+        Move `updateVirtualizedElements` calls to the owner `WI.TreeOutline` to ensure that they get
>+        called. Make the remaining calls non-async, as `set hidden`, `collapse`, and `expand` are
>+        all effectively "immediate" events that should redraw as soon as possible.
>+
>+        * 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.
>+
> 2018-12-21  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: Styles Redesign: remove unused CSS style icons
>diff --git a/Source/WebInspectorUI/UserInterface/Views/ScopeChainDetailsSidebarPanel.js b/Source/WebInspectorUI/UserInterface/Views/ScopeChainDetailsSidebarPanel.js
>index bb4f0d63890670fb074ada0edbab505ba3ac15cb..226a2697e7e992b681163ace95c38ff983ac49e4 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/ScopeChainDetailsSidebarPanel.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/ScopeChainDetailsSidebarPanel.js
>@@ -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);
> 
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TreeElement.js b/Source/WebInspectorUI/UserInterface/Views/TreeElement.js
>index 7af88394cc1b906df54e00c09d29a12aab3b1c47..523858206608a335ca5482570a3f252812d8d927 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TreeElement.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TreeElement.js
>@@ -165,10 +165,7 @@ WI.TreeElement = class TreeElement extends WI.Object
>             this._childrenListNode.hidden = this._hidden;
> 
>         if (this.treeOutline) {
>-            let focusedTreeElement = null;
>-            if (!this._hidden && this.selected)
>-                focusedTreeElement = this;
>-            this.treeOutline.soon.updateVirtualizedElements(focusedTreeElement);
>+            this.treeOutline.updateVirtualizedElements({forceRefresh: true});
> 
>             this.treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementVisibilityDidChange, {element: this});
>         }
>@@ -261,9 +258,6 @@ WI.TreeElement = class TreeElement extends WI.Object
>                 this.parent._childrenListNode.insertBefore(this._childrenListNode, this._listItemNode.nextSibling);
>         }
> 
>-        if (this.treeOutline)
>-            this.treeOutline.soon.updateVirtualizedElements();
>-
>         if (this.selected)
>             this.select();
>         if (this.expanded)
>@@ -278,9 +272,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.soon.updateVirtualizedElements();
>     }
> 
>     static treeElementToggled(event)
>@@ -347,7 +338,7 @@ WI.TreeElement = class TreeElement extends WI.Object
>             this.oncollapse(this);
> 
>         if (this.treeOutline) {
>-            this.treeOutline.soon.updateVirtualizedElements(this);
>+            this.treeOutline.updateVirtualizedElements({forceRefresh: true});
> 
>             this.treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementDisclosureDidChanged, {element: this});
>         }
>@@ -414,7 +405,7 @@ WI.TreeElement = class TreeElement extends WI.Object
>             this.onexpand(this);
> 
>         if (this.treeOutline) {
>-            this.treeOutline.soon.updateVirtualizedElements(this);
>+            this.treeOutline.updateVirtualizedElements({forceRefresh: true});
> 
>             this.treeOutline.dispatchEventToListeners(WI.TreeOutline.Event.ElementDisclosureDidChanged, {element: this});
>         }
>@@ -467,7 +458,7 @@ WI.TreeElement = class TreeElement extends WI.Object
>         // This must be called before onreveal, as some subclasses will scrollIntoViewIfNeeded and
>         // we should update the visible elements before attempting to scroll.
>         if (this.treeOutline)
>-            this.treeOutline.updateVirtualizedElements(this);
>+            this.treeOutline.updateVirtualizedElements({focusedTreeElement: this});
> 
>         if (this.onreveal)
>             this.onreveal(this);
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>index 22840a1e435e375b2135ff01395f9be4b22ecbd8..fe314b698cdb2b4fc2699bfde9498457ae6eecb0 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>@@ -396,20 +396,20 @@ 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++;
>-
>-        let index = this._indexOfTreeElement(element);
>-        if (index >= 0) {
>-            console.assert(!element.selected, "TreeElement should not be selected before being inserted.");
>-            this._selectionController.didInsertItem(index);
>+            let index = this._indexOfTreeElement(element);
>+            if (index >= 0) {
>+                console.assert(!element.selected, "TreeElement should not be selected before being inserted.");
>+                this._selectionController.didInsertItem(index);
>+            }
>         }
>+
>+        if (this.virtualized && !this._scheduledUpdateVirtualizedElementsId)
>+            this._scheduledUpdateVirtualizedElementsId = requestAnimationFrame(() => this.updateVirtualizedElements());
>     }
> 
>     _forgetTreeElement(element)
>@@ -426,6 +426,9 @@ WI.TreeOutline = class TreeOutline extends WI.Object
>             this._knownTreeElements[element.identifier].remove(element, true);
>             this._cachedNumberOfDescendents--;
>         }
>+
>+        if (this.virtualized && !this._scheduledUpdateVirtualizedElementsId)
>+            this._scheduledUpdateVirtualizedElementsId = requestAnimationFrame(() => this.updateVirtualizedElements());
>     }
> 
>     _forgetChildrenRecursive(parentElement)
>@@ -681,7 +684,9 @@ WI.TreeOutline = class TreeOutline extends WI.Object
> 
>     registerScrollVirtualizer(scrollContainer, treeItemHeight)
>     {
>+        console.assert(scrollContainer);
>         console.assert(!isNaN(treeItemHeight));
>+        console.assert(!this.virtualized);
> 
>         this._virtualizedVisibleTreeElements = new Set;
>         this._virtualizedAttachedTreeElements = new Set;
>@@ -694,13 +699,34 @@ WI.TreeOutline = class TreeOutline extends WI.Object
>         this._virtualizedScrollContainer.addEventListener("scroll", (event) => {
>             throttler.updateVirtualizedElements();
>         });
>+
>+        const observerHandler = (entries) => {
>+            console.assert(entries.length === 1);
>+            if (entries[0].isIntersecting)
>+                this.updateVirtualizedElements({shouldRefresh: true});
>+        };
>+        const observerOptions = {
>+            root: this._virtualizedScrollContainer,
>+            threshold: [0],
>+        };
>+        let observer = new IntersectionObserver(observerHandler, observerOptions);
>+        observer.observe(this.element);
>+
>+        this.updateVirtualizedElements();
>     }
> 
>-    updateVirtualizedElements(focusedTreeElement)
>+    updateVirtualizedElements({focusedTreeElement, forceRefresh} = {})
>     {
>         if (!this.virtualized)
>             return;
> 
>+        if (this._scheduledUpdateVirtualizedElementsId) {
>+            cancelAnimationFrame(this._scheduledUpdateVirtualizedElementsId);
>+            this._scheduledUpdateVirtualizedElementsId = null;
>+        }
>+
>+        this.updateVirtualizedElements.cancelThrottle();
>+
>         function walk(parent, callback, count = 0) {
>             let shouldReturn = false;
>             for (let child of parent.children) {
>@@ -722,7 +748,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)) {
>@@ -750,19 +791,21 @@ 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.listItemElement.parentNode)
>                 treeElementsToDetach.add(treeElement);
> 
>             return false;
>         }).count;
> 
>-        // Redraw if we are about to scroll.
>-        if (!shouldScroll) {
>-            // Redraw if all of the previously centered `WI.TreeElement` are no longer centered.
>-            if (visibleTreeElements.intersects(this._virtualizedVisibleTreeElements)) {
>-                // Redraw if there is a `WI.TreeElement` that should be shown that isn't attached.
>-                if (visibleTreeElements.isSubsetOf(this._virtualizedAttachedTreeElements))
>-                    return;
>+        if (!forceRefresh) {
>+            // Redraw if we are about to scroll.
>+            if (!shouldScroll) {
>+                // Redraw if all of the previously centered `WI.TreeElement` are no longer centered.
>+                if (visibleTreeElements.intersects(this._virtualizedVisibleTreeElements)) {
>+                    // Redraw if there is a `WI.TreeElement` that should be shown that isn't attached.
>+                    if (visibleTreeElements.isSubsetOf(this._virtualizedAttachedTreeElements))
>+                        return;
>+                }
>             }
>         }
> 
>@@ -770,24 +813,24 @@ WI.TreeOutline = class TreeOutline extends WI.Object
>         this._virtualizedAttachedTreeElements = treeElementsToAttach;
> 
>         for (let treeElement of treeElementsToDetach)
>-            treeElement.element.remove();
>+            treeElement.listItemElement.remove();
> 
>         for (let treeElement of treeElementsToAttach) {
>-            treeElement.parent._childrenListNode.appendChild(treeElement.element);
>+            treeElement.parent._childrenListNode.appendChild(treeElement.listItemElement);
>             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);
>     }
> 
>     // SelectionController delegate
>@@ -972,20 +1015,6 @@ 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,
>-        };
>-    }
>-
>     _handleContextmenu(event)
>     {
>         let treeElement = this.treeElementFromEvent(event);

Actions: View | Formatted Diff | Diff

Attachments on bug 192648: 358179 | 364437 | 364485 | 365395