View.removeSubview not removing the element properly when not parented I addressed a review comment incompletely... I updated the assertion appropriately but not the code.
Created attachment 363193 [details] [PATCH] Proposed Fix Would only be possible right now debugging Worker Threads in CPU Timeline View. 1. Reload maps.google.com a few times with CPU timeline view
Comment on attachment 363193 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=363193&action=review > Source/WebInspectorUI/ChangeLog:10 > + Since the element may not be a direct child, just use Element.prototype.remove. Nice fix! I think we should add an assertion around View.js:110, before we try to insert the child view's DOM element: console.assert(!view.element.parentNode || this._element.contains(view.element.parentNode), "Subview DOM element must be a descendant of the parent view element."); This can balance the assertion at the top of View.removeSubview.
Comment on attachment 363193 [details] [PATCH] Proposed Fix r=me We should add a test to LayoutTests/inspector/view/basics.html: suite.addTestCase({ name: "View.removeSubview.IndirectDescendant", test() { let parent = new WI.View; let middleElement = parent.element.appendChild(document.createElement("div")); let child = new WI.View; middleElement.appendChild(child.element); parent.addSubview(child); parent.removeSubview(child); InspectorTest.expectFalse(child.element.parentNode, "Removed view should not be in the DOM."); return true; } });
Great suggestions, taking both!
https://trac.webkit.org/r242241
<rdar://problem/48494372>