Bug 107261

Summary:

Crash in AccessibilityTableCell::parentTable()

Product:

WebKit

Reporter:

Sergio Villar Senin <svillar>

Component:

Accessibility

Assignee:

Joanmarie Diggs <jdiggs>

Status:

RESOLVED FIXED

Severity:

Major

CC:

aboxhall, apinheiro, buildbot, cfleizach, dmazzoni, jdiggs, mario, rniwa, svillar, webkit.review.bot, zan

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

106903

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Same fix; now with layout test.	none

Sergio Villar Senin

Reported 2013-01-18 04:53:33 PST

It's quite easy to get a crash when performing browsing actions on pages including tables. I always get one when doing things like: 1- go to www.amazon.com 2- input some text in the search box and click on "Go" 3- sroll down to the end of the list and click on "Next Page" you'd get a crash like this one (release webkitgtk build): Program received signal SIGSEGV, Segmentation fault. 0x00007ffff510a14b in WebCore::AccessibilityTableCell::parentTable() const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 (gdb) bt #0 0x00007ffff510a14b in WebCore::AccessibilityTableCell::parentTable() const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #1 0x00007ffff510a09d in WebCore::AccessibilityTableCell::isTableCell() const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #2 0x00007ffff510a0dd in WebCore::AccessibilityTableCell::roleValue() const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #3 0x00007ffff5f8d106 in webkitAccessibleDetach () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #4 0x00007ffff510de3c in WebCore::AXObjectCache::remove(unsigned int) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #5 0x00007ffff510e05e in WebCore::AXObjectCache::remove(WebCore::RenderObject*) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #6 0x00007ffff58bb4ac in WebCore::RenderObject::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #7 0x00007ffff57dbd5d in WebCore::RenderBlock::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #9 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #10 0x00007ffff58bb1fa in WebCore::RenderObject::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #11 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #12 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #13 0x00007ffff58bb1fa in WebCore::RenderObject::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #14 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #15 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #16 0x00007ffff57dbc7c in WebCore::RenderBlock::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #17 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #18 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #19 0x00007ffff57dbc7c in WebCore::RenderBlock::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #20 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #21 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #22 0x00007ffff57dbc7c in WebCore::RenderBlock::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #23 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #24 0x00007ffff5346ac2 in WebCore::Node::detach() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #25 0x00007ffff52d6b6e in WebCore::ContainerNode::detach() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #26 0x00007ffff5322734 in WebCore::Element::detach() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #27 0x00007ffff53232cc in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #28 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #29 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #30 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #31 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #32 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #33 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #34 0x00007ffff52f72db in WebCore::Document::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #35 0x00007ffff52f76ce in WebCore::Document::updateStyleIfNeeded() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #36 0x00007ffff52f8212 in WebCore::Document::updateLayout() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #37 0x00007ffff52fa939 in WebCore::Document::updateLayoutIgnorePendingStylesheets() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #38 0x00007ffff572590b in WebCore::DOMWindow::scrollTo(int, int) const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0 #39 0x00007ffff5af5848 in WebCore::jsDOMWindowPrototypeFunctionScrollTo(JSC::ExecState*) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0

Attachments
Patch (1.70 KB, patch) 2013-01-18 04:55 PST, Sergio Villar Senin	no flags	Details Formatted Diff Diff
Patch (3.20 KB, patch) 2013-01-18 08:05 PST, Joanmarie Diggs	no flags	Details Formatted Diff Diff
Patch (3.83 KB, patch) 2013-01-18 10:03 PST, Joanmarie Diggs	no flags	Details Formatted Diff Diff
Same fix; now with layout test. (6.45 KB, patch) 2013-01-18 10:59 PST, Joanmarie Diggs	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Sergio Villar Senin

Comment 1 2013-01-18 04:55:58 PST

Created attachment 183426 [details] Patch

Sergio Villar Senin

Comment 2 2013-01-18 05:27:39 PST

Comment on attachment 183426 [details] Patch Actually I'm no longer setting it for review, because it looked like an ad hoc fix. The crashes are still there in some other cases even if the patch is applied.

chris fleizach

Comment 3 2013-01-18 06:24:54 PST

I think the isTableCell call can rely solely on roleValue() and we could Do the same thing we just did for table rows, which is determine the role up front and cache it

Joanmarie Diggs

Comment 4 2013-01-18 06:30:40 PST

(In reply to comment #3) > I think the isTableCell call can rely solely on roleValue() and we could > Do the same thing we just did for table rows, which is determine the role up front and cache it Cool. I'll give that a try. Thanks! I'm also working on creating a layout test for this bug.

Joanmarie Diggs

Comment 5 2013-01-18 08:05:06 PST

Created attachment 183461 [details] Patch

Joanmarie Diggs

Comment 6 2013-01-18 08:05:59 PST

It's taking longer than I thought to create this layout test. :( So tossing the proposed patch at EWS in the meantime.

chris fleizach

Comment 7 2013-01-18 08:09:22 PST

I think the concept looks good.

Joanmarie Diggs

Comment 8 2013-01-18 09:33:40 PST

(In reply to comment #7) > I think the concept looks good. And while the EWS takes its time and spits up, I ran the mac tests. Looks like we have a similar issue with search-when-element-starts-in-table.html. I'll fix that and submit a new patch.

Build Bot

Comment 9 2013-01-18 09:41:56 PST

Comment on attachment 183461 [details] Patch Attachment 183461 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15939679 New failing tests: platform/mac/accessibility/search-when-element-starts-in-table.html

Joanmarie Diggs

Comment 10 2013-01-18 10:03:52 PST

Created attachment 183493 [details] Patch

Joanmarie Diggs

Comment 11 2013-01-18 10:09:07 PST

Comment on attachment 183493 [details] Patch Chris, this addresses the regression on the mac. I'm having problems coming up with a reliable crasher Layout Test. Given that we have a bunch of table tests to ensure this doesn't introduce a regression, and given that non-AT users have been complaining quite a bit about this one.... How essential is it that I create a new layout test in order to get this fix committed?

chris fleizach

Comment 12 2013-01-18 10:13:36 PST

(In reply to comment #11) > (From update of attachment 183493 [details]) > Chris, this addresses the regression on the mac. > > I'm having problems coming up with a reliable crasher Layout Test. Given that we have a bunch of table tests to ensure this doesn't introduce a regression, and given that non-AT users have been complaining quite a bit about this one.... How essential is it that I create a new layout test in order to get this fix committed? looking at the BT, it looks like if you listen for a scroll to event, then delete an entire table, after having accessed the whole table through accessibility, it should occur it seems like as soon as webkitAccessibleDetach calls roleValue() on a tableCell who's parent is already gone, you should hit it. perhaps having a reference to the table cell and then deleting the parent... it's also possible this one fixed it? or have you been reproducing on a build after this? http://trac.webkit.org/changeset/139444

Joanmarie Diggs

Comment 13 2013-01-18 10:16:17 PST

(In reply to comment #12) > it's also possible this one fixed it? or have you been reproducing on a build after this? > > http://trac.webkit.org/changeset/139444 I can reproduce with git master. :(

Joanmarie Diggs

Comment 14 2013-01-18 10:39:04 PST

(In reply to comment #12) > looking at the BT, it looks like if you listen for a scroll to event, then delete an entire table, after having accessed the whole table through accessibility, it should occur Brilliant! I don't even have to listen for (or even have) that event. Thanks!!

Joanmarie Diggs

Comment 15 2013-01-18 10:59:21 PST

Created attachment 183509 [details] Same fix; now with layout test.

Joanmarie Diggs

Comment 16 2013-01-18 17:54:51 PST

(In reply to comment #7) > I think the concept looks good. How about the code and layout test? :) I'm not sure what's holding up the EFL bot but all the other EWS bots are green.

Sergio Villar Senin

Comment 17 2013-01-20 03:37:47 PST

*** Bug 106431 has been marked as a duplicate of this bug. ***

chris fleizach

Comment 18 2013-01-21 08:54:31 PST

Comment on attachment 183509 [details] Same fix; now with layout test. r=me

WebKit Review Bot

Comment 19 2013-01-21 08:59:07 PST

Comment on attachment 183509 [details] Same fix; now with layout test. Clearing flags on attachment: 183509 Committed r140340: <http://trac.webkit.org/changeset/140340>

WebKit Review Bot

Comment 20 2013-01-21 08:59:12 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.