Changeset 256347 in webkit


Ignore:
Timestamp:
Feb 11, 2020 1:33:41 PM (4 years ago)
Author:
Andres Gonzalez
Message:

Fix for crashes in WebAccessibilityObjectWrapper after notification updates in IsolatedTree mode.
https://bugs.webkit.org/show_bug.cgi?id=207558

Reviewed by Chris Fleizach.

  • Accessibility methods invoked in the secondary thread that Return id

values retrieved from the main thread, need to retain/autorelease the
returned ids.

  • When serving a request on the AX thread that requires retrieving a

value from the main thread, the backing obbject on the main thread may
have gone away, so need to check for nullity of the backing object also
on the main thread.

  • accessibility/AccessibilityObjectInterface.h:

(WebCore::Accessibility::retrieveAutoreleasedValueFromMainThread):

  • accessibility/mac/WebAccessibilityObjectWrapperMac.mm:

(-[WebAccessibilityObjectWrapper attachmentView]):
(-[WebAccessibilityObjectWrapper textMarkerRangeFromRange:]):
(-[WebAccessibilityObjectWrapper renderWidgetChildren]):
(-[WebAccessibilityObjectWrapper associatedPluginParent]):
(-[WebAccessibilityObjectWrapper scrollViewParent]):
(-[WebAccessibilityObjectWrapper windowElement:]):
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:]):

Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r256311 r256347  
     12020-02-11  Andres Gonzalez  <andresg_22@apple.com>
     2
     3        Fix for crashes in WebAccessibilityObjectWrapper after notification updates in IsolatedTree mode.
     4        https://bugs.webkit.org/show_bug.cgi?id=207558
     5
     6        Reviewed by Chris Fleizach.
     7
     8        - Accessibility methods invoked in the secondary thread that Return id
     9        values retrieved from the main thread, need to retain/autorelease the
     10        returned ids.
     11        - When serving a request on the AX thread that requires retrieving a
     12        value from the main thread, the backing obbject on the main thread may
     13        have gone away, so need to check for nullity of the backing object also
     14        on the main thread.
     15
     16        * accessibility/AccessibilityObjectInterface.h:
     17        (WebCore::Accessibility::retrieveAutoreleasedValueFromMainThread):
     18        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
     19        (-[WebAccessibilityObjectWrapper attachmentView]):
     20        (-[WebAccessibilityObjectWrapper textMarkerRangeFromRange:]):
     21        (-[WebAccessibilityObjectWrapper renderWidgetChildren]):
     22        (-[WebAccessibilityObjectWrapper associatedPluginParent]):
     23        (-[WebAccessibilityObjectWrapper scrollViewParent]):
     24        (-[WebAccessibilityObjectWrapper windowElement:]):
     25        (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:]):
     26
    1272020-02-11  Zalan Bujtas  <zalan@apple.com>
    228
  • trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h

    r255381 r256347  
    12081208}
    12091209
     1210#if PLATFORM(COCOA)
     1211template<typename T, typename U> inline T retrieveAutoreleasedValueFromMainThread(U&& lambda)
     1212{
     1213    if (isMainThread())
     1214        return lambda().autorelease();
     1215
     1216    RetainPtr<T> value;
     1217    callOnMainThreadAndWait([&value, &lambda] {
     1218        value = lambda();
     1219    });
     1220    return value.autorelease();
     1221}
     1222#endif
     1223
    12101224} // namespace Accessibility
    12111225
  • trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm

    r255044 r256347  
    558558    ASSERT(self.axBackingObject->isAttachment());
    559559
    560     return Accessibility::retrieveValueFromMainThread<id>([protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     560    return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    561561        auto* widget = protectedSelf.get().axBackingObject->widgetForAttachmentView();
    562562        if (!widget)
     
    800800- (id)textMarkerRangeFromRange:(const RefPtr<Range>)range
    801801{
    802     return textMarkerRangeFromRange(self.axBackingObject->axObjectCache(), range);
     802    if (auto* backingObject = self.axBackingObject)
     803        return textMarkerRangeFromRange(backingObject->axObjectCache(), range);
     804    return nil;
    803805}
    804806
     
    18961898{
    18971899    return Accessibility::retrieveValueFromMainThread<NSArray *>([protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> NSArray * {
    1898         Widget* widget = protectedSelf.get().axBackingObject->widget();
     1900        auto* backingObject = protectedSelf.get().axBackingObject;
     1901        if (!backingObject)
     1902            return nil;
     1903
     1904        Widget* widget = backingObject->widget();
    18991905        if (!widget)
    19001906            return nil;
     
    19481954- (id)associatedPluginParent
    19491955{
    1950     return Accessibility::retrieveValueFromMainThread<id>([protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     1956    return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    19511957        if (!protectedSelf.get().axBackingObject || !protectedSelf.get().axBackingObject->hasApplePDFAnnotationAttribute())
    19521958            return nil;
     
    22992305- (id)scrollViewParent
    23002306{
    2301     return Accessibility::retrieveValueFromMainThread<id>([protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     2307    return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    23022308        if (!is<AccessibilityScrollView>(protectedSelf.get().axBackingObject))
    23032309            return nil;
     
    23442350- (id)windowElement:(NSString*)attributeName
    23452351{
    2346     return Accessibility::retrieveValueFromMainThread<id>([attributeName, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     2352    return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([attributeName, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    23472353        id remoteParent = [protectedSelf remoteAccessibilityParentObject];
    23482354        if (remoteParent) {
     
    23522358        }
    23532359
    2354         if (auto* fv = protectedSelf.get().axBackingObject->documentFrameView())
     2360        auto* backingObject = protectedSelf.get().axBackingObject;
     2361        if (!backingObject)
     2362            return nil;
     2363
     2364        if (auto* fv = backingObject->documentFrameView())
    23552365            return [fv->platformWidget() window];
    23562366
     
    39583968
    39593969    if ([attribute isEqualToString:NSAccessibilityEndTextMarkerForBoundsParameterizedAttribute]) {
    3960         return Accessibility::retrieveValueFromMainThread<id>([&rect, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     3970        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([&rect, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    39613971            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    39623972            if (!cache)
     
    39703980
    39713981    if ([attribute isEqualToString:NSAccessibilityStartTextMarkerForBoundsParameterizedAttribute]) {
    3972         return Accessibility::retrieveValueFromMainThread<id>([&rect, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     3982        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([&rect, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    39733983            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    39743984            if (!cache)
     
    40154025
    40164026    if ([attribute isEqualToString:@"AXTextMarkerRangeForUIElement"]) {
    4017         return Accessibility::retrieveValueFromMainThread<id>([&uiElement, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4027        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([&uiElement, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    40184028            RefPtr<Range> range = uiElement.get()->elementRange();
    40194029            return [protectedSelf textMarkerRangeFromRange:range];
     
    40434053            return nil;
    40444054
    4045         return Accessibility::retrieveValueFromMainThread<id>([&webCorePoint, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4055        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([&webCorePoint, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    40464056            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    40474057            if (!cache)
     
    40624072    if ([attribute isEqualToString:NSAccessibilityBoundsForRangeParameterizedAttribute]) {
    40634073        NSRect rect = Accessibility::retrieveValueFromMainThread<NSRect>([&range, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> NSRect {
    4064             auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
     4074            auto* backingObject = protectedSelf.get().axBackingObject;
     4075            if (!backingObject)
     4076                return CGRectZero;
     4077
     4078            auto* cache = backingObject->axObjectCache();
    40654079            if (!cache)
    40664080                return CGRectZero;
    40674081
    4068             CharacterOffset start = cache->characterOffsetForIndex(range.location, protectedSelf.get().axBackingObject);
    4069             CharacterOffset end = cache->characterOffsetForIndex(range.location+range.length, protectedSelf.get().axBackingObject);
     4082            CharacterOffset start = cache->characterOffsetForIndex(range.location, backingObject);
     4083            CharacterOffset end = cache->characterOffsetForIndex(range.location+range.length, backingObject);
    40704084            if (start.isNull() || end.isNull())
    40714085                return CGRectZero;
    40724086
    40734087            RefPtr<Range> range = cache->rangeForUnorderedCharacterOffsets(start, end);
    4074             auto bounds = FloatRect(protectedSelf.get().axBackingObject->boundsForRange(range));
     4088            auto bounds = FloatRect(backingObject->boundsForRange(range));
    40754089            return [protectedSelf convertRectToSpace:bounds space:AccessibilityConversionSpace::Screen];
    40764090        });
     
    41284142            return nil;
    41294143
    4130         return Accessibility::retrieveValueFromMainThread<id>([textMarker1, textMarker2, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4144        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker1, textMarker2, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    41314145            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    41324146            if (!cache)
     
    41514165
    41524166    if ([attribute isEqualToString:@"AXLeftWordTextMarkerRangeForTextMarker"]) {
    4153         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4167        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    41544168            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    41554169            if (!cache)
     
    41634177
    41644178    if ([attribute isEqualToString:@"AXRightWordTextMarkerRangeForTextMarker"]) {
    4165         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4179        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    41664180            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    41674181            if (!cache)
     
    41874201
    41884202    if ([attribute isEqualToString:@"AXSentenceTextMarkerRangeForTextMarker"]) {
    4189         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4203        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    41904204            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    41914205            if (!cache)
     
    41994213
    42004214    if ([attribute isEqualToString:@"AXParagraphTextMarkerRangeForTextMarker"]) {
    4201         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4215        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    42024216            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    42034217            if (!cache)
     
    42114225
    42124226    if ([attribute isEqualToString:@"AXNextWordEndTextMarkerForTextMarker"]) {
    4213         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4227        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    42144228            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    42154229            if (!cache)
     
    42234237
    42244238    if ([attribute isEqualToString:@"AXPreviousWordStartTextMarkerForTextMarker"]) {
    4225         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4239        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    42264240            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    42274241            if (!cache)
     
    42454259   
    42464260    if ([attribute isEqualToString:@"AXNextSentenceEndTextMarkerForTextMarker"]) {
    4247         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4261        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    42484262            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    42494263            if (!cache)
     
    42574271
    42584272    if ([attribute isEqualToString:@"AXPreviousSentenceStartTextMarkerForTextMarker"]) {
    4259         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4273        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    42604274            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    42614275            if (!cache)
     
    42694283
    42704284    if ([attribute isEqualToString:@"AXNextParagraphEndTextMarkerForTextMarker"]) {
    4271         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4285        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    42724286            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    42734287            if (!cache)
     
    42814295
    42824296    if ([attribute isEqualToString:@"AXPreviousParagraphStartTextMarkerForTextMarker"]) {
    4283         return Accessibility::retrieveValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> id {
     4297        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker, protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
    42844298            auto* cache = protectedSelf.get().axBackingObject->axObjectCache();
    42854299            if (!cache)
Note: See TracChangeset for help on using the changeset viewer.