Bug 148309 – Add RELEASE_ASSERT to RenderView's renderer counter.

• Component: Layout and Rendering
• Status: NEW

[alan]

To check if we ever go below 0.

[alan]

Created attachment 259621 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 259621 [details]
Patch

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

> Source/WebCore/rendering/RenderObject.cpp:134
> +    if (!m_node.isDocumentNode())
> +        view().didDestroyRenderer();

Seems a shame to hurt runtime with a !m_node.isDocumentNode() check here (on every renderer!), for an assert which I don't think would catch any bug that we know about?

[alan]

Comment on attachment 259621 [details]
Patch

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

> Source/WebCore/rendering/RenderObject.cpp:135
>      ASSERT(!hasRareData());

This is about balancing the didCreate/didDestroy calls and probably should be fixed regardless of whether we end up asserting on m_rendererCount.

[alan]

(In reply to comment #3)
> Comment on attachment 259621 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=259621&action=review
> 
> > Source/WebCore/rendering/RenderObject.cpp:135
> >      ASSERT(!hasRareData());
> 
> This is about balancing the didCreate/didDestroy calls and probably should
> be fixed regardless of whether we end up asserting on m_rendererCount.
Optionally we could remove these checks from both the c'tor and the d'tor.

[alan]

Created attachment 259624 [details]
Patch

[Build Bot]

Comment on attachment 259624 [details]
Patch

Attachment 259624 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/86335

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 259628 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[alan]

(In reply to comment #2)
> Comment on attachment 259621 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=259621&action=review
> 
> > Source/WebCore/rendering/RenderObject.cpp:134
> > +    if (!m_node.isDocumentNode())
> > +        view().didDestroyRenderer();
> 
> Seems a shame to hurt runtime with a !m_node.isDocumentNode() check here (on
> every renderer!), for an assert which I don't think would catch any bug that
> we know about?
Actually it already caught this one bug 148352