Bug 25751 – REGRESSION: Gmail popups cause an assertion failure and crash

• Status: RESOLVED
• Resolution: WORKSFORME
• Component: Layout and Rendering

[Brett Wilson (Google)]

I reproed this in my own debug build of WebKit on Mac inside Safari. I originally noticed this in Chromium about a week ago.

Go to Gmail in a debug build. Click on "Move To" or "Labels." You will get an assertion failure. "More actions" doesn't have the problem, presumably because it's more simple (the other two have a scrolling element and an input).

Stack:

WebCore::RenderTextControl::setSelectionRange(int start=0, int end=0)  Line 251

WebCore::RenderTextControl::select()  Line 230

WebCore::HTMLInputElement::select()  Line 507

WebCore::InputElement::updateFocusAppearance(WebCore::InputElementData & data={...}, WebCore::Document * document=0x04105000, bool restorePreviousSelection=true)  Line 103

WebCore::HTMLInputElement::updateFocusAppearance(bool restorePreviousSelection=true)  Line 165

WebCore::Element::focus(bool restorePreviousSelection=true)  Line 1188

WebCore::ElementInternal::focusCallback(const v8::Arguments & args={...})  Line 353


If I step past this assertion, I get a null pointer dereference and it crashes.

[Brett Wilson (Google)]

Satck on Mac in Safari (custom trunk build):

#0  0x03ba2c11 in WebCore::RenderTextControl::setSelectionRange (this=0x20518f3c, start=0, end=0) at /Users/brettw/wk/WebCore/rendering/RenderTextControl.cpp:251
#1  0x03ba2e35 in WebCore::RenderTextControl::select (this=0x20518f3c) at /Users/brettw/wk/WebCore/rendering/RenderTextControl.cpp:230
#2  0x037e8ba8 in WebCore::HTMLInputElement::select (this=0x201c1470) at /Users/brettw/wk/WebCore/html/HTMLInputElement.cpp:506
#3  0x03878e04 in WebCore::InputElement::updateFocusAppearance (data=@0x201c14b8, document=0x75ef400, restorePreviousSelection=true) at /Users/brettw/wk/WebCore/dom/InputElement.cpp:103
#4  0x037e8f9e in WebCore::HTMLInputElement::updateFocusAppearance (this=0x201c1470, restorePreviousSelection=true) at /Users/brettw/wk/WebCore/html/HTMLInputElement.cpp:165
#5  0x0372a563 in WebCore::Element::focus (this=0x201c1470, restorePreviousSelection=true) at /Users/brettw/wk/WebCore/dom/Element.cpp:1158
#6  0x038fd581 in WebCore::jsElementPrototypeFunctionFocus (exec=0x1be5473c, thisValue={m_ptr = 0x203b3160}, args=@0xbfffd828) at /Users/brettw/wk/WebKitBuild/Debug/DerivedSources/WebCore/JSElement.cpp:564
#7  0x1bdf320f in ?? ()

[Brady Eidson]

<rdar://problem/6883877>

[Brady Eidson]

The assertion has been around forever.  Running a Debug build of WebKit from *last* May, the ASSERT still fires in this case.

However, I'm not seeing the case where after the ASSERT there's a crash.  It seems that this code handles null startPosition or endPosition.

[Brady Eidson]

"If I step past this assertion, I get a null pointer dereference and it crashes."

Can you give more details on this?  I cannot reproduce.

[Brett Wilson (Google)]

This is the stack to the NULL pointer dereference. I commented out the assert and ran this in Safari on Mac.

Program received signal:  “EXC_BAD_ACCESS”.
(gdb) bt
#0  0x03a94625 in WebCore::Node::shadowAncestorNode (this=0x0) at /Users/brettw/wk/WebCore/dom/Node.cpp:1340
#1  0x03ba2c99 in WebCore::RenderTextControl::setSelectionRange (this=0x1d3d0ebc, start=0, end=0) at /Users/brettw/wk/WebCore/rendering/RenderTextControl.cpp:252
#2  0x03ba2e7b in WebCore::RenderTextControl::select (this=0x1d3d0ebc) at /Users/brettw/wk/WebCore/rendering/RenderTextControl.cpp:230
#3  0x037e8c68 in WebCore::HTMLInputElement::select (this=0x1e1e9790) at /Users/brettw/wk/WebCore/html/HTMLInputElement.cpp:506
#4  0x03878ec4 in WebCore::InputElement::updateFocusAppearance (data=@0x1e1e97d8, document=0x71db400, restorePreviousSelection=true) at /Users/brettw/wk/WebCore/dom/InputElement.cpp:103
#5  0x037e905e in WebCore::HTMLInputElement::updateFocusAppearance (this=0x1e1e9790, restorePreviousSelection=true) at /Users/brettw/wk/WebCore/html/HTMLInputElement.cpp:165
#6  0x0372a623 in WebCore::Element::focus (this=0x1e1e9790, restorePreviousSelection=true) at /Users/brettw/wk/WebCore/dom/Element.cpp:1158

[Brady Eidson]

That null deref is simply in the next assertion on the next line, and those two assertions are logically connected.  I was under the impression you found a null deref downstream unrelated to these assertions - have you?

[Brett Wilson (Google)]

Ah, sorry I missed that. There's no other crash except inside the assertions.

[Dimitri Glazkov (Google)]

Corresponding Chromium bug is http://crbug.com/12908

[Ojan Vafai]

We no longer hit an assert in a ToT build. I don't know if the problem was fixed or if gmail changed. In either case, we can no longer reproduce this. Is worksforme the right resolution?