Bug 46261 – Web Inspector: CRASH at node highlight on MAC Safari

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Ilya Tikhonovsky]

1) run-safari --debug
2) open inspector
3) open elements panel
4) hover mouse over elements panel items
5) try to switch to another tab. In my case Timeline
6) CRASH

call stack

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000328
0x000000010147b546 in WTF::RefPtr<WebCore::Node>::operator! (this=0x328) at RefPtr.h:68
68	        bool operator!() const { return !m_ptr; }
(gdb) BT
#0  0x000000010147b546 in WTF::RefPtr<WebCore::Node>::operator! (this=0x328) at RefPtr.h:68
#1  0x00000001019ea1f9 in WebCore::InspectorController::drawNodeHighlight (this=0x0, context=@0x7fff5fbfd2f0) at /Users/Shared/loislo/Projects/chromium/src/third_party/WebKit/WebCore/inspector/InspectorController.cpp:1802
#2  0x0000000100f840e0 in -[WebNodeHighlightView drawRect:] (self=0x11e479100, _cmd=0x7fff874a0e08, rect={origin = {x = 0, y = 0}, size = {width = 1371, height = 784}}) at WebNodeHighlightView.mm:75
#3  0x00007fff86e65081 in -[NSView _drawRect:clip:] ()
#4  0x00007fff86e63cf4 in -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] ()
#5  0x00007fff86e6405e in -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] ()
#6  0x00007fff86e623c6 in -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#7  0x00007fff86f80b84 in -[NSNextStepFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] ()
#8  0x00007fff86e5e79a in -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] ()
#9  0x00007fff86dd7ff6 in -[NSView displayIfNeeded] ()
#10 0x00007fff86d8d7e4 in -[NSNextStepFrame displayIfNeeded] ()
#11 0x00007fff86dd2ea2 in _handleWindowNeedsDisplay ()
#12 0x00007fff811aea2d in __NSFireTimer ()
#13 0x00007fff85318678 in __CFRunLoopRun ()
#14 0x00007fff8531684f in CFRunLoopRunSpecific ()
#15 0x00007fff881c991a in RunCurrentEventLoopInMode ()
#16 0x00007fff881c967d in ReceiveNextEventCommon ()
#17 0x00007fff881c95d8 in BlockUntilNextEventMatchingListInMode ()
#18 0x00007fff86da829e in _DPSNextEvent ()
#19 0x00007fff86da7bed in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#20 0x00000001000165d8 in ?? ()
#21 0x00007fff86d6d8d3 in -[NSApplication run] ()
#22 0x00007fff86d665f8 in NSApplicationMain ()
#23 0x000000010000a4a4 in ?? ()
Current language:  auto; currently c++
(gdb) ГЗ
Undefined command: "".  Try "help".
(gdb) up
#1  0x00000001019ea1f9 in WebCore::InspectorController::drawNodeHighlight (this=0x0, context=@0x7fff5fbfd2f0) at /Users/Shared/loislo/Projects/chromium/src/third_party/WebKit/WebCore/inspector/InspectorController.cpp:1802
1802	    if (!m_highlightedNode)
(gdb) list
1797	    return mainFramePoint - IntPoint();
1798	}
1799	
1800	void InspectorController::drawNodeHighlight(GraphicsContext& context) const
1801	{
1802	    if (!m_highlightedNode)
1803	        return;
1804	
1805	    RenderObject* renderer = m_highlightedNode->renderer();
1806	    Frame* containingFrame = m_highlightedNode->document()->frame();
(gdb) p this
$1 = (const 'WebCore::InspectorController' * const) 0x0
(gdb) up
#2  0x0000000100f840e0 in -[WebNodeHighlightView drawRect:] (self=0x11e479100, _cmd=0x7fff874a0e08, rect={origin = {x = 0, y = 0}, size = {width = 1371, height = 784}}) at WebNodeHighlightView.mm:75
75	    [_webNodeHighlight inspectorController]->drawNodeHighlight(context);
Current language:  auto; currently objective-c++
(gdb)

[Ilya Tikhonovsky]

Created attachment 68663 [details]
[patch] initial version.

[Pavel Feldman]

Comment on attachment 68663 [details]
[patch] initial version.

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

> WebKit/mac/WebInspector/WebNodeHighlightView.mm:77
> +        [NSGraphicsContext restoreGraphicsState];

Should restore unconditionally.

[Ilya Tikhonovsky]

Created attachment 68666 [details]
[patch] initial version.

[WebKit Commit Bot]

Comment on attachment 68666 [details]
[patch] initial version.

Clearing flags on attachment: 68666

Committed r68247: <http://trac.webkit.org/changeset/68247>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 68666 [details]
[patch] initial version.

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

I know this landed. Here are some things to think about to improve later patches.

> WebKit/mac/ChangeLog:14
> +        Web Inspector: CRASH at node highlight on MAC Safari.
> +        1) run-safari --debug
> +        2) open inspector
> +        3) open elements panel
> +        4) hover mouse over elements panel items multiple times
> +        5) CRASH
> +        Looks like it is a race condition. WebNodeHighlightView doesn't check
> +        the pointer to WebNodeHighligh object and it can be nil.
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=46261

This still isn't the usual ChangeLog style, but I guess everyones does it differently
so I'll stop bringing it up. Just noticed there was a typo, "WebNodeHighligh".


> WebKit/mac/WebInspector/WebNodeHighlightView.mm:71
> +    if (_webNodeHighlight) {
> +        [NSGraphicsContext saveGraphicsState];

I think an early return would have made this easier to read, and is a common style.