Facebook in tiled fullscreen is slow

• Component: New Bugs
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Facebook in tiled fullscreen is slow

[Simon Fraser (smfr)]

Created attachment 257154 [details]
Patch

[Simon Fraser (smfr)]

rdar://problem/21823349

[Said Abou-Hallawa]

Comment on attachment 257154 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:291
> +    if (!documentWidthChanged && !viewWidthChanged)

why the check for m_webPage.mainFrame()->view()->needsLayout() was removed?

> Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:300
> +    if (m_isScalingViewToFitDocument && !documentWidthChanged) {

Does m_isScalingViewToFitDocument need to be a member of the class?

> Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:336
> +        IntSize fixedLayoutSize(documentWidth, std::ceil((m_webPage.size().height() - m_webPage.corePage()->topContentInset()) / viewScale));

This block is very similar to the above one, can't the code be refactored such that the same code is not repeated?

[Tim Horton]

Comment on attachment 257154 [details]
Patch

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

> Source/WebKit2/ChangeLog:13
> +        This happened because the the fixed layout size was computed using ceil(m_webPage.size().width() / viewScale)

Do the scrollbars flicker all weirdly now when resizing a simple page inside the scaling region?

>> Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:291
>> +    if (!documentWidthChanged && !viewWidthChanged)
> 
> why the check for m_webPage.mainFrame()->view()->needsLayout() was removed?

Because smfr added a layoutIfNeeded above, so it will always be true.

>> Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:300
>> +    if (m_isScalingViewToFitDocument && !documentWidthChanged) {
> 
> Does m_isScalingViewToFitDocument need to be a member of the class?

Yes? And it already is.

[Simon Fraser (smfr)]

(In reply to comment #4)
> Comment on attachment 257154 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257154&action=review
> 
> > Source/WebKit2/ChangeLog:13
> > +        This happened because the the fixed layout size was computed using ceil(m_webPage.size().width() / viewScale)
> 
> Do the scrollbars flicker all weirdly now when resizing a simple page inside
> the scaling region?

Not any worse than without the patch.
> > Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:336
> > +        IntSize fixedLayoutSize(documentWidth, std::ceil((m_webPage.size().height() - m_webPage.corePage()->topContentInset()) / viewScale));
> 
> This block is very similar to the above one, can't the code be refactored
> such that the same code is not repeated?

I tried to use a lambda and it wasn't clean.

[Simon Fraser (smfr)]

https://trac.webkit.org/r187054