Bug 189681

Summary: "DidFirstVisuallyNonEmptyLayout" callback does not get called when restoring a page from PageCache
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Layout and RenderingAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, ews-watchlist, rniwa, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews100 for mac-sierra
none
Archive of layout-test-results from ews112 for mac-sierra
none
Archive of layout-test-results from ews105 for mac-sierra-wk2
none
Patch
none
Patch none

Description Chris Dumez 2018-09-17 14:47:05 PDT
"DidFirstNonVisuallyEmptyLayout" callback does not get called when restoring a page from PageCache.
Comment 1 Chris Dumez 2018-09-17 14:47:21 PDT
<rdar://problem/44526171>
Comment 2 Chris Dumez 2018-09-17 14:53:05 PDT
Created attachment 349943 [details]
Patch
Comment 3 Chris Dumez 2018-09-17 14:55:33 PDT
Created attachment 349944 [details]
Patch
Comment 4 zalan 2018-09-17 14:57:16 PDT
Comment on attachment 349943 [details]
Patch

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

> Source/WebCore/history/CachedFrame.cpp:159
> +    m_view->clear();

This might reset a bit more than what we actually need (and should).
Comment 5 EWS Watchlist 2018-09-17 15:40:46 PDT
Comment on attachment 349944 [details]
Patch

Attachment 349944 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9248396

New failing tests:
compositing/iframes/page-cache-layer-tree.html
Comment 6 EWS Watchlist 2018-09-17 15:40:48 PDT
Created attachment 349955 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 7 EWS Watchlist 2018-09-17 16:38:00 PDT
Comment on attachment 349944 [details]
Patch

Attachment 349944 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9248802

New failing tests:
compositing/iframes/page-cache-layer-tree.html
Comment 8 EWS Watchlist 2018-09-17 16:38:02 PDT
Created attachment 349969 [details]
Archive of layout-test-results from ews112 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 9 Chris Dumez 2018-09-17 16:47:34 PDT
(In reply to Build Bot from comment #7)
> Comment on attachment 349944 [details]
> Patch
> 
> Attachment 349944 [details] did not pass mac-debug-ews (mac):
> Output: https://webkit-queues.webkit.org/results/9248802
> 
> New failing tests:
> compositing/iframes/page-cache-layer-tree.html

--- /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/layout-test-results/compositing/iframes/page-cache-layer-tree-expected.txt
+++ /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/layout-test-results/compositing/iframes/page-cache-layer-tree-actual.txt
@@ -24,7 +24,7 @@
               (children 1
                 (GraphicsLayer
                   (anchor 0.00 0.00)
-                  (bounds 285.00 135.00)
+                  (bounds 285.00 150.00)
                   (children 1
                     (GraphicsLayer
                       (children 1
@@ -74,7 +74,7 @@
                   (children 1
                     (GraphicsLayer
                       (anchor 0.00 0.00)
-                      (bounds 285.00 135.00)
+                      (bounds 285.00 150.00)
                       (children 1
                         (GraphicsLayer
                           (children 1
Comment 10 EWS Watchlist 2018-09-17 16:51:21 PDT
Comment on attachment 349944 [details]
Patch

Attachment 349944 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9249478

New failing tests:
compositing/iframes/page-cache-layer-tree.html
fast/scrolling/iframe-scrollable-after-back.html
Comment 11 EWS Watchlist 2018-09-17 16:51:22 PDT
Created attachment 349973 [details]
Archive of layout-test-results from ews105 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 12 Chris Dumez 2018-09-17 16:58:19 PDT
(In reply to Chris Dumez from comment #9)
> (In reply to Build Bot from comment #7)
> > Comment on attachment 349944 [details]
> > Patch
> > 
> > Attachment 349944 [details] did not pass mac-debug-ews (mac):
> > Output: https://webkit-queues.webkit.org/results/9248802
> > 
> > New failing tests:
> > compositing/iframes/page-cache-layer-tree.html
> 
> ---
> /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/layout-test-results/
> compositing/iframes/page-cache-layer-tree-expected.txt
> +++
> /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug/layout-test-results/
> compositing/iframes/page-cache-layer-tree-actual.txt
> @@ -24,7 +24,7 @@
>                (children 1
>                  (GraphicsLayer
>                    (anchor 0.00 0.00)
> -                  (bounds 285.00 135.00)
> +                  (bounds 285.00 150.00)
>                    (children 1
>                      (GraphicsLayer
>                        (children 1
> @@ -74,7 +74,7 @@
>                    (children 1
>                      (GraphicsLayer
>                        (anchor 0.00 0.00)
> -                      (bounds 285.00 135.00)
> +                      (bounds 285.00 150.00)
>                        (children 1
>                          (GraphicsLayer
>                            (children 1

The difference is that the scrollbars are gone, likely due to the setScrollbarsSuppressed(true) call in FrameView::clear().

I guess I will only reset the layout milestone related flags.
Comment 13 Chris Dumez 2018-09-17 17:00:19 PDT
Created attachment 349974 [details]
Patch
Comment 14 zalan 2018-09-17 17:20:04 PDT
Comment on attachment 349974 [details]
Patch

Please replace the relevant variables in FrameView::reset() with the resetLayoutMilestones() call.
Comment 15 Chris Dumez 2018-09-17 18:21:44 PDT
(In reply to zalan from comment #14)
> Comment on attachment 349974 [details]
> Patch
> 
> Please replace the relevant variables in FrameView::reset() with the
> resetLayoutMilestones() call.

Good idea, will do.
Comment 16 Chris Dumez 2018-09-18 08:43:17 PDT
Created attachment 350022 [details]
Patch
Comment 17 WebKit Commit Bot 2018-09-18 09:06:17 PDT
Comment on attachment 350022 [details]
Patch

Clearing flags on attachment: 350022

Committed r236142: <https://trac.webkit.org/changeset/236142>
Comment 18 WebKit Commit Bot 2018-09-18 09:06:19 PDT
All reviewed patches have been landed.  Closing bug.