Bug 162642 – Web Inspector: TimelineDataGridNode assertions when refreshing page

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

Summary:
TimelineDataGridNode assertions when refreshing page.

NetworkSidebarPanel is marking NetworkGridContentView visible when the network timeline resets. Once marked visible, the view will try to create data grid nodes for incoming timeline records, causing layouts and causing a flood of assertions.

Note:
ElementsTabContentView also attempts to show a DOM tree content view when the main frame navigates.

Steps to Reproduce:
1. Open Inspector
2. Make active tab something other than Network
3. Reload inspected page
  => Inspector² shows hundreds of asserts:
[Error] Assertion Failed
	refreshGraph (TimelineDataGridNode.js:230)
	refreshGraph

and:
[Error] Assertion Failed
	refresh (TimelineRecordBar.js:238)
	createBar (TimelineDataGridNode.js:243)
	createBar
	createCombinedBars (TimelineRecordBar.js:87)
	refreshGraph (TimelineDataGridNode.js:284)
	refreshGraph

[Radar WebKit Bug Importer]

<rdar://problem/28505898>

[Matt Baker]

Created attachment 290447 [details]
Patch

[Timothy Hatcher]

Comment on attachment 290447 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:15
> +        * UserInterface/Views/ElementsTabContentView.js:
> +        (WebInspector.ElementsTabContentView):
> +        (WebInspector.ElementsTabContentView.prototype.shown):
> +        Drive-by fix: defer showing the DOM content view until the tab is shown.

I worry this might have some side effects. The view might need to be created eagerly to get event listeners set up early. Can you look into this more or split it out for further consideration?

> Source/WebInspectorUI/UserInterface/Views/NetworkGridContentView.js:145
> +        this._dataGrid.updateLayout(WebInspector.View.LayoutReason.Resize);

Is this needed because the DataGrid isn't added as a subview?

[Matt Baker]

Comment on attachment 290447 [details]
Patch

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

>> Source/WebInspectorUI/ChangeLog:15
>> +        Drive-by fix: defer showing the DOM content view until the tab is shown.
> 
> I worry this might have some side effects. The view might need to be created eagerly to get event listeners set up early. Can you look into this more or split it out for further consideration?

FrameDOMTreeContentView will be lazily created, and so will its DOMTree.Event.RootDOMNodeInvalidated listener. This is fine because the handler for the listener just requests a new DOM root node, which is always done when the content view is constructed anyway.

>> Source/WebInspectorUI/UserInterface/Views/NetworkGridContentView.js:145
>> +        this._dataGrid.updateLayout(WebInspector.View.LayoutReason.Resize);
> 
> Is this needed because the DataGrid isn't added as a subview?

The grid is added as a subview, this is needed because View doesn't have a shown/hidden concept.

[Matt Baker]

Created attachment 295184 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 295184 [details]
Patch for landing

Clearing flags on attachment: 295184

Committed r208895: <http://trac.webkit.org/changeset/208895>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.