Bug 192090

<rdar://problem/46318614>

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

(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).

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]​

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.

Created attachment 357351 [details] Patch

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).

Created attachment 357365 [details] Patch for landing

Comment on attachment 357365 [details] Patch for landing Clearing flags on attachment: 357365 Committed r239246: <https://trac.webkit.org/changeset/239246>

All reviewed patches have been landed. Closing bug.