Bug 148309

Summary: Add RELEASE_ASSERT to RenderView's renderer counter.
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: NEW ---    
Severity: Normal CC: buildbot, commit-queue, esprehn+autocc, glenn, kondapallykalyan, rniwa, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews106 for mac-mavericks-wk2 none

Description zalan 2015-08-21 09:48:14 PDT
To check if we ever go below 0.
Comment 1 zalan 2015-08-21 09:50:36 PDT
Created attachment 259621 [details]
Patch
Comment 2 Simon Fraser (smfr) 2015-08-21 09:58:55 PDT
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?
Comment 3 zalan 2015-08-21 10:01:28 PDT
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.
Comment 4 zalan 2015-08-21 10:03:35 PDT
(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.
Comment 5 zalan 2015-08-21 10:14:52 PDT
Created attachment 259624 [details]
Patch
Comment 6 Build Bot 2015-08-21 10:31:13 PDT
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.
Comment 7 Build Bot 2015-08-21 10:31:16 PDT
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
Comment 8 zalan 2015-08-21 20:46:38 PDT
(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