[webkit-reviews] review denied: [Bug 84522] Page::setDefersLoading() should have a call count : [Attachment 138221] Patch

[bugzilla-daemon at webkit.org]

Andy Estes <aestes at apple.com> has denied Jon Honeycutt <jhoneycutt at apple.com>'s
request for review:
Bug 84522: Page::setDefersLoading() should have a call count
https://bugs.webkit.org/show_bug.cgi?id=84522

Attachment 138221: Patch
https://bugs.webkit.org/attachment.cgi?id=138221&action=review

------- Additional Comments from Andy Estes <aestes at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=138221&action=review


As we discussed on IRC, as it stands this'll cause issues with WebKit nightly
builds.

> Source/WebCore/page/Page.cpp:575
> +    ASSERT(defers || m_deferLoadingCallCount > 0);

If you made m_deferLoadingCallCount unsigned you could simplify this
expression.

>> Source/WebCore/page/Page.cpp:576
>> +	if (defers && ++m_deferLoadingCallCount > 1)
> 
> An else if statement should be written as an if statement when the prior "if"
concludes with a return, break, continue or goto statement. 
[readability/control_flow] [4]

Like StyleBot says...

> Source/WebCore/page/Page.cpp:578
> +    else if (!defers && --m_deferLoadingCallCount > 0)

If you made m_deferLoadingCallCount unsigned you could simplify this
expression.

> Source/WebCore/page/Page.h:221
> +	   bool defersLoading() const { return m_deferLoadingCallCount > 0; }

If you made m_deferLoadingCallCount unsigned you could simplify this
expression.

> Source/WebCore/page/Page.h:391
> +	   int m_deferLoadingCallCount;

m_defersLoadingCallCount seems like a better name given the name of the method.