Bug 134117

Summary: Unreproducible crashes under WebCore::ScrollingTree::updateTreeFromStateNode() from messaging a deleted Obj-C object
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Layout and RenderingAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, cmarcelo, commit-queue, gyuyoung.kim, jamesr, luiz, simon.fraser, thorton, tonikitoo
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch simon.fraser: review+, simon.fraser: commit-queue-

Description Beth Dakin 2014-06-20 13:28:32 PDT
There are many unreproducible crashes under WebCore::ScrollingTree::updateTreeFromStateNode() from messaging a deleted Obj-C object. We suspect that we could fix this if ScrollingStateNodes retained their LayerRepresentations. 

<rdar://problem/17149252>
Comment 1 Beth Dakin 2014-06-20 13:32:15 PDT
Created attachment 233446 [details]
Patch
Comment 2 Tim Horton 2014-06-20 13:35:36 PDT
Comment on attachment 233446 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=233446&action=review

> Source/WebCore/page/scrolling/ScrollingStateNode.h:179
> -        PlatformLayer *m_platformLayer;
> +        PlatformLayer* m_platformLayer;

the star was already on the right side
Comment 3 Beth Dakin 2014-06-20 13:47:25 PDT
Created attachment 233447 [details]
Patch
Comment 4 Simon Fraser (smfr) 2014-06-20 14:05:08 PDT
Comment on attachment 233447 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=233447&action=review

> Source/WebCore/page/scrolling/ScrollingStateNode.h:88
> +    LayerRepresentation(const LayerRepresentation& other)
> +        : m_platformLayer(other.m_platformLayer)
> +        , m_layerID(other.m_layerID)
> +        , m_representation(other.m_representation)

This fails to assign m_graphicsLayer.

> Source/WebCore/page/scrolling/ScrollingStateNode.h:133
> +    LayerRepresentation& operator=(const LayerRepresentation& other)
> +    {
> +        m_platformLayer = other.m_platformLayer;
> +        m_layerID = other.m_layerID;
> +        m_representation = other.m_representation;
> +
> +        if (m_representation == PlatformLayerRepresentation)
> +            retainPlatformLayer(m_platformLayer);
> +
> +        return *this;
> +    }

This fails to copy m_graphicsLayer.
Comment 5 Beth Dakin 2014-06-20 16:44:55 PDT
Thanks, Simon! http://trac.webkit.org/changeset/170224
Comment 6 Gyuyoung Kim 2014-06-20 22:14:53 PDT
(In reply to comment #5)
> Thanks, Simon! http://trac.webkit.org/changeset/170224

I add retainPlatformLayer() and releasePlatformLayer() to EFL port in order to fix a build break since r170224.

http://trac.webkit.org/changeset/170229