Bug 159696 - Uncaught Exception: TypeError: null is not an object (evaluating 'this._contentViewContainer.currentContentView.showsFilterBar')
Summary: Uncaught Exception: TypeError: null is not an object (evaluating 'this._conte...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Matt Baker
URL: http://daringfireball.net/
Keywords: InRadar
Depends on:
Reported: 2016-07-12 16:34 PDT by Joseph Pecoraro
Modified: 2016-07-13 16:34 PDT (History)
7 users (show)

See Also:

[Patch] Proposed Fix (1.85 KB, patch)
2016-07-13 15:29 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
[Patch] For Landing (1.77 KB, patch)
2016-07-13 15:37 PDT, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-07-12 16:34:45 PDT
Uncaught exception closing Timeline Tab while viewing Script & Events Timeline.

Steps to Reproduce:
1. Inspect this page
2. Show timeline tab
3. Enable Scripts + Events timeline
4. Reload => populate timeline data
5. Select Script + Events timeline
6. Close Timeline Tab
  => Uncaught exception

Uncaught Exceptions:
 - TypeError: null is not an object (evaluating 'this._contentViewContainer.currentContentView.showsFilterBar') (at ScriptClusterTimelineView.js:65:80)
    showsFilterBar @ ScriptClusterTimelineView.js:65:80
    _updateFilterBar @ TimelineRecordingContentView.js:800:102
    _contentViewSelectionPathComponentDidChange @ TimelineRecordingContentView.js:325:30
    dispatch @ Object.js:162:30
    dispatchEventToListeners @ Object.js:177:21
    _currentContentViewDidChange @ ClusterContentView.js:225:38
    dispatch @ Object.js:162:30
    dispatchEventToListeners @ Object.js:169:17
    closeAllContentViews @ ContentViewContainer.js:345:38
    closed @ ClusterContentView.js:75:56
    _disassociateFromContentView @ ContentViewContainer.js:476:27
    closeAllContentViews @ ContentViewContainer.js:339:46
    closed @ TimelineRecordingContentView.js:198:79
    _disassociateFromContentView @ ContentViewContainer.js:476:27
    closeAllContentViews @ ContentViewContainer.js:339:46
    closed @ ContentBrowserTabContentView.js:121:71
    closed @ TimelineTabContentView.js:298:21
    _disassociateFromContentView @ ContentViewContainer.js:476:27
    closeContentView @ ContentViewContainer.js:309:46
    _tabBarItemRemoved @ TabBrowser.js:238:52
    dispatch @ Object.js:162:30
    dispatchEventToListeners @ Object.js:169:17
    removeTabBarItem @ TabBar.js:298:38
    _handleClick @ TabBar.js:607:34
    _handleClick @ [native code]
Comment 1 Radar WebKit Bug Importer 2016-07-12 16:35:06 PDT
Comment 2 Matt Baker 2016-07-13 13:41:41 PDT
By the time ScriptClusterTimelineView showFilterBar is called, currentContentView has been destroyed. Simply adding a null check fixes this, but just kicks the can down the road:

[Error] Assertion Failed: Record selected in hidden overview graph – null
	selectRecord (TimelineOverview.js:399)
	_contentViewSelectionPathComponentDidChange (TimelineRecordingContentView.js:338)
	dispatch (Object.js:162)
	dispatchEventToListeners (Object.js:177)
	_currentContentViewDidChange (ClusterContentView.js:225)
	dispatch (Object.js:162)
	dispatchEventToListeners (Object.js:169)
	closeAllContentViews (ContentViewContainer.js:345)
	closed (ClusterContentView.js:75)
	_disassociateFromContentView (ContentViewContainer.js:476)
	closeAllContentViews (ContentViewContainer.js:339)
	closed (TimelineRecordingContentView.js:198)
	_disassociateFromContentView (ContentViewContainer.js:476)
	closeAllContentViews (ContentViewContainer.js:339)
	closed (ContentBrowserTabContentView.js:121)
	closed (TimelineTabContentView.js:298)
	_disassociateFromContentView (ContentViewContainer.js:476)
	closeContentView (ContentViewContainer.js:309)
	_tabBarItemRemoved (TabBrowser.js:238)
	dispatch (Object.js:162)
	dispatchEventToListeners (Object.js:169)
	removeTabBarItem (TabBar.js:234)
	_handleClick (TabBar.js:607)

Working on a solution which addresses both.
Comment 3 Joseph Pecoraro 2016-07-13 13:42:42 PDT
Well, probably _all_ of these forwarding messages should be resilient.
Comment 4 Matt Baker 2016-07-13 15:17:40 PDT
(In reply to comment #3)
> Well, probably _all_ of these forwarding messages should be resilient.

This is a separate but related issue. Both problems stem from TimelineRecordingContentView handling SelectedPathComponentsDidChange, which is emitted by ContentViewContainer.closeAllContentViews().

If ContentView had a "closed" property we could early return from the SelectedPathComponentsDidChange handle. Luckily we can use the "visible" property in the same way. Skipping these updates when the tab is hidden should be safe, since TimelineRecordingContentView.shown() triggers a CurrentContentViewDidChange -> SelectedPathComponentsDidChange.
Comment 5 Matt Baker 2016-07-13 15:29:49 PDT
Created attachment 283579 [details]
[Patch] Proposed Fix
Comment 6 Joseph Pecoraro 2016-07-13 15:33:22 PDT
Comment on attachment 283579 [details]
[Patch] Proposed Fix

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


> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:322
> -        if (event.target !== this._timelineContentBrowser.currentContentView)
> +        if (!this.visible || event.target !== this._timelineContentBrowser.currentContentView)

Style: This would read better as a separate if + bail. These are two very different conditions.
Comment 7 Matt Baker 2016-07-13 15:37:38 PDT
Created attachment 283581 [details]
[Patch] For Landing
Comment 8 WebKit Commit Bot 2016-07-13 16:34:27 PDT
Comment on attachment 283581 [details]
[Patch] For Landing

Clearing flags on attachment: 283581

Committed r203198: <http://trac.webkit.org/changeset/203198>
Comment 9 WebKit Commit Bot 2016-07-13 16:34:31 PDT
All reviewed patches have been landed.  Closing bug.