Bug 140618 - AX: crash in accessibilityRootObjectWrapper method (WebPageAccessibilityObjectAtk.cpp)
Summary: AX: crash in accessibilityRootObjectWrapper method (WebPageAccessibilityObjec...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-19 05:57 PST by Fabien Vallée
Modified: 2015-01-26 08:25 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.64 KB, patch)
2015-01-19 06:10 PST, Fabien Vallée
no flags Details | Formatted Diff | Diff
Patch (1.68 KB, patch)
2015-01-20 00:55 PST, Fabien Vallée
no flags Details | Formatted Diff | Diff
Patch (1.94 KB, patch)
2015-01-21 02:24 PST, Fabien Vallée
no flags Details | Formatted Diff | Diff
Patch (1.94 KB, patch)
2015-01-26 01:38 PST, Fabien Vallée
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fabien Vallée 2015-01-19 05:57:31 PST
We have a scenario where webkit crashes in  WebCore::AXObjectCache::rootObject() 
(unfortunately I have no way to reduce the crash scenario or to provide a regressin test).

The stack track shows that the crash is due to AXObjectCache::rootObject()  invoked with this = nullptr

0x00007fd0474eb898 in WebCore::AXObjectCache::rootObject() () from XXXXX/libwebkit2gtk-4.0.so.37
(gdb) bt
#0 0x00007fd0474eb898 in WebCore::AXObjectCache::rootObject() () from XXXXX/libwebkit2gtk-4.0.so.37
#1 0x00007fd0473e998b in accessibilityRootObjectWrapper(_AtkObject*) () from XXXXX/libwebkit2gtk-4.0.so.37
#2 0x00007fd0473e9b62 in webPageAccessibilityObjectRefresh () from XXXXX/libwebkit2gtk-4.0.so.37
#3 0x00007fd047a257e0 in WebCore::FrameLoader::dispatchDidClearWindowObjectInWorld(WebCore::DOMWrapperWorld&) ()
   from XXXXX/libwebkit2gtk-4.0.so.37
#4 0x00007fd047a258af in WebCore::FrameLoader::dispatchDidClearWindowObjectsInAllWorlds() () from XXXXX/libwebkit2gtk-4.0.so.37
#5 0x00007fd047a31ac2 in WebCore::FrameLoader::receivedFirstData() () from XXXXX/libwebkit2gtk-4.0.so.37
#6 0x00007fd047a0eb18 in WebCore::DocumentLoader::commitData(char const*, unsigned long) () from XXXXX/libwebkit2gtk-4.0.so.37
#7 0x00007fd04733fc6e in WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) ()
   from XXXXX/libwebkit2gtk-4.0.so.37
#8 0x00007fd047a0ec2f in WebCore::DocumentLoader::commitLoad(char const*, int) () from XXXXX/libwebkit2gtk-4.0.so.37
#9 0x00007fd047a8e034 in WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, unsigned int) ()
   from XXXXX/libwebkit2gtk-4.0.so.37
#10 0x00007fd047a8e17c in WebCore::CachedRawResource::addDataBuffer(WebCore::SharedBuffer&) () from XXXXX/libwebkit2gtk-4.0.so.37
#11 0x00007fd047a57f9f in WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, WTF::PassRefPtr<WebCore::SharedBuffer>, long long, WebCore::DataPayloadType) ()
   from XXXXX/libwebkit2gtk-4.0.so.37
#12 0x00007fd047a5810a in WebCore::SubresourceLoader::didReceiveBuffer(WTF::PassRefPtr<WebCore::SharedBuffer>, long long, WebCore::DataPayloadType) ()
   from XXXXX/libwebkit2gtk-4.0.so.37
#13 0x00007fd047a50cdf in WebCore::ResourceLoader::didReceiveBuffer(WebCore::ResourceHandle*, WTF::PassRefPtr<WebCore::SharedBuffer>, int) ()
   from XXXXX/libwebkit2gtk-4.0.so.37
#14 0x00007fd0480f9c9b in WebCore::readCallback(_GObject*, _GAsyncResult*, void*) () from XXXXX/libwebkit2gtk-4.0.so.37
Comment 1 Fabien Vallée 2015-01-19 06:10:36 PST
Created attachment 244900 [details]
Patch
Comment 2 chris fleizach 2015-01-19 06:53:39 PST
Comment on attachment 244900 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/atk/WebPageAccessibilityObjectAtk.cpp:59
> +    AccessibilityObject* coreRootObject = coreFrame.document()->axObjectCache() ? coreFrame.document()->axObjectCache()->rootObject() : 0;

I think you can cache the AXObjectCache in a local variable
Also use nullptr instead of 0
Can you add a test for tgus
Comment 3 chris fleizach 2015-01-19 06:54:21 PST
Tgus=this


(In reply to comment #2)
> Comment on attachment 244900 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244900&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/atk/WebPageAccessibilityObjectAtk.cpp:59
> > +    AccessibilityObject* coreRootObject = coreFrame.document()->axObjectCache() ? coreFrame.document()->axObjectCache()->rootObject() : 0;
> 
> I think you can cache the AXObjectCache in a local variable
> Also use nullptr instead of 0
> Can you add a test for tgus
Comment 4 Fabien Vallée 2015-01-20 00:55:41 PST
Created attachment 244973 [details]
Patch
Comment 5 Fabien Vallée 2015-01-20 01:00:45 PST
Thanks for review. Patch updated with cache as a local variable and nullptr.

I really wish I could add a regression test but the only case we have to reproduce the crash is using non-standard browser extensions - which are not related to / using the AXObjectCache.
Comment 6 chris fleizach 2015-01-20 07:29:38 PST
Comment on attachment 244973 [details]
Patch

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

> Source/WebKit2/ChangeLog:9
> +

Can you add a comment here regarding the conditions that lead to the crash and why no test is possible
Comment 7 Fabien Vallée 2015-01-20 08:26:24 PST
BTW I just figured out but I think the exact same change has been done in mac port !

http://trac.webkit.org/changeset/167136/trunk/Source/WebKit2/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectBase.mm

I will add comments in changelog and update the patch.
Comment 8 Fabien Vallée 2015-01-21 02:24:04 PST
Created attachment 245054 [details]
Patch
Comment 9 chris fleizach 2015-01-26 00:42:45 PST
Comment on attachment 245054 [details]
Patch

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

> Source/WebKit2/ChangeLog:8
> +        check if document()->axObjectCache() is nullptr to fix the crash

Can you make these comments complete sentences. Capitalize, use periods at the end

> Source/WebKit2/ChangeLog:12
> +        crash occured on <http://itv.mit-xperts.com/hbbtvtest/appmanager/>

indentation in this block is off. the lines do not line up
Comment 10 Fabien Vallée 2015-01-26 01:38:19 PST
Created attachment 245333 [details]
Patch
Comment 11 WebKit Commit Bot 2015-01-26 08:25:12 PST
Comment on attachment 245333 [details]
Patch

Clearing flags on attachment: 245333

Committed r179115: <http://trac.webkit.org/changeset/179115>
Comment 12 WebKit Commit Bot 2015-01-26 08:25:16 PST
All reviewed patches have been landed.  Closing bug.