On pages with lots of variables per-scope (e.g. most minified scripts), WebInspector can slow down significantly as we try to create/render a `WI.ObjectTreeView` per-variable.
<rdar://problem/46800949>
Created attachment 358179 [details] Patch
Comment on attachment 358179 [details] Patch Attachment 358179 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/10603406 New failing tests: imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html
Comment on attachment 358179 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358179&action=review > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:713 > + 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); What is this doing that the scroll handler is not doing? Is it supposed to be handling the first display of the element? I did notice this assert firing quite a bit in local testing. console.assert(entries.length === 1); I also noticed this handler firing a lot during scrolling, which seems like it would maybe eliminate the need for the throttler? I also noticed an IntersectionObserver crash, which I've filed appropriately.
Comment on attachment 358179 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358179&action=review >> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:713 >> + observer.observe(this.element); > > What is this doing that the scroll handler is not doing? Is it supposed to be handling the first display of the element? > > I did notice this assert firing quite a bit in local testing. > > console.assert(entries.length === 1); > > I also noticed this handler firing a lot during scrolling, which seems like it would maybe eliminate the need for the throttler? > > I also noticed an IntersectionObserver crash, which I've filed appropriately. Yeah, this is supposed to handle the case where you start out with it being visible, but I think the `updateVirtualizedElements` call below handles that just as well. I'll remove this unless there is some other case I'm forgetting (or didn't know about) that needs this. As far as the assertions, my guess is that in the process of adding/resizing the top/bottom spacers, we cause the element to shift suddenly. I'll investigate a bit more.
Comment on attachment 358179 [details] Patch r- for now given the assertions I was seeing imply something amiss. If you can't reproduce let me know.
Created attachment 364437 [details] Patch
Comment on attachment 364437 [details] Patch Attachment 364437 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11482702 New failing tests: http/tests/cache/memory-cache-pruning.html
Created attachment 364485 [details] Archive of layout-test-results from ews201 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment on attachment 364437 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364437&action=review r=me > Source/WebInspectorUI/UserInterface/Base/Utilities.js:588 > + return true; Nice. Should update the existing Array.prototype.remove tests in: LayoutTests/inspector/unit-tests/array-utilities.html > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:452 > + if (this.virtualized) > + this._virtualizedDebouncer.delayForFrame(); Previously we would only delayForFrame if the element was not known. Do we want to do this always now (probably fine).
Comment on attachment 364437 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364437&action=review >> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:452 >> + this._virtualizedDebouncer.delayForFrame(); > > Previously we would only delayForFrame if the element was not known. Do we want to do this always now (probably fine). I don't think there's a difference. It should be an error for us to re-add the same element twice without removing it in between. Always doing it also gives us an "assurance" that each time we try to add an element, we should try to show it.
Created attachment 365395 [details] Patch
Comment on attachment 365395 [details] Patch Clearing flags on attachment: 365395 Committed r243242: <https://trac.webkit.org/changeset/243242>
All reviewed patches have been landed. Closing bug.