Summary: | AX: crash in accessibilityRootObjectWrapper method (WebPageAccessibilityObjectAtk.cpp) | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fabien Vallée <fvallee> | ||||||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cfleizach, commit-queue, mario | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Fabien Vallée
2015-01-19 05:57:31 PST
Created attachment 244900 [details]
Patch
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 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 Created attachment 244973 [details]
Patch
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 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 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. Created attachment 245054 [details]
Patch
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 Created attachment 245333 [details]
Patch
Comment on attachment 245333 [details] Patch Clearing flags on attachment: 245333 Committed r179115: <http://trac.webkit.org/changeset/179115> All reviewed patches have been landed. Closing bug. |