Bug 25751 - REGRESSION: Gmail popups cause an assertion failure and crash
Summary: REGRESSION: Gmail popups cause an assertion failure and crash
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Nobody
URL:
Keywords: InRadar, NeedsReduction, Regression
Depends on:
Blocks:
 
Reported: 2009-05-13 10:10 PDT by Brett Wilson (Google)
Modified: 2009-06-26 11:27 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brett Wilson (Google) 2009-05-13 10:10:39 PDT
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.
Comment 1 Brett Wilson (Google) 2009-05-13 10:23:47 PDT
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 ?? ()
Comment 2 Brady Eidson 2009-05-13 10:32:12 PDT
<rdar://problem/6883877>
Comment 3 Brady Eidson 2009-05-13 12:33:31 PDT
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.
Comment 4 Brady Eidson 2009-05-13 12:35:42 PDT
"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.
Comment 5 Brett Wilson (Google) 2009-05-13 12:39:12 PDT
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
Comment 6 Brady Eidson 2009-05-13 12:42:15 PDT
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?
Comment 7 Brett Wilson (Google) 2009-05-13 12:44:25 PDT
Ah, sorry I missed that. There's no other crash except inside the assertions.
Comment 8 Dimitri Glazkov (Google) 2009-06-01 12:24:55 PDT
Corresponding Chromium bug is http://crbug.com/12908
Comment 9 Ojan Vafai 2009-06-26 11:27:32 PDT
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?