Changeset 225872 in webkit


Ignore:
Timestamp:
Dec 13, 2017 2:13:32 PM (6 years ago)
Author:
Alan Bujtas
Message:

RenderImage can be destroyed even before setting the style on it.
https://bugs.webkit.org/show_bug.cgi?id=180767
<rdar://problem/33965995>

Reviewed by Simon Fraser.

Source/WebCore:

In certain cases, when the newly constructed renderer can't be inserted into the tree (parent can only have specific type of children etc),
RenderTreeUpdater destroys it right away. While destroying a RenderImage, the associated image resource assumes
that the image renderer has been initialized through RenderElement::initializeStyle(). This is an incorrect
assumption.
This patch also makes RenderImageResource's m_renderer a weak pointer.

Test: fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html

  • rendering/RenderImageResource.cpp:

(WebCore::RenderImageResource::initialize):
(WebCore::RenderImageResource::setCachedImage):
(WebCore::RenderImageResource::resetAnimation):
(WebCore::RenderImageResource::image const):
(WebCore::RenderImageResource::setContainerContext):
(WebCore::RenderImageResource::imageSize const):

  • rendering/RenderImageResource.h:

(WebCore::RenderImageResource::renderer const):

  • rendering/RenderImageResourceStyleImage.cpp:

(WebCore::RenderImageResourceStyleImage::shutdown):

LayoutTests:

  • fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle-expected.txt: Added.
  • fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r225869 r225872  
     12017-12-13  Zalan Bujtas  <zalan@apple.com>
     2
     3        RenderImage can be destroyed even before setting the style on it.
     4        https://bugs.webkit.org/show_bug.cgi?id=180767
     5        <rdar://problem/33965995>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle-expected.txt: Added.
     10        * fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html: Added.
     11
    1122017-12-13  Matt Lewis  <jlewis3@apple.com>
    213
  • trunk/Source/WebCore/ChangeLog

    r225868 r225872  
     12017-12-13  Zalan Bujtas  <zalan@apple.com>
     2
     3        RenderImage can be destroyed even before setting the style on it.
     4        https://bugs.webkit.org/show_bug.cgi?id=180767
     5        <rdar://problem/33965995>
     6
     7        Reviewed by Simon Fraser.
     8
     9        In certain cases, when the newly constructed renderer can't be inserted into the tree (parent can only have specific type of children etc),
     10        RenderTreeUpdater destroys it right away. While destroying a RenderImage, the associated image resource assumes
     11        that the image renderer has been initialized through RenderElement::initializeStyle(). This is an incorrect
     12        assumption.
     13        This patch also makes RenderImageResource's m_renderer a weak pointer.
     14           
     15        Test: fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html
     16
     17        * rendering/RenderImageResource.cpp:
     18        (WebCore::RenderImageResource::initialize):
     19        (WebCore::RenderImageResource::setCachedImage):
     20        (WebCore::RenderImageResource::resetAnimation):
     21        (WebCore::RenderImageResource::image const):
     22        (WebCore::RenderImageResource::setContainerContext):
     23        (WebCore::RenderImageResource::imageSize const):
     24        * rendering/RenderImageResource.h:
     25        (WebCore::RenderImageResource::renderer const):
     26        * rendering/RenderImageResourceStyleImage.cpp:
     27        (WebCore::RenderImageResourceStyleImage::shutdown):
     28
    1292017-12-13  Ryosuke Niwa  <rniwa@webkit.org>
    230
  • trunk/Source/WebCore/rendering/RenderImageResource.cpp

    r224537 r225872  
    4848    ASSERT(!m_renderer);
    4949    ASSERT(!m_cachedImage);
    50     m_renderer = &renderer;
     50    m_renderer = makeWeakPtr(renderer);
    5151    m_cachedImage = styleCachedImage;
    5252    m_cachedImageRemoveClientIsNeeded = !styleCachedImage;
     
    6666    ASSERT(m_renderer);
    6767    if (m_cachedImage && m_cachedImageRemoveClientIsNeeded)
    68         m_cachedImage->removeClient(*m_renderer);
     68        m_cachedImage->removeClient(*renderer());
    6969    m_cachedImage = newImage;
    7070    m_cachedImageRemoveClientIsNeeded = true;
     
    7272        return;
    7373
    74     m_cachedImage->addClient(*m_renderer);
     74    m_cachedImage->addClient(*renderer());
    7575    if (m_cachedImage->errorOccurred())
    76         m_renderer->imageChanged(m_cachedImage.get());
     76        renderer()->imageChanged(m_cachedImage.get());
    7777}
    7878
     
    8585    image()->resetAnimation();
    8686
    87     if (!m_renderer->needsLayout())
    88         m_renderer->repaint();
     87    if (!renderer()->needsLayout())
     88        renderer()->repaint();
    8989}
    9090
     
    9393    if (!m_cachedImage)
    9494        return &Image::nullImage();
    95     if (auto image = m_cachedImage->imageForRenderer(m_renderer))
     95    if (auto image = m_cachedImage->imageForRenderer(renderer()))
    9696        return image;
    9797    return &Image::nullImage();
     
    103103        return;
    104104    ASSERT(m_renderer);
    105     m_cachedImage->setContainerContextForClient(*m_renderer, imageContainerSize, m_renderer->style().effectiveZoom(), imageURL);
     105    m_cachedImage->setContainerContextForClient(*renderer(), imageContainerSize, renderer()->style().effectiveZoom(), imageURL);
    106106}
    107107
     
    110110    if (!m_cachedImage)
    111111        return LayoutSize();
    112     LayoutSize size = m_cachedImage->imageSizeForRenderer(m_renderer, multiplier, type);
    113     if (is<RenderImage>(m_renderer))
    114         size.scale(downcast<RenderImage>(*m_renderer).imageDevicePixelRatio());
     112    LayoutSize size = m_cachedImage->imageSizeForRenderer(renderer(), multiplier, type);
     113    if (is<RenderImage>(renderer()))
     114        size.scale(downcast<RenderImage>(*renderer()).imageDevicePixelRatio());
    115115    return size;
    116116}
  • trunk/Source/WebCore/rendering/RenderImageResource.h

    r224537 r225872  
    3030#include "StyleImage.h"
    3131#include <wtf/IsoMalloc.h>
     32#include <wtf/WeakPtr.h>
    3233
    3334namespace WebCore {
     
    6465
    6566protected:
    66     RenderElement* renderer() const { return m_renderer; }
     67    RenderElement* renderer() const { return m_renderer.get(); }
    6768    void initialize(RenderElement&, CachedImage*);
    6869   
     
    7071    virtual LayoutSize imageSize(float multiplier, CachedImage::SizeType) const;
    7172
    72     RenderElement* m_renderer { nullptr };
     73    WeakPtr<RenderElement> m_renderer;
    7374    CachedResourceHandle<CachedImage> m_cachedImage;
    7475    bool m_cachedImageRemoveClientIsNeeded { true };
  • trunk/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp

    r224537 r225872  
    5151void RenderImageResourceStyleImage::shutdown()
    5252{
    53     ASSERT(renderer());
    5453    RenderImageResource::shutdown();
    55     m_styleImage->removeClient(renderer());
     54    if (renderer())
     55        m_styleImage->removeClient(renderer());
    5656}
    5757
Note: See TracChangeset for help on using the changeset viewer.