Changeset 223151 in webkit


Ignore:
Timestamp:
Oct 10, 2017 3:47:30 PM (7 years ago)
Author:
Alan Bujtas
Message:

AccessibilityRenderObject should not hold a raw pointer to RenderObject
​https://bugs.webkit.org/show_bug.cgi?id=178144
<rdar://problem/34919287>

Reviewed by Chris Fleizach.

m_renderer's lifetime is not directly tied to the AX wrapper object's lifetime.

Covered by existing tests.

  • accessibility/AccessibilityListBox.cpp:

(WebCore::AccessibilityListBox::elementAccessibilityHitTest const):

  • accessibility/AccessibilityMathMLElement.cpp:

(WebCore::AccessibilityMathMLElement::isMathFenceOperator const):
(WebCore::AccessibilityMathMLElement::isMathSeparatorOperator const):
(WebCore::AccessibilityMathMLElement::mathLineThickness const):

  • accessibility/AccessibilityMenuList.cpp:

(WebCore::AccessibilityMenuList::press):
(WebCore::AccessibilityMenuList::isCollapsed const):

  • accessibility/AccessibilityRenderObject.cpp:

(WebCore::AccessibilityRenderObject::AccessibilityRenderObject):
(WebCore::AccessibilityRenderObject::renderBoxModelObject const):
(WebCore::AccessibilityRenderObject::setRenderer):
(WebCore::AccessibilityRenderObject::previousSibling const):
(WebCore::AccessibilityRenderObject::anchorElement const):
(WebCore::AccessibilityRenderObject::helpText const):
(WebCore::AccessibilityRenderObject::boundingBoxRect const):
(WebCore::AccessibilityRenderObject::supportsPath const):
(WebCore::AccessibilityRenderObject::elementPath const):
(WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const):
(WebCore::AccessibilityRenderObject::index const):
(WebCore::AccessibilityRenderObject::handleActiveDescendantChanged):
(WebCore::AccessibilityRenderObject::observableObject const):
(WebCore::AccessibilityRenderObject::determineAccessibilityRole):
(WebCore::AccessibilityRenderObject::textChanged):
(WebCore::AccessibilityRenderObject::remoteSVGRootElement const):
(WebCore::AccessibilityRenderObject::roleValueForMSAA const):
(WebCore::AccessibilityRenderObject::getScrollableAreaIfScrollable const):
(WebCore::AccessibilityRenderObject::scrollTo const):

  • accessibility/AccessibilityRenderObject.h:

(WebCore::AccessibilityRenderObject::setRenderObject):

  • accessibility/AccessibilitySlider.cpp:

(WebCore::AccessibilitySlider::elementAccessibilityHitTest const):

  • accessibility/AccessibilityTable.cpp:

(WebCore::AccessibilityTable::addChildren):

  • accessibility/AccessibilityTableCell.cpp:

(WebCore::AccessibilityTableCell::computeAccessibilityIsIgnored const):
(WebCore::AccessibilityTableCell::parentTable const):
(WebCore::AccessibilityTableCell::rowIndexRange const):
(WebCore::AccessibilityTableCell::columnIndexRange const):
(WebCore::AccessibilityTableCell::titleUIElement const):

Location:
trunk/Source/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r223149 r223151  
     12017-10-10  Zalan Bujtas  <zalan@apple.com>
     2
     3        AccessibilityRenderObject should not hold a raw pointer to RenderObject
     4        https://bugs.webkit.org/show_bug.cgi?id=178144
     5        <rdar://problem/34919287>
     6
     7        Reviewed by Chris Fleizach.
     8
     9        m_renderer's lifetime is not directly tied to the AX wrapper object's lifetime.
     10
     11        Covered by existing tests.
     12
     13        * accessibility/AccessibilityListBox.cpp:
     14        (WebCore::AccessibilityListBox::elementAccessibilityHitTest const):
     15        * accessibility/AccessibilityMathMLElement.cpp:
     16        (WebCore::AccessibilityMathMLElement::isMathFenceOperator const):
     17        (WebCore::AccessibilityMathMLElement::isMathSeparatorOperator const):
     18        (WebCore::AccessibilityMathMLElement::mathLineThickness const):
     19        * accessibility/AccessibilityMenuList.cpp:
     20        (WebCore::AccessibilityMenuList::press):
     21        (WebCore::AccessibilityMenuList::isCollapsed const):
     22        * accessibility/AccessibilityRenderObject.cpp:
     23        (WebCore::AccessibilityRenderObject::AccessibilityRenderObject):
     24        (WebCore::AccessibilityRenderObject::renderBoxModelObject const):
     25        (WebCore::AccessibilityRenderObject::setRenderer):
     26        (WebCore::AccessibilityRenderObject::previousSibling const):
     27        (WebCore::AccessibilityRenderObject::anchorElement const):
     28        (WebCore::AccessibilityRenderObject::helpText const):
     29        (WebCore::AccessibilityRenderObject::boundingBoxRect const):
     30        (WebCore::AccessibilityRenderObject::supportsPath const):
     31        (WebCore::AccessibilityRenderObject::elementPath const):
     32        (WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const):
     33        (WebCore::AccessibilityRenderObject::index const):
     34        (WebCore::AccessibilityRenderObject::handleActiveDescendantChanged):
     35        (WebCore::AccessibilityRenderObject::observableObject const):
     36        (WebCore::AccessibilityRenderObject::determineAccessibilityRole):
     37        (WebCore::AccessibilityRenderObject::textChanged):
     38        (WebCore::AccessibilityRenderObject::remoteSVGRootElement const):
     39        (WebCore::AccessibilityRenderObject::roleValueForMSAA const):
     40        (WebCore::AccessibilityRenderObject::getScrollableAreaIfScrollable const):
     41        (WebCore::AccessibilityRenderObject::scrollTo const):
     42        * accessibility/AccessibilityRenderObject.h:
     43        (WebCore::AccessibilityRenderObject::setRenderObject):
     44        * accessibility/AccessibilitySlider.cpp:
     45        (WebCore::AccessibilitySlider::elementAccessibilityHitTest const):
     46        * accessibility/AccessibilityTable.cpp:
     47        (WebCore::AccessibilityTable::addChildren):
     48        * accessibility/AccessibilityTableCell.cpp:
     49        (WebCore::AccessibilityTableCell::computeAccessibilityIsIgnored const):
     50        (WebCore::AccessibilityTableCell::parentTable const):
     51        (WebCore::AccessibilityTableCell::rowIndexRange const):
     52        (WebCore::AccessibilityTableCell::columnIndexRange const):
     53        (WebCore::AccessibilityTableCell::titleUIElement const):
     54
    1552017-10-10  Sam Weinig  <sam@webkit.org>
    256

  • trunk/Source/WebCore/accessibility/AccessibilityListBox.cpp

    r177733 r223151  
    173173        return listBoxOption;
    174174   
    175     return axObjectCache()->getOrCreate(m_renderer);
     175    return axObjectCache()->getOrCreate(renderer());
    176176}
    177177

  • trunk/Source/WebCore/accessibility/AccessibilityMathMLElement.cpp

    r202961 r223151  
    154154bool AccessibilityMathMLElement::isMathFenceOperator() const
    155155{
    156     if (!is<RenderMathMLOperator>(m_renderer))
     156    if (!is<RenderMathMLOperator>(renderer()))
    157157        return false;
    158158
    … …  
    162162bool AccessibilityMathMLElement::isMathSeparatorOperator() const
    163163{
    164     if (!is<RenderMathMLOperator>(m_renderer))
     164    if (!is<RenderMathMLOperator>(renderer()))
    165165        return false;
    166166
    … …  
    446446int AccessibilityMathMLElement::mathLineThickness() const
    447447{
    448     if (!is<RenderMathMLFraction>(m_renderer))
     448    if (!is<RenderMathMLFraction>(renderer()))
    449449        return -1;
    450450

  • trunk/Source/WebCore/accessibility/AccessibilityMenuList.cpp

    r184097 r223151  
    4646{
    4747#if !PLATFORM(IOS)
    48     RenderMenuList* menuList = static_cast<RenderMenuList*>(m_renderer);
     48    RenderMenuList* menuList = static_cast<RenderMenuList*>(renderer());
    4949    if (menuList->popupIsVisible())
    5050        menuList->hidePopup();
    … …  
    9494{
    9595#if !PLATFORM(IOS)
    96     return !static_cast<RenderMenuList*>(m_renderer)->popupIsVisible();
     96    return !static_cast<RenderMenuList*>(renderer())->popupIsVisible();
    9797#else
    9898    return true;

  • trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

    r223127 r223151  
    109109AccessibilityRenderObject::AccessibilityRenderObject(RenderObject* renderer)
    110110    : AccessibilityNodeObject(renderer->node())
    111     , m_renderer(renderer)
     111    , m_renderer(makeWeakPtr(renderer))
    112112{
    113113#ifndef NDEBUG
    … …  
    146146RenderBoxModelObject* AccessibilityRenderObject::renderBoxModelObject() const
    147147{
    148     if (!is<RenderBoxModelObject>(m_renderer))
    149         return nullptr;
    150     return downcast<RenderBoxModelObject>(m_renderer);
     148    if (!is<RenderBoxModelObject>(renderer()))
     149        return nullptr;
     150    return downcast<RenderBoxModelObject>(renderer());
    151151}
    152152
    153153void AccessibilityRenderObject::setRenderer(RenderObject* renderer)
    154154{
    155     m_renderer = renderer;
     155    m_renderer = makeWeakPtr(renderer);
    156156    setNode(renderer->node());
    157157}
    … …  
    325325    RenderInline* startOfConts;
    326326    if (is<RenderBox>(*m_renderer) && (startOfConts = startOfContinuations(*m_renderer)))
    327         previousSibling = childBeforeConsideringContinuations(startOfConts, m_renderer);
     327        previousSibling = childBeforeConsideringContinuations(startOfConts, renderer());
    328328
    329329    // Case 2: Anonymous block parent of the end of a continuation - skip all the way to before
    … …  
    562562   
    563563    // Search up the render tree for a RenderObject with a DOM node.  Defer to an earlier continuation, though.
    564     for (currentRenderer = m_renderer; currentRenderer && !currentRenderer->node(); currentRenderer = currentRenderer->parent()) {
     564    for (currentRenderer = renderer(); currentRenderer && !currentRenderer->node(); currentRenderer = currentRenderer->parent()) {
    565565        if (currentRenderer->isAnonymousBlock()) {
    566566            if (RenderObject* continuation = downcast<RenderBlock>(*currentRenderer).continuation())
    … …  
    597597   
    598598    String description = accessibilityDescription();
    599     for (RenderObject* ancestor = m_renderer; ancestor; ancestor = ancestor->parent()) {
     599    for (RenderObject* ancestor = renderer(); ancestor; ancestor = ancestor->parent()) {
    600600        if (is<HTMLElement>(ancestor->node())) {
    601601            HTMLElement& element = downcast<HTMLElement>(*ancestor->node());
    … …  
    824824LayoutRect AccessibilityRenderObject::boundingBoxRect() const
    825825{
    826     RenderObject* obj = m_renderer;
     826    RenderObject* obj = renderer();
    827827   
    828828    if (!obj)
    … …  
    886886bool AccessibilityRenderObject::supportsPath() const
    887887{
    888     return is<RenderSVGShape>(m_renderer);
     888    return is<RenderSVGShape>(renderer());
    889889}
    890890
    891891Path AccessibilityRenderObject::elementPath() const
    892892{
    893     if (is<RenderSVGShape>(m_renderer) && downcast<RenderSVGShape>(*m_renderer).hasPath()) {
     893    if (is<RenderSVGShape>(renderer()) && downcast<RenderSVGShape>(*m_renderer).hasPath()) {
    894894        Path path = downcast<RenderSVGShape>(*m_renderer).path();
    895895       
    … …  
    11961196   
    11971197    // WebAreas should be ignored if their iframe container is marked as presentational.
    1198     if (webAreaIsPresentational(m_renderer))
     1198    if (webAreaIsPresentational(renderer()))
    11991199        return true;
    1200        
     1200
    12011201    // An ARIA tree can only have tree items and static text as children.
    12021202    if (!isAllowedChildOfTree())
    … …  
    13331333        // However, if this is not a native image, fallback to the attribute on the Element.
    13341334        AccessibilityObjectInclusion altTextInclusion = DefaultBehavior;
    1335         bool isRenderImage = is<RenderImage>(m_renderer);
     1335        bool isRenderImage = is<RenderImage>(renderer());
    13361336        if (isRenderImage)
    13371337            altTextInclusion = objectInclusionFromAltText(downcast<RenderImage>(*m_renderer).altText());
    … …  
    21982198        return -1;
    21992199
    2200     if (renderObjectContainsPosition(m_renderer, position.deepEquivalent()))
     2200    if (renderObjectContainsPosition(renderer(), position.deepEquivalent()))
    22012201        return indexForVisiblePosition(position);
    22022202   
    … …  
    25112511
    25122512    if (activeDescendant() && shouldNotifyActiveDescendant())
    2513         renderer()->document().axObjectCache()->postNotification(m_renderer, AXObjectCache::AXActiveDescendantChanged);
     2513        renderer()->document().axObjectCache()->postNotification(renderer(), AXObjectCache::AXActiveDescendantChanged);
    25142514}
    25152515
    … …  
    25742574{
    25752575    // Find the object going up the parent chain that is used in accessibility to monitor certain notifications.
    2576     for (RenderObject* renderer = m_renderer; renderer && renderer->node(); renderer = renderer->parent()) {
     2576    for (RenderObject* renderer = this->renderer(); renderer && renderer->node(); renderer = renderer->parent()) {
    25772577        if (renderObjectIsObservable(*renderer)) {
    25782578            if (AXObjectCache* cache = axObjectCache())
    … …  
    27282728    // This return value is what will be used if AccessibilityTableCell determines
    27292729    // the cell should not be treated as a cell (e.g. because it is a layout table.
    2730     if (is<RenderTableCell>(m_renderer))
     2730    if (is<RenderTableCell>(renderer()))
    27312731        return TextGroupRole;
    27322732
    … …  
    29602960        return;
    29612961   
    2962     for (RenderObject* renderParent = m_renderer; renderParent; renderParent = renderParent->parent()) {
     2962    for (RenderObject* renderParent = renderer(); renderParent; renderParent = renderParent->parent()) {
    29632963        AccessibilityObject* parent = cache->get(renderParent);
    29642964        if (!parent)
    … …  
    30473047AccessibilitySVGRoot* AccessibilityRenderObject::remoteSVGRootElement(CreationChoice createIfNecessary) const
    30483048{
    3049     if (!is<RenderImage>(m_renderer))
     3049    if (!is<RenderImage>(renderer()))
    30503050        return nullptr;
    30513051   
    … …  
    36673667        return m_roleForMSAA;
    36683668
    3669     m_roleForMSAA = msaaRoleForRenderer(m_renderer);
     3669    m_roleForMSAA = msaaRoleForRenderer(renderer());
    36703670
    36713671    if (m_roleForMSAA == UnknownRole)
    … …  
    36973697        return nullptr;
    36983698
    3699     if (!is<RenderBox>(m_renderer))
     3699    if (!is<RenderBox>(renderer()))
    37003700        return nullptr;
    37013701
    … …  
    37093709void AccessibilityRenderObject::scrollTo(const IntPoint& point) const
    37103710{
    3711     if (!is<RenderBox>(m_renderer))
     3711    if (!is<RenderBox>(renderer()))
    37123712        return;
    37133713

  • trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h

    r222422 r223151  
    3131#include "AccessibilityNodeObject.h"
    3232#include "LayoutRect.h"
     33#include "RenderObject.h"
    3334#include <wtf/Forward.h>
    3435#include <wtf/WeakPtr.h>
    … …  
    115116   
    116117    void setRenderer(RenderObject*);
    117     RenderObject* renderer() const override { return m_renderer; }
     118    RenderObject* renderer() const override { return m_renderer.get(); }
    118119    RenderBoxModelObject* renderBoxModelObject() const;
    119120    Node* node() const override;
    … …  
    205206protected:
    206207    explicit AccessibilityRenderObject(RenderObject*);
    207     void setRenderObject(RenderObject* renderer) { m_renderer = renderer; }
    208208    ScrollableArea* getScrollableAreaIfScrollable() const override;
    209209    void scrollTo(const IntPoint&) const override;
    … …  
    218218#endif
    219219
    220     RenderObject* m_renderer;
     220    WeakPtr<RenderObject> m_renderer;
    221221
    222222private:

  • trunk/Source/WebCore/accessibility/AccessibilitySlider.cpp

    r177733 r223151  
    109109    }
    110110   
    111     return axObjectCache()->getOrCreate(m_renderer);
     111    return axObjectCache()->getOrCreate(renderer());
    112112}
    113113

  • trunk/Source/WebCore/accessibility/AccessibilityTable.cpp

    r222790 r223151  
    387387   
    388388    m_haveChildren = true;
    389     if (!is<RenderTable>(m_renderer))
     389    if (!is<RenderTable>(renderer()))
    390390        return;
    391391   

  • trunk/Source/WebCore/accessibility/AccessibilityTableCell.cpp

    r216167 r223151  
    6767   
    6868    // Ignore anonymous table cells as long as they're not in a table (ie. when display:table is used).
    69     RenderObject* renderTable = is<RenderTableCell>(m_renderer) ? downcast<RenderTableCell>(*m_renderer).table() : nullptr;
     69    RenderObject* renderTable = is<RenderTableCell>(renderer()) ? downcast<RenderTableCell>(*m_renderer).table() : nullptr;
    7070    bool inTable = renderTable && renderTable->node() && (renderTable->node()->hasTagName(tableTag) || nodeHasRole(renderTable->node(), "grid"));
    7171    if (!node() && !inTable)
    … …  
    8080AccessibilityTable* AccessibilityTableCell::parentTable() const
    8181{
    82     if (!is<RenderTableCell>(m_renderer))
     82    if (!is<RenderTableCell>(renderer()))
    8383        return nullptr;
    8484
    … …  
    315315void AccessibilityTableCell::rowIndexRange(std::pair<unsigned, unsigned>& rowRange) const
    316316{
    317     if (!is<RenderTableCell>(m_renderer))
     317    if (!is<RenderTableCell>(renderer()))
    318318        return;
    319319   
    … …  
    333333void AccessibilityTableCell::columnIndexRange(std::pair<unsigned, unsigned>& columnRange) const
    334334{
    335     if (!is<RenderTableCell>(m_renderer))
     335    if (!is<RenderTableCell>(renderer()))
    336336        return;
    337337   
    … …  
    354354    // then it can act as the title ui element. (This is only in the
    355355    // case when the table is not appearing as an AXTable.)
    356     if (isTableCell() || !is<RenderTableCell>(m_renderer))
     356    if (isTableCell() || !is<RenderTableCell>(renderer()))
    357357        return nullptr;
    358358
Note: See TracChangeset for help on using the changeset viewer.