[Webkit-unassigned] [Bug 131696] Only webkit2 pages should have a page throttler.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=131696


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #229404|review?                     |review-
               Flag|                            |




--- Comment #2 from Darin Adler <darin at apple.com>  2014-04-16 10:41:10 PST ---
(From update of attachment 229404)
View in context: https://bugs.webkit.org/attachment.cgi?id=229404&action=review

> Source/WebCore/page/Page.cpp:1116
> +    m_pageThrottler = PageThrottler::create(*this, m_viewState);

Would be nice to assert !m_pageThrottler here. And it should be using make_unique instead of reference counting:

    m_pageThrottler = std::make_unique<PageThrottler>(*this, m_viewState);

> Source/WebCore/page/Page.h:388
> +    void initializePageThrottler();

I think this function should be called createPageThrottler. “initialize” is something you do to something that already exists.

> Source/WebCore/page/Page.h:545
> +    RefPtr<PageThrottler> m_pageThrottler;

Should be std::unique_ptr<PageThrottler>.

> Source/WebCore/page/PageThrottler.cpp:35
> +#include <wtf/text/CString.h>

Why was this needed?

> Source/WebCore/page/PageThrottler.cpp:42
> +PassRefPtr<PageThrottler> PageThrottler::create(Page& page, ViewState::Flags viewState)
> +{
> +     return adoptRef(new PageThrottler(page, viewState));
> +}

Don’t need this.

> Source/WebCore/page/PageThrottler.cpp:102
> -
> +    

Please don’t add this whitespace.

> Source/WebCore/page/PageThrottler.h:44
> +class PageThrottler : public RefCounted<PageThrottler> {

Reference counting does not seem appropriate here; there’s no need for sharing. Lets use std::unique_ptr and have this be single ownership. We should just leave this class alone entirely, and use make_unique to create it.

> Source/WebCore/page/Settings.cpp:661
> +    if (m_page->pageThrottler())
> +    m_page->pageThrottler()->hiddenPageDOMTimerThrottlingStateChanged();

Need to indent the body of the if statement.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.