[Webkit-unassigned] [Bug 134398] Flush throttling with remote layers

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #234017|review?                     |review+
               Flag|                            |




--- Comment #7 from Darin Adler <darin at apple.com>  2014-06-27 14:31:52 PST ---
(From update of attachment 234017)
View in context: https://bugs.webkit.org/attachment.cgi?id=234017&action=review

> Source/WebCore/loader/FrameLoader.cpp:3413
>  void FrameLoader::loadProgressingStatusChanged()
>  {
> -    FrameView* view = m_frame.mainFrame().view();
> -    if (!view)
> -        return;
> -
> -    view->updateLayerFlushThrottlingInAllFrames();
> -    view->adjustTiledBackingCoverage();
> +    if (auto* view = m_frame.mainFrame().view())
> +        view->loadProgressingStatusChanged();
>  }

Could the callers work directly on FrameView instead of calling through FrameLoader?

> Source/WebCore/page/FrameView.cpp:2276
> -    bool isMainLoadProgressing = frame().page()->progress().isMainLoadProgressing();
> +    ASSERT(frame().isMainFrame());
> +
> +    bool mainLoadProgressing = frame().page()->progress().isMainLoadProgressing();

I think the old name with "is" is better.

> Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:259
> +    static const auto initialFlushDelay = 500_ms;
> +    static const auto flushDelay = 1500_ms;

I don’t think the “static” here are helpful.

> Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:284
> +    // Re-schedule the flush if we stopped throttling.
> +    if (!wasThrottlingLayerFlushes || m_isThrottlingLayerFlushes)
> +        return true;
> +    if (!m_layerFlushTimer.isActive())
> +        return true;
> +    m_layerFlushTimer.stop();
> +
> +    scheduleCompositingLayerFlush();

I think we could rewrite this to match the comment better:

    if (wasThrottlingLayerFlushes && !m_isThrottlingLayerFlushes && m_layerFlushTimer.isActive()) {
        m_layerFlushTimer.stop();
        scheduleCompositingLayerFlush();
    }

Seems clearer to me than the early return version.

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