Bug 234572 – Web Inspector: Assertion Failed removing subview in ContentViewContainer.prototype._disassociateFromContentView

• Component: Web Inspector
• Status: RESOLVED FIXED

[Patrick Angle]

[Error] Assertion Failed: Subview DOM element must be a child of the parent view element.
	removeSubview (View.js:121)
	_disassociateFromContentView (ContentViewContainer.js:428)
	replaceContentView (ContentViewContainer.js:199)
	showContentView (ContentViewContainer.js:85)
	_showDOMTreeContentViewIfNeeded (ElementsTabContentView.js:144)
	_mainFrameDidChange (ElementsTabContentView.js:149)
	dispatch (Object.js:134)
	dispatchEventToListeners (Object.js:142)
	_mainFrameDidChange (NetworkManager.js:1384)
	_processMainFrameResourceTreePayload (NetworkManager.js:1240)
	_processMainFrameResourceTreePayload
	_dispatchResponseToCallback (Connection.js:152)
	_dispatchResponse (Connection.js:122)
	dispatch (Connection.js:77)
	dispatchProvisionalMessages (Connection.js:68)
	didCommitProvisionalTarget (TargetManager.js:151)
	didCommitProvisionalTarget (TargetObserver.js:37)
	_dispatchEvent (Connection.js:210)
	dispatch (Connection.js:79)
	dispatch (InspectorBackend.js:233)
	(anonymous function) (MessageDispatcher.js:42)

https://trac.webkit.org/changeset/283859/webkit removed the `isAttached` check for the content view by accident when refactoring this method to work with extension tabs, which should not be removed from the DOM, but it is not actually guaranteed that the content view is attached.

[Radar WebKit Bug Importer]

<rdar://problem/86778405>

[Patrick Angle]

Created attachment 447741 [details]
Patch v1.0

[Devin Rousso]

Comment on attachment 447741 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=447741&action=review

r=me

> Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:429
> +            this.removeSubview(contentView);

So this call was added in r283728 and was originally guarded by `contentView.constructor.shouldNotRemoveFromDOMWhenHidden()`.  I'm not really sure if we want this to be something that _always_ happens outside of that check (especially since almost all callers of this will call `_hideEntry` right before, which also does `removeSubview`).  Maybe we should restructure this so that it's all underneath `contentView.constructor.shouldNotRemoveFromDOMWhenHidden()`?
```
// Hidden/non-visible extension tabs must remain attached to the DOM to avoid reloading.
if (contentView.constructor.shouldNotRemoveFromDOMWhenHidden()) {
    if (!contentView.visible)
        return;

    if (contentView.attached)
        this.removeSubview(contentView);
}
```

[Patrick Angle]

Created attachment 447743 [details]
Patch v1.1

[EWS]

Committed r287346 (245488@main): <https://commits.webkit.org/245488@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447743 [details].