Canvas Recording loading goes significantly slower when "Frame" tree element is expanded Steps to Reproduce: 1. Inspect <https://devinrousso.com/demo/DoubleHelixCircle.html> 2. Make Inspector Window very large 3. Capture a single frame recording => Notice frame loading is going smoothly 4. Expand "Frame 1" tree element => Loading goes significantly slower
Created attachment 352146 [details] Patch
Created attachment 352147 [details] Patch Remove the previous system of remembering the index in favor of thottling
Created attachment 352148 [details] Patch
Created attachment 352229 [details] Patch
Comment on attachment 352229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352229&action=review > Source/WebInspectorUI/UserInterface/Base/Utilities.js:142 > +Object.defineProperty(Set.prototype, "contains", > +{ > + value(other) > + { > + if (!this.size || !other.size) > + return false; Nit: Should we name this `containsSet` or invert it and name `isSubsetOf`? Right now `contains` sounds just like `has` given the API name in other places... Shouldn't a non-empty Set contain the empty Set? let s = new Set([1]); let x = new Set; s.contains(x); // expect true In which case we would just drop this early return. > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:737 > + return; This early return seems super sketchy. It looks ike the spacers are being resized below, which I would expect might need to always happen. > LayoutTests/inspector/unit-tests/set-utilities.html:25 > + testFalse([], [], "empy sets should not intersect."); Typo: "empy" => "empty" > LayoutTests/inspector/unit-tests/set-utilities.html:49 > + testFalse([], [], "an empy sets should not contain another empty set."); Typo: "empy" => "empty" > LayoutTests/inspector/unit-tests/set-utilities.html:53 > + I'd expect a non-empty set to contain the empty set. Need a test for that since currently the results doesn't match my expectations.
Comment on attachment 352229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352229&action=review >> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:737 >> + return; > > This early return seems super sketchy. It looks ike the spacers are being resized below, which I would expect might need to always happen. We don't want to update the spacers unless we are moving DOM nodes around. If we aren't adding/removing DOM nodes, regular scrolling works as expected. The only time we want to update spacers is when we are trying to add new elements that are in a different scroll position than what was shown before.
Created attachment 352529 [details] Patch
Comment on attachment 352529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352529&action=review r=me > Source/WebInspectorUI/UserInterface/Base/Utilities.js:134 > + }, Let's remove the trailing comma on these because we don't ever expect any other properties. > Source/WebInspectorUI/UserInterface/Base/Utilities.js:147 > + }, Let's remove the trailing comma on these because we don't ever expect any other properties.
Created attachment 353126 [details] Patch
Comment on attachment 353126 [details] Patch Clearing flags on attachment: 353126 Committed r237436: <https://trac.webkit.org/changeset/237436>
All reviewed patches have been landed. Closing bug.
<rdar://problem/45572257>