Bug 144601 – RenderWidget::setWidgetGeometry() can end up destroying *this*.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

Refcount RenderWidget so that we don't end up with an invalid *this* during layout.

[zalan]

rdar://problem/20753994

[zalan]

Created attachment 252342 [details]
Patch

[Andreas Kling]

Comment on attachment 252342 [details]
Patch

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

> Source/WebCore/rendering/RenderView.cpp:361
> +    releaseProtectedRenderWidgets();

Let's move this to FrameView instead so it also works for subtree layouts.

> Source/WebCore/rendering/RenderWidget.h:77
> +    inline void ref() { ++m_refCount; }

No need to specify "inline" here.

[Andreas Kling]

Comment on attachment 252342 [details]
Patch

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

> Source/WebCore/rendering/RenderView.cpp:361
> +    releaseProtectedRenderWidgets();

Let's move this to FrameView instead so it also works for subtree layouts.

> Source/WebCore/rendering/RenderWidget.h:77
> +    inline void ref() { ++m_refCount; }

No need to specify "inline" here.

[zalan]

Created attachment 252350 [details]
Patch

[Andreas Kling]

Comment on attachment 252350 [details]
Patch

r=me

[Chris Dumez]

Comment on attachment 252350 [details]
Patch

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

> Source/WebCore/rendering/RenderObject.cpp:2028
> +    if (is<RenderWidget>(this)) {

if (is<RenderWidget>(*this)) to avoid unnecessary null check

> Source/WebCore/rendering/RenderObject.cpp:2029
> +        downcast<RenderWidget>(this)->deref();

We usually do: downcast<RenderWidget>(*this).deref();

[zalan]

Created attachment 252351 [details]
Patch

[zalan]

Created attachment 252353 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 252353 [details]
Patch

Clearing flags on attachment: 252353

Committed r183788: <http://trac.webkit.org/changeset/183788>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

I’m disappointed that we have to add back the reference counting here. And doubly disappointed that we are not using the RefCounted template for the reference counting.