Bug 211265

Summary: Books sometimes ends up with blank pages, especially after adjusting font size
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, esprehn+autocc, ews-watchlist, fred.wang, glenn, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Tim Horton
Reported 2020-04-30 16:29:58 PDT
Books sometimes ends up with blank pages, especially after adjusting font size
Attachments
Patch (36.98 KB, patch)
2020-04-30 16:30 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (36.98 KB, patch)
2020-04-30 16:33 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (37.03 KB, patch)
2020-04-30 18:37 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (37.09 KB, patch)
2020-04-30 19:05 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (37.62 KB, patch)
2020-05-01 17:20 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (37.86 KB, patch)
2020-05-01 17:25 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Tim Horton
Comment 1 2020-04-30 16:30:45 PDT
Created attachment 398118 [details] Patch
Tim Horton
Comment 2 2020-04-30 16:30:47 PDT
<rdar://problem/59898144>
Tim Horton
Comment 3 2020-04-30 16:33:32 PDT
Created attachment 398120 [details] Patch
Tim Horton
Comment 4 2020-04-30 18:37:03 PDT
Created attachment 398134 [details] Patch
Tim Horton
Comment 5 2020-04-30 19:05:27 PDT
Created attachment 398138 [details] Patch
Simon Fraser (smfr)
Comment 6 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.
Tim Horton
Comment 7 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.
Tim Horton
Comment 8 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?
Tim Horton
Comment 9 2020-04-30 21:55:38 PDT
Gonna need some test apps I guess (or to expand the existing layout tests).
Tim Horton
Comment 10 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.
Tim Horton
Comment 11 2020-05-01 17:20:01 PDT
Created attachment 398263 [details] Patch
Tim Horton
Comment 12 2020-05-01 17:25:40 PDT
Created attachment 398264 [details] Patch
Darin Adler
Comment 13 2020-05-01 18:16:23 PDT
Comment on attachment 398264 [details] Patch Obviously Simon should review again, too, but this looks great.
Simon Fraser (smfr)
Comment 14 2020-05-01 19:59:33 PDT
Comment on attachment 398264 [details] Patch r+
EWS
Comment 15 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].
Note You need to log in before you can comment on or make changes to this bug.