The Chromium renderer crashes when it accesses objects that have been removed from the webkit accessibility tree. http://code.google.com/p/chromium/issues/detail?id=54973 WebAccessibilityCacheImpl::addOrGetId should know that AccessibilityObject's with an id of 0 are invalid - they've been removed from the webkit accessibility tree.
Created attachment 67258 [details] Return invalidObjectID in WebAccessibilityCacheImpl when an AccessibilityObject's id is 0
Created attachment 67280 [details] Need to ensure updateBackingStore is called Looks like updateBackingStore is where the accessibility object can become invalid.
Created attachment 67281 [details] Compiles now
Created attachment 67283 [details] Logic is correct now
Attachment 67283 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/chromium/src/WebAccessibilityObject.cpp:110: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 67285 [details] Passes style check now
Comment on attachment 67285 [details] Passes style check now > Index: WebKit/chromium/ChangeLog > =================================================================== > --- WebKit/chromium/ChangeLog (revision 67263) > +++ WebKit/chromium/ChangeLog (working copy) > @@ -1,3 +1,16 @@ > +2010-09-10 Chris Guillory <chris.guillory@google.com> > + > + Reviewed by NOBODY (OOPS!). > + > + WebAccessibilityCacheImpl needs to handle invalid accessibility object ids period at end of sentence > + WEBKIT_API bool isAxObjectIdValid() const; since this is inside of an ax object, the method name should probably just be "isIdValid()" > > WEBKIT_API unsigned childCount() const; > > +bool WebAccessibilityObject::isAxObjectIdValid() const > +{ > + if (!m_private) > + return false; > + > + m_private->updateBackingStore(); after calling updateBackingStore() it's possible that m_private will be invalidated (on the WebCore side at least, where it's called m_object). you should probably confirm that the object is still valid after calling this method > + return m_private->axObjectID(); > +} > +
> after calling updateBackingStore() it's possible that m_private will be invalidated (on the WebCore side at least, where it's called m_object). > you should probably confirm that the object is still valid after calling this method I'm adding this method to determine if the WebAccessibilityObject is valid. Perhaps I should just name this method isValid(). An invalid WebCore::AccessibilityObject has an ID of 0.
(In reply to comment #8) > > after calling updateBackingStore() it's possible that m_private will be invalidated (on the WebCore side at least, where it's called m_object). > > you should probably confirm that the object is still valid after calling this method > I'm adding this method to determine if the WebAccessibilityObject is valid. Perhaps I should just name this method isValid(). An invalid WebCore::AccessibilityObject has an ID of 0. yea, if updateBackingStore is just checking validity, it should be renamed.
Created attachment 67360 [details] Naming new method isValid
Comment on attachment 67360 [details] Naming new method isValid r=me
Comment on attachment 67360 [details] Naming new method isValid Clearing flags on attachment: 67360 Committed r67418: <http://trac.webkit.org/changeset/67418>
All reviewed patches have been landed. Closing bug.