Summary: | AX:Null pointer may be dereferenced. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Lukasz Gajowy <l.gajowy> | ||||||||
Component: | Accessibility | Assignee: | Lukasz Gajowy <l.gajowy> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, g.czajkowski, jdiggs, k.czech, mario, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Lukasz Gajowy
2013-08-26 05:25:52 PDT
that would indicate that we're passing in a new Widget type. Certainly that should be protected and ASSERTED, but I don't think that is actually happening anywhere Created attachment 209761 [details]
Patch
Comment on attachment 209761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209761&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:334 > + // Will crash later when widget type is other than FrameView or ScrollBar. This comment should read something like -- "Catch the case if an (unsupported) widget type is used. Only FrameView and ScrollBar are supported now." > Source/WebCore/accessibility/AXObjectCache.cpp:335 > + ASSERT(newObj); we should also do a if (!newObject) return; here the ASSERT will catch this problem in DEBUG, but not in release Created attachment 209849 [details]
Patch #2
Comment on attachment 209849 [details] Patch #2 View in context: https://bugs.webkit.org/attachment.cgi?id=209849&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:334 > + // Catch the case if an (unsupported) widget type is used. Only FrameView and ScrollBar are supported now. But we should still have the ASSERT. We want both assert and (!newObj) check. That way if it happens in DEBUG we'll know right away, but if it happens in release, no one will crash Thanks Created attachment 209850 [details]
Patch #3
Sorry, I misread your review the last time. No it is the way you explained it.
Comment on attachment 209850 [details]
Patch #3
thanks. looks good. let's make sure the EWS bots are good before committing
Comment on attachment 209850 [details] Patch #3 Attachment 209850 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1605174 Comment on attachment 209850 [details]
Patch #3
Windows build bot seems busted, so moving forward with this
Comment on attachment 209850 [details] Patch #3 Clearing flags on attachment: 209850 Committed r154767: <http://trac.webkit.org/changeset/154767> All reviewed patches have been landed. Closing bug. |