Summary: | REGRESSION: Frames stop appearing after browsing for a while | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||
Component: | Frames | Assignee: | Alexey Proskuryakov <ap> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | simon.fraser, webkit.review.bot | ||||
Priority: | P1 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Alexey Proskuryakov
2010-01-13 22:31:10 PST
Created attachment 46540 [details]
proposed fix
Attachment 46540 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/page/Page.h:135: More than one command on the same line [whitespace/newline] [4]
WebCore/page/Page.h:136: More than one command on the same line [whitespace/newline] [4]
Total errors found: 2
Comment on attachment 46540 [details]
proposed fix
r+
(fun layout test)
Committed <http://trac.webkit.org/changeset/53274>. Comment on attachment 46540 [details] proposed fix > +#if !ASSERT_DISABLED > +void Page::checkFrameCountConsistency() const > +{ > + ASSERT(m_frameCount >= 0); > + > + int frameCount = 0; > + for (Frame* frame = mainFrame(); frame; frame = frame->tree()->traverseNext()) > + ++frameCount; > + > + ASSERT(m_frameCount + 1 == frameCount); > +} > +#endif > } // namespace WebCore Seems to me it should be m_subframeCount, since the main frame is not included in the count. Missing blank line here too. > +#if ASSERT_DISABLED > + void checkFrameCountConsistency() const { } > +#else > + void checkFrameCountConsistency() const; > +#endif I normally prefer to keep the #if out of the class definition in a case like this, using a separate inline function definition later in the header. > Seems to me it should be m_subframeCount, since the main frame is not included
> in the count.
It definitely should! Brady and me agreed that this change shouldn't be part of this patch though.
Renamed to subframe count in http://trac.webkit.org/changeset/129707 |