Bug 192090 – Web Inspector: REGRESSION(r238599): Uncaught Exception: TypeError: null is not an object (evaluating 'treeElement.listItemElement.classList')

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

-------
Inspected URL:        file:///Volumes/DATA/Build/Release/WebInspectorUI.framework/Resources/Main.html
Loading completed:    true
Frontend User Agent:  Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_4) AppleWebKit/605.1.15 (KHTML, like Gecko)

Uncaught Exceptions:
 - TypeError: null is not an object (evaluating 'treeElement.listItemElement.classList') (at TreeOutline.js:855:44)
    selectionControllerSelectionDidChange @ TreeOutline.js:855:44
    _updateSelectedItems @ SelectionController.js:391:61
    selectItem @ SelectionController.js:102:34
    selectedTreeElement @ TreeOutline.js:114:49
    selectTreeElementInternal @ TreeOutline.js:917:13
    select @ TreeElement.js:521:46
    refresh @ ThreadTreeElement.js:67:46
    onattach @ ThreadTreeElement.js:113:21
    _attach @ TreeElement.js:252:30
    appendChild @ TreeOutline.js:247:26
    _addTarget @ DebuggerSidebarPanel.js:741:47
    DebuggerSidebarPanel @ DebuggerSidebarPanel.js:215:28
    navigationSidebarPanel @ TabContentView.js:171:87
    showRepresentedObject @ ContentBrowserTabContentView.js:229:17
    showRepresentedObject @ Main.js:1221:41
    _treeSelectionDidChange @ ResourceSidebarPanel.js:480:37
    dispatch @ Object.js:170:30
    dispatchEventToListeners @ Object.js:177:17
    _dispatchSelectionDidChangeEvent @ TreeOutline.js:1101:38
    selectionControllerSelectionDidChange @ TreeOutline.js:863:46
    _updateSelectedItems @ SelectionController.js:391:61
    selectItem @ SelectionController.js:102:34
    handleItemMouseDown @ SelectionController.js:265:28
    _handleMouseDown @ TreeOutline.js:1037:54
    _handleMouseDown @ [native code]
-------

* STEPS TO REPRODUCE
1. open inspector^2
2. set a breakpoint somewhere that will be hit
3. wait for the breakpoint to be hit

[Radar WebKit Bug Importer]

<rdar://problem/46318614>

[Devin Rousso]

I'm now also seeing another error similar to this (likely from r238859 <https://webkit.org/b/192119>):

 - TypeError: null is not an object (evaluating 'this._previousSelectedTreeElement.listItemElement.classList')​ (at TreeOutline.js:​829:​66)​
    selectionControllerSelectionDidChange @ TreeOutline.js:​829:​66
    _updateSelectedItems @ SelectionController.js:​391:​61
    selectItem @ SelectionController.js:​102:​34
    selectedTreeElement @ TreeOutline.js:​114:​49
    selectTreeElementInternal @ TreeOutline.js:​888:​13
    select @ TreeElement.js:​518:​46
    refresh @ ThreadTreeElement.js:​67:​46
    onattach @ ThreadTreeElement.js:​113:​21
    _attach @ TreeElement.js:​251:​30
    appendChild @ TreeOutline.js:​249:​26
    _addTarget @ DebuggerSidebarPanel.js:​741:​47
    DebuggerSidebarPanel @ DebuggerSidebarPanel.js:​215:​28
    navigationSidebarPanel @ TabContentView.js:​171:​87
    shown @ ContentBrowserTabContentView.js:​101:​17
    prepareToShow @ BackForwardEntry.js:​84:​35
    _showEntry @ ContentViewContainer.js:​450:​28
    showBackForwardEntryForIndex @ ContentViewContainer.js:​166:​28
    showContentView @ ContentViewContainer.js:​142:​42
    _tabBarItemSelected @ TabBrowser.js:​243:​55
    dispatch @ Object.js:​170:​30
    dispatchEventToListeners @ Object.js:​177:​17
    selectedTabBarItem @ TabBar.js:​359:​38
    showTabForContentView @ TabBrowser.js:​186:​21
    showDebuggerTab @ Main.js:​1024:​42
    _debuggerDidPause @ Main.js:​1445:​25
    dispatch @ Object.js:​170:​30
    dispatchEventToListeners @ Object.js:​177:​17
    debuggerDidPause @ DebuggerManager.js:​674:​42
    paused @ DebuggerObserver.js:​68:​44
    dispatchEvent @ InspectorBackend.js:​334:​42
    _dispatchEvent @ Connection.js:​195:​32
    dispatch @ Connection.js:​72:​32
    dispatchMessageFromTarget @ TargetManager.js:​101:​35
    dispatchMessageFromTarget @ TargetObserver.js:​42:​51
    dispatchEvent @ InspectorBackend.js:​334:​42
    _dispatchEvent @ Connection.js:​195:​32
    dispatch @ Connection.js:​72:​32
    dispatch @ InspectorBackend.js:​172:​52
    dispatchNextQueuedMessageFromBackend @ MessageDispatcher.js:​42:​34

Additional Details:​
    cause --> An uncaught exception was thrown while handling event:​ Debugger.paused

[Devin Rousso]

(In reply to Devin Rousso from comment #0)
> * STEPS TO REPRODUCE
> 1. open inspector^2
> 2. set a breakpoint somewhere that will be hit
> 3. wait for the breakpoint to be hit
You might also have to change tabs in inspector^2, close, and reopen it (e.g. remove any DOM elements for any `WI.TreeElement`).

I'm guessing that this situation occurs when the `WI.TreeElement` gets selected before it has a chance to actually add itself to the DOM (e.g. it's been added to the `WI.TreeOutline` model object hierarchy with relationships to other `WI.TreeElement`s, but that hasn't been carried over into the DOM yet).

[Matt Baker]

Okay, I can reliably reproduce this without opening Inspector²:

Steps to Reproduce:
1. Goto daringfireball.net
2. Open Inspector > Debugger tab
3. Add breakpoint on FancyZoon.js:68
4. Select a different tab 
5. Close and reopen Inspector
6. Reload page
  => Debugger tab shown, content is blank, sidebar won't expand
7. Switch to a different tab and back
  => Error

TypeError:​ null is not an object (evaluating 'treeElement.listItemElement.classList')​ (at TreeOutline.js:​816:​44)​
    selectionControllerSelectionDidChange @ TreeOutline.js:​816:​44
    _updateSelectedItems @ SelectionController.js:​443:​61
    selectItem @ SelectionController.js:​106:​34
    selectedTreeElement @ TreeOutline.js:​114:​49
    selectTreeElementInternal @ TreeOutline.js:​889:​13
    select @ TreeElement.js:​518:​46
    refresh @ ThreadTreeElement.js:​67:​46
    onattach @ ThreadTreeElement.js:​113:​21
    _attach @ TreeElement.js:​251:​30
    appendChild @ TreeOutline.js:​249:​26
    _addTarget @ DebuggerSidebarPanel.js:​741:​47
    DebuggerSidebarPanel @ DebuggerSidebarPanel.js:​215:​28
    navigationSidebarPanel @ TabContentView.js:​171:​87
    shown @ ContentBrowserTabContentView.js:​101:​17
    prepareToShow @ BackForwardEntry.js:​84:​35
    _showEntry @ ContentViewContainer.js:​450:​28
    showBackForwardEntryForIndex @ ContentViewContainer.js:​166:​28
    showContentView @ ContentViewContainer.js:​142:​42
    _tabBarItemSelected @ TabBrowser.js:​243:​55
    dispatch @ Object.js:​170:​30
    dispatchEventToListeners @ Object.js:​177:​17
    selectedTabBarItem @ LegacyTabBar.js:​386:​38
    _handleMouseDown @ LegacyTabBar.js:​623:​13
    _handleMouseDown @ [native code]​

[Matt Baker]

Analysis of the above crash trace:

TypeError:​ null is not an object (evaluating 'treeElement.listItemElement.classList')​ (at TreeOutline.js:​816:​44)​
D)  selectionControllerSelectionDidChange @ TreeOutline.js:​816:​44
    _updateSelectedItems @ SelectionController.js:​443:​61
    selectItem @ SelectionController.js:​106:​34
    selectedTreeElement @ TreeOutline.js:​114:​49
    selectTreeElementInternal @ TreeOutline.js:​889:​13
C)  select @ TreeElement.js:​518:​46
B)  refresh @ ThreadTreeElement.js:​67:​46
A)  onattach @ ThreadTreeElement.js:​113:​21
    _attach @ TreeElement.js:​251:​30
    appendChild @ TreeOutline.js:​249:​26
    _addTarget @ DebuggerSidebarPanel.js:​741:​47
    DebuggerSidebarPanel @ DebuggerSidebarPanel.js:​215:​28
    ...

A) ThreadTreeElement is now attached (its `listItemElement` property is valid).
B) ThreadTreeElement creates and appends child CallFrameTreeElements. Since the ThreadTreeElement hasn't been expanded, its `_childrenListNode` is null and the children won't be attached.
C) The CallFrameTreeElement for the active call frame is selected by its parent.
D) TreeOutline accesses the  `listItemElement` property of the selected TreeElement, which is invalid (see B), leading to the exception.

Another crash with an identical cause can occur accessing `this._previousSelectedTreeElement.listItemElement`. I think the fix is simply to check that `listItemElement` exists before using it. The "selected" class will be added by TreeElement when it is eventually attached.

[Matt Baker]

Created attachment 357351 [details]
Patch

[Devin Rousso]

Comment on attachment 357351 [details]
Patch

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

rs=me

Perhaps this is a good opportunity to move `addClassName` and `removeClassName` from `WI.GeneralTreeElement` to `WI.TreeElement`.  It would effectively solve this problem.

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:805
> +            if (treeElement && treeElement.listItemElement) {

This would prevent `deselect` from being called if the element hasn't been created yet.  I think we only want to avoid adding a class to the element, not any other functionality.

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:815
> +            if (treeElement && treeElement.listItemElement) {

Ditto (>805).

[Matt Baker]

Created attachment 357365 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 357365 [details]
Patch for landing

Clearing flags on attachment: 357365

Committed r239246: <https://trac.webkit.org/changeset/239246>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.