Attachment 373639 Details for Bug 199575 – Patch

[patch] Patch

bug-199575-20190708101208.patch (text/plain), 9.88 KB, created by Wenson Hsieh on 2019-07-08 10:12:08 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-07-08 10:12:08 PDT

Size: 9.88 KB

patch

obsolete

>Subversion Revision: 247162
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index d0b709cf547adfd0e798c726f0153334360aece9..7677c2be3344a5a47842ebb2d0a2e06076b494fa 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-07-08  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iPadOS] Viewport pops while loading sohu.com, xinhuanet.com, and various other websites
>+        https://bugs.webkit.org/show_bug.cgi?id=199575
>+        <rdar://problem/51842220>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Currently, the shrink-to-fit-content heuristic added to scale desktop sites fit within the viewport on iPadOS
>+        only runs during two intervals: after document load, and after page load. On very script-heavy websites, this
>+        may cause a visible jump, as rendering may commence well before the document is finished parsing.
>+
>+        To mitigate this, we move the first opportunity for the shrink-to-fit heuristic from after document, to right
>+        after the page transition has completed (before we unfreeze the layer tree).
>+
>+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
>+        (WebKit::WebFrameLoaderClient::dispatchDidFinishDocumentLoad):
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::didCompletePageTransition):
>+        (WebKit::WebPage::didFinishDocumentLoad): Deleted.
>+
>+        Remove this didFinishDocumentLoad hook, which was only used to schedule the shrink-to-fit timer.
>+
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::immediatelyShrinkToFitContent):
>+
>+        m_mainFrame might be null after page transition completes, so we need to ensure immediatelyShrinkToFitContent is
>+        robust in this scenario.
>+
> 2019-07-05  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r247123.
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>index 8a4444ef3d5599325c411b96ed7e43ae0ddef475..010ab3bc52e4e7e804c74d41cb55de3f6a70134b 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>@@ -614,8 +614,6 @@ void WebFrameLoaderClient::dispatchDidFinishDocumentLoad()
> 
>     // Notify the UIProcess.
>     webPage->send(Messages::WebPageProxy::DidFinishDocumentLoadForFrame(m_frame->frameID(), navigationID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
>-
>-    webPage->didFinishDocumentLoad(*m_frame);
> }
> 
> void WebFrameLoaderClient::dispatchDidFinishLoad()
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index a5c6f4abbc01495027457bcfadf147a74a72ddbb..e776e5ebc3887845c7da1c9123780363201b6b91 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -3270,6 +3270,11 @@ void WebPage::didStartPageTransition()
> 
> void WebPage::didCompletePageTransition()
> {
>+#if ENABLE(VIEWPORT_RESIZING)
>+    if (immediatelyShrinkToFitContent())
>+        viewportConfigurationChanged(ZoomToInitialScale::Yes);
>+#endif
>+
>     unfreezeLayerTree(LayerTreeFreezeReason::PageTransition);
> 
>     RELEASE_LOG_IF_ALLOWED("%p - WebPage - Did complete page transition", this);
>@@ -5795,16 +5800,6 @@ void WebPage::didCommitLoad(WebFrame* frame)
>     updateMockAccessibilityElementAfterCommittingLoad();
> }
> 
>-void WebPage::didFinishDocumentLoad(WebFrame& frame)
>-{
>-    if (!frame.isMainFrame())
>-        return;
>-
>-#if ENABLE(VIEWPORT_RESIZING)
>-    scheduleShrinkToFitContent();
>-#endif
>-}
>-
> void WebPage::didFinishLoad(WebFrame& frame)
> {
>     if (!frame.isMainFrame())
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 4e0ce4a47927d6176f5ba9aaa55e8ad2f443b2ad..1a0fb60655d2d810273bdd55e29629e8ca770c08 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -359,7 +359,6 @@ public:
>     void didCommitLoad(WebFrame*);
>     void willReplaceMultipartContent(const WebFrame&);
>     void didReplaceMultipartContent(const WebFrame&);
>-    void didFinishDocumentLoad(WebFrame&);
>     void didFinishLoad(WebFrame&);
>     void show();
>     String userAgent(const URL&) const;
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 326840231821a83adcffc879e09e08a13b9a023e..a96c652b5e7932d34022ce0bee2f0bab7d62f3bb 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -3317,6 +3317,9 @@ bool WebPage::immediatelyShrinkToFitContent()
>     if (m_isClosed)
>         return false;
> 
>+    if (!m_mainFrame)
>+        return false;
>+
>     if (!m_page->settings().allowViewportShrinkToFitContent())
>         return false;
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index e174552a511871bcc50b16b6bb95fe969033e4aa..a3839d30c110a196fe6df30f275eeb57231d45c6 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-07-08  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iPadOS] Viewport pops while loading sohu.com, xinhuanet.com, and various other websites
>+        https://bugs.webkit.org/show_bug.cgi?id=199575
>+        <rdar://problem/51842220>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Adjusts an existing layout test, and introduces a new test to verify that the shrink-to-fit heuristic is given a
>+        chance to run before document load. See WebKit ChangeLog for more details.
>+
>+        * fast/viewport/ios/shrink-to-fit-content-before-document-load-expected.txt: Added.
>+        * fast/viewport/ios/shrink-to-fit-content-before-document-load.html: Added.
>+        * fast/viewport/ios/shrink-to-fit-content-temporary-overflow.html:
>+
>+        Tweak this layout test to add the temporarily overflowing element after document load.
>+
> 2019-07-07  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Unable to paste from Notes into Excel 365 spreadsheet
>diff --git a/LayoutTests/fast/viewport/ios/shrink-to-fit-content-before-document-load-expected.txt b/LayoutTests/fast/viewport/ios/shrink-to-fit-content-before-document-load-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..6d812f859c2a2be5bf71ee268ccae30feb9dba7d
>--- /dev/null
>+++ b/LayoutTests/fast/viewport/ios/shrink-to-fit-content-before-document-load-expected.txt
>@@ -0,0 +1,11 @@
>+Verifies that the shrink-to-fit heuristic is given a chance to run before document load finishes. This test requires WebKitTestRunner.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS innerWidth is 1280
>+PASS visualViewport.scale.toFixed(2) is (screen.width / 1280).toFixed(2)
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/viewport/ios/shrink-to-fit-content-before-document-load.html b/LayoutTests/fast/viewport/ios/shrink-to-fit-content-before-document-load.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..c7c906da138b09dc4673511b4ca156a2fd5c6def
>--- /dev/null
>+++ b/LayoutTests/fast/viewport/ios/shrink-to-fit-content-before-document-load.html
>@@ -0,0 +1,38 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+<meta name="viewport" content="width=device-width, initial-scale=1">
>+<style>
>+body, html {
>+    margin: 0;
>+    width: 100%;
>+    height: 100%;
>+}
>+
>+#bar {
>+    width: 1280px;
>+    height: 100px;
>+    background: linear-gradient(to right, red 0%, green 50%, blue 100%);
>+}
>+</style>
>+<script src="../../../resources/ui-helper.js"></script>
>+<script src="../../../resources/js-test.js"></script>
>+<script>
>+description("Verifies that the shrink-to-fit heuristic is given a chance to run before document load finishes. This test requires WebKitTestRunner.");
>+
>+addEventListener("DOMContentLoaded", () => {
>+    shouldBe("innerWidth", "1280");
>+    shouldBe("visualViewport.scale.toFixed(2)", "(screen.width / 1280).toFixed(2)");
>+});
>+</script>
>+</head>
>+<body>
>+<div id="bar"></div>
>+<div id="description"></div>
>+<div class="square"></div>
>+<script>
>+document.scrollingElement.scrollTo(0, 1);
>+document.scrollingElement.scrollTop;
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/viewport/ios/shrink-to-fit-content-temporary-overflow.html b/LayoutTests/fast/viewport/ios/shrink-to-fit-content-temporary-overflow.html
>index c86ee1d8e83e2b67c37ba022ae7531c9b24eceae..bfa1db54adae0355b7c9e2f43f3fca4c0fd8d64e 100644
>--- a/LayoutTests/fast/viewport/ios/shrink-to-fit-content-temporary-overflow.html
>+++ b/LayoutTests/fast/viewport/ios/shrink-to-fit-content-temporary-overflow.html
>@@ -48,6 +48,20 @@ jsTestIsAsync = true;
> 
> description("This test verifies that a temporary change in content width does not cause the viewport width to permanently expand to try and accomodate the content. To test manually, load the page and check that the box below reads 'PASS'. This test is only intended to run on devices with less than 1200px screen width.");
> 
>+function forceLayoutByScrollingTo(y) {
>+    document.scrollingElement.scrollTo(0, y);
>+    return document.scrollingElement.scrollTop;
>+}
>+
>+addEventListener("DOMContentLoaded", () => {
>+    forceLayoutByScrollingTo(1);
>+    const bar = document.createElement("div");
>+    bar.classList.add("bar");
>+    document.body.appendChild(bar);
>+    forceLayoutByScrollingTo(0);
>+    bar.remove();
>+});
>+
> addEventListener("load", async () => {
>     if (!window.testRunner)
>         return;
>@@ -62,14 +76,5 @@ addEventListener("load", async () => {
> <body>
> <div id="description"></div>
> <div class="square"></div>
>-<script>
>-const bar = document.createElement("div");
>-bar.classList.add("bar");
>-document.body.appendChild(bar);
>-document.scrollingElement.scrollTo(0, 1);
>-document.scrollingElement.scrollTo(0, 0);
>-document.scrollingElement.scrollTop;
>-bar.remove();
>-</script>
> </body>
> </html>

Actions: View | Formatted Diff | Diff

Attachments on bug 199575: 373639