Attachment 358547 Details for Bug 193215 – Patch

[patch] Patch

bug-193215-20190107155037.patch (text/plain), 8.87 KB, created by Chris Dumez on 2019-01-07 15:50:39 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-07 15:50:39 PST

Size: 8.87 KB

patch

obsolete

>Subversion Revision: 239634
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index a378b23c3f6427b22fd9257c215c2dd99b0a9cae..c8ba0dea77e834e0a8e49a1fcf028814a636127d 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-01-07  Chris Dumez  <cdumez@apple.com>
>+
>+        Cannot scoll for 5 seconds after swiping back on quoteunquoteapps.com
>+        https://bugs.webkit.org/show_bug.cgi?id=193215
>+        <rdar://problem/45108222>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When doing the history navigation, if the source and destination history
>+        items are clones then we will not trigger a main frame load. We may
>+        however trigger loads in subframes if needed. This was an issue for the
>+        ViewGestureController because it was expecting a main frame load after
>+        calling WebPageProxy::goToBackForwardItem() in order to determine when
>+        taking down the view snapshot is appropriate.
>+
>+        To address the problem, the ViewGestureController now takes the snapshot
>+        down as soon as the swipe gesture ends when the source and destination
>+        items are clones.
>+
>+        * Shared/WebBackForwardListItem.cpp:
>+        (WebKit::childItemWithTarget):
>+        (WebKit::WebBackForwardListItem::itemIsInSameDocument const):
>+        (WebKit::hasSameFrames):
>+        (WebKit::WebBackForwardListItem::itemIsClone):
>+        * Shared/WebBackForwardListItem.h:
>+        * UIProcess/ios/ViewGestureControllerIOS.mm:
>+        (WebKit::ViewGestureController::endSwipeGesture):
>+        * UIProcess/mac/ViewGestureControllerMac.mm:
>+        (WebKit::ViewGestureController::endSwipeGesture):
>+
> 2019-01-04  Youenn Fablet  <youenn@apple.com>
> 
>         CSP violation reports should bypass CSP checks
>diff --git a/Source/WebCore/history/HistoryItem.cpp b/Source/WebCore/history/HistoryItem.cpp
>index 0ccfda39376444429193c54833421a418bea007d..44d49c76f930f2f6d8ac846a90519ebd8e5af7f5 100644
>--- a/Source/WebCore/history/HistoryItem.cpp
>+++ b/Source/WebCore/history/HistoryItem.cpp
>@@ -372,6 +372,7 @@ void HistoryItem::clearChildren()
> // - The other item corresponds to the same set of documents, including frames (for history entries created via regular navigation)
> bool HistoryItem::shouldDoSameDocumentNavigationTo(HistoryItem& otherItem) const
> {
>+    // The following logic must be kept in sync with WebKit::WebBackForwardListItem::itemIsInSameDocument().
>     if (this == &otherItem)
>         return false;
> 
>diff --git a/Source/WebCore/loader/HistoryController.cpp b/Source/WebCore/loader/HistoryController.cpp
>index 5f38cda69137e7e1bea6658794a0129e0e92dfc7..5401eea71f6b519501d811e62ec11994271c07b2 100644
>--- a/Source/WebCore/loader/HistoryController.cpp
>+++ b/Source/WebCore/loader/HistoryController.cpp
>@@ -766,6 +766,7 @@ void HistoryController::recursiveGoToItem(HistoryItem& item, HistoryItem* fromIt
>     }
> }
> 
>+// The following logic must be kept in sync with WebKit::WebBackForwardListItem::itemIsClone().
> bool HistoryController::itemsAreClones(HistoryItem& item1, HistoryItem* item2) const
> {
>     // If the item we're going to is a clone of the item we're at, then we do
>diff --git a/Source/WebKit/Shared/WebBackForwardListItem.cpp b/Source/WebKit/Shared/WebBackForwardListItem.cpp
>index 644fffccd02a09e8569ba16023fad3e069645425..03b43f1c97b1eb87ddc0f44153ec828f5ce44498 100644
>--- a/Source/WebKit/Shared/WebBackForwardListItem.cpp
>+++ b/Source/WebKit/Shared/WebBackForwardListItem.cpp
>@@ -77,6 +77,16 @@ static const FrameState* childItemWithDocumentSequenceNumber(const FrameState& f
>     return nullptr;
> }
> 
>+static const FrameState* childItemWithTarget(const FrameState& frameState, const String& target)
>+{
>+    for (const auto& child : frameState.children) {
>+        if (child.target == target)
>+            return &child;
>+    }
>+
>+    return nullptr;
>+}
>+
> static bool documentTreesAreEqual(const FrameState& a, const FrameState& b)
> {
>     if (a.documentSequenceNumber != b.documentSequenceNumber)
>@@ -99,7 +109,7 @@ bool WebBackForwardListItem::itemIsInSameDocument(const WebBackForwardListItem&
>     if (m_pageID != other.m_pageID)
>         return false;
> 
>-    // The following logic must be kept in sync with WebCore::HistoryItem::shouldDoSameDocumentNavigationTo.
>+    // The following logic must be kept in sync with WebCore::HistoryItem::shouldDoSameDocumentNavigationTo().
> 
>     const FrameState& mainFrameState = m_itemState.pageState.mainFrameState;
>     const FrameState& otherMainFrameState = other.m_itemState.pageState.mainFrameState;
>@@ -116,6 +126,38 @@ bool WebBackForwardListItem::itemIsInSameDocument(const WebBackForwardListItem&
>     return documentTreesAreEqual(mainFrameState, otherMainFrameState);
> }
> 
>+static bool hasSameFrames(const FrameState& a, const FrameState& b)
>+{
>+    if (a.target != b.target)
>+        return false;
>+
>+    if (a.children.size() != b.children.size())
>+        return false;
>+
>+    for (const auto& child : a.children) {
>+        if (!childItemWithTarget(b, child.target))
>+            return false;
>+    }
>+
>+    return true;
>+}
>+
>+bool WebBackForwardListItem::itemIsClone(const WebBackForwardListItem& other)
>+{
>+    // The following logic must be kept in sync with WebCore::HistoryItem::itemsAreClones().
>+
>+    if (this == &other)
>+        return false;
>+
>+    const FrameState& mainFrameState = m_itemState.pageState.mainFrameState;
>+    const FrameState& otherMainFrameState = other.m_itemState.pageState.mainFrameState;
>+
>+    if (mainFrameState.itemSequenceNumber != otherMainFrameState.itemSequenceNumber)
>+        return false;
>+
>+    return hasSameFrames(mainFrameState, otherMainFrameState);
>+}
>+
> void WebBackForwardListItem::setSuspendedPage(SuspendedPageProxy* page)
> {
>     if (m_suspendedPage == page)
>diff --git a/Source/WebKit/Shared/WebBackForwardListItem.h b/Source/WebKit/Shared/WebBackForwardListItem.h
>index 3dad9b78d8214328fc7ad6de9c88ab601b331e2f..aaf6514e7a67c36187b11f99d7ad4e7d03efd4f2 100644
>--- a/Source/WebKit/Shared/WebBackForwardListItem.h
>+++ b/Source/WebKit/Shared/WebBackForwardListItem.h
>@@ -64,6 +64,7 @@ public:
>     const String& title() const { return m_itemState.pageState.title; }
> 
>     bool itemIsInSameDocument(const WebBackForwardListItem&) const;
>+    bool itemIsClone(const WebBackForwardListItem&);
> 
> #if PLATFORM(COCOA)
>     ViewSnapshot* snapshot() const { return m_itemState.snapshot.get(); }
>diff --git a/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm b/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm
>index 0d3711fb8d1f26d3c7609fc832c18eb5db5bfcc4..05d6f9924bca249d90f94a48f8d5c4bd695fb0e8 100644
>--- a/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm
>+++ b/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm
>@@ -285,6 +285,21 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
>         return;
>     }
> 
>+    auto* currentItem = m_webPageProxy.backForwardList().currentItem();
>+    bool areItemsClones = currentItem && currentItem->itemIsClone(*targetItem);
>+    // The main frame will not be navigated so hide the snapshot right away.
>+    if (areItemsClones) {
>+        // removeSwipeSnapshot will clear m_webPageProxyForBackForwardListForCurrentSwipe, so hold on to it here.
>+        RefPtr<WebPageProxy> webPageProxyForBackForwardListForCurrentSwipe = m_webPageProxyForBackForwardListForCurrentSwipe;
>+        removeSwipeSnapshot();
>+        webPageProxyForBackForwardListForCurrentSwipe->navigationGestureDidEnd(true, *targetItem);
>+        if (&m_webPageProxy != m_webPageProxyForBackForwardListForCurrentSwipe)
>+            m_webPageProxy.navigationGestureDidEnd();
>+
>+        m_webPageProxyForBackForwardListForCurrentSwipe->goToBackForwardItem(*targetItem);
>+        return;
>+    }
>+
>     m_snapshotRemovalTargetRenderTreeSize = 0;
>     if (ViewSnapshot* snapshot = targetItem->snapshot())
>         m_snapshotRemovalTargetRenderTreeSize = snapshot->renderTreeSize() * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
>diff --git a/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm b/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm
>index cc44f8b3b6c2a615707d7045063ab0b83ca00e2a..5e44c44fb94592cd446c6c519871931a4bb8527e 100644
>--- a/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm
>+++ b/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm
>@@ -734,6 +734,16 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
>         return;
>     }
> 
>+    auto* currentItem = m_webPageProxy.backForwardList().currentItem();
>+    bool areItemsClones = currentItem && currentItem->itemIsClone(*targetItem);
>+    // The main frame will not be navigated so hide the snapshot right away.
>+    if (areItemsClones) {
>+        removeSwipeSnapshot();
>+        m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
>+        m_webPageProxy.goToBackForwardItem(*targetItem);
>+        return;
>+    }
>+
>     uint64_t renderTreeSize = 0;
>     if (ViewSnapshot* snapshot = targetItem->snapshot())
>         renderTreeSize = snapshot->renderTreeSize();

Actions: View | Formatted Diff | Diff

Attachments on bug 193215: 358536 | 358547 | 358563 | 358567