Bug 211265 - Books sometimes ends up with blank pages, especially after adjusting font size
Summary: Books sometimes ends up with blank pages, especially after adjusting font size
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-30 16:29 PDT by Tim Horton
Modified: 2020-05-01 21:57 PDT (History)
10 users (show)

See Also:


Attachments
Patch (36.98 KB, patch)
2020-04-30 16:30 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (36.98 KB, patch)
2020-04-30 16:33 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (37.03 KB, patch)
2020-04-30 18:37 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (37.09 KB, patch)
2020-04-30 19:05 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (37.62 KB, patch)
2020-05-01 17:20 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (37.86 KB, patch)
2020-05-01 17:25 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2020-04-30 16:29:58 PDT
Books sometimes ends up with blank pages, especially after adjusting font size
Comment 1 Tim Horton 2020-04-30 16:30:45 PDT
Created attachment 398118 [details]
Patch
Comment 2 Tim Horton 2020-04-30 16:30:47 PDT
<rdar://problem/59898144>
Comment 3 Tim Horton 2020-04-30 16:33:32 PDT
Created attachment 398120 [details]
Patch
Comment 4 Tim Horton 2020-04-30 18:37:03 PDT
Created attachment 398134 [details]
Patch
Comment 5 Tim Horton 2020-04-30 19:05:27 PDT
Created attachment 398138 [details]
Patch
Comment 6 Simon Fraser (smfr) 2020-04-30 20:33:40 PDT
Comment on attachment 398138 [details]
Patch

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

> Source/WebKit/ChangeLog:11
> +        - There is a window during page creation where a WKWebView created

Maybe don't use the term "window" here.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:518
> +    if (auto exposedRect = frameView.viewExposedRectInContentCoordinates())
> +        visibleRect.intersect(*exposedRect);

This doesn't seem right. You're intersecting a rect affected by scrolling with another that is not, and we want to start layer flushing above the layer that scrolls, I think.

> Source/WebKit/UIProcess/WebPageProxy.cpp:3786
> +    if (m_drawingArea)
> +        m_drawingArea->didChangeViewExposedRect();

Should this only be called when it changed?

> Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:326
> +    if (auto exposedRect = m_webPage.mainFrameView()->viewExposedRectInContentCoordinates())
> +        visibleRect.intersect(*exposedRect);

Same dubious scrolled vs non-scrolled rects.

> Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:469
> +        if (auto exposedRect = m_webPage.mainFrameView()->viewExposedRectInContentCoordinates())
> +            visibleRect.intersect(*exposedRect);

Here too.
Comment 7 Tim Horton 2020-04-30 21:53:43 PDT
Super confused now, because the rects that are being intersected are not different... just the timing of the transformation is different. Previously, DrawingArea did the -> contents transform before setting the rect on FrameView.
Comment 8 Tim Horton 2020-04-30 21:55:15 PDT
(In reply to Tim Horton from comment #7)
> Super confused now, because the rects that are being intersected are not
> different... just the timing of the transformation is different. Previously,
> DrawingArea did the -> contents transform before setting the rect on
> FrameView.

Maybe it's always been wrong but all clients of clips-to-visible also expand the view so that there's not actually any WebCore-visible scrolling?
Comment 9 Tim Horton 2020-04-30 21:55:38 PDT
Gonna need some test apps I guess (or to expand the existing layout tests).
Comment 10 Tim Horton 2020-05-01 16:51:11 PDT
(In reply to Tim Horton from comment #9)
> Gonna need some test apps I guess (or to expand the existing layout tests).

You are totally right (it was always broken), and it just didn't have any impact in any clients.
Comment 11 Tim Horton 2020-05-01 17:20:01 PDT
Created attachment 398263 [details]
Patch
Comment 12 Tim Horton 2020-05-01 17:25:40 PDT
Created attachment 398264 [details]
Patch
Comment 13 Darin Adler 2020-05-01 18:16:23 PDT
Comment on attachment 398264 [details]
Patch

Obviously Simon should review again, too, but this looks great.
Comment 14 Simon Fraser (smfr) 2020-05-01 19:59:33 PDT
Comment on attachment 398264 [details]
Patch

r+
Comment 15 EWS 2020-05-01 21:57:17 PDT
Committed r261044: <https://trac.webkit.org/changeset/261044>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398264 [details].