[webkit-reviews] review granted: [Bug 185627] Pause display links when window is not visible. : [Attachment 340361] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 185627: Pause display links when window is not visible.
https://bugs.webkit.org/show_bug.cgi?id=185627

Attachment 340361: Patch

https://bugs.webkit.org/attachment.cgi?id=340361&action=review




--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 340361
  --> https://bugs.webkit.org/attachment.cgi?id=340361
Patch

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

> Source/WebKit/UIProcess/WebPageProxy.cpp:1602
> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
> +	       if (m_displayLink)
> +		   m_displayLink->pause();
> +#endif

Should we use this display list machinery on all OSes, not just new ones?

> Source/WebKit/UIProcess/mac/DisplayLink.cpp:96
> +bool DisplayLink::pause()
> +{
> +    if (!CVDisplayLinkIsRunning(m_displayLink))
> +	   return true;
> +    return CVDisplayLinkStop(m_displayLink) == kCVReturnSuccess;
> +}
> +
> +bool DisplayLink::resume()
> +{
> +    if (CVDisplayLinkIsRunning(m_displayLink))
> +	   return true;
> +    return CVDisplayLinkStart(m_displayLink) == kCVReturnSuccess;
> +}

I don't think there's any point returning bools here. Your call sites ignore
them; what is the caller supposed to do? Maybe just add an assertion to catch
programmer mistakes.