Cannot scoll for 5 seconds after swiping back on quoteunquoteapps.com
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 04:27:00 +0000 (04:27 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 04:27:00 +0000 (04:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193215
<rdar://problem/45108222>

Reviewed by Tim Horton.

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239720 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/history/HistoryItem.cpp
Source/WebCore/loader/HistoryController.cpp
Source/WebKit/ChangeLog
Source/WebKit/Shared/WebBackForwardListItem.cpp
Source/WebKit/Shared/WebBackForwardListItem.h
Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm
Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm

index 0ccfda3..44d49c7 100644 (file)
@@ -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;
 
index 5f38cda..5401eea 100644 (file)
@@ -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
index cae5710..340f9b4 100644 (file)
@@ -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 Tim Horton.
+
+        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-07  David Kilzer  <ddkilzer@apple.com>
 
         Leak of ScrollCompletionCallbackData (16 bytes) in com.apple.WebKit.WebContent running WebKit layout tests
index 644fffc..03b43f1 100644 (file)
@@ -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)
index 3dad9b7..aaf6514 100644 (file)
@@ -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(); }
index 0d3711f..93319d2 100644 (file)
@@ -300,6 +300,13 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
         return;
     }
 
+    auto* currentItem = m_webPageProxyForBackForwardListForCurrentSwipe->backForwardList().currentItem();
+    // The main frame will not be navigated so hide the snapshot right away.
+    if (currentItem && currentItem->itemIsClone(*targetItem)) {
+        removeSwipeSnapshot();
+        return;
+    }
+
     // FIXME: Should we wait for VisuallyNonEmptyLayout like we do on Mac?
     m_snapshotRemovalTracker.start(SnapshotRemovalTracker::RenderTreeSizeThreshold
         | SnapshotRemovalTracker::RepaintAfterNavigation
index cc44f8b..6405ecd 100644 (file)
@@ -743,6 +743,13 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
     m_webPageProxy.goToBackForwardItem(*targetItem);
 
+    auto* currentItem = m_webPageProxy.backForwardList().currentItem();
+    // The main frame will not be navigated so hide the snapshot right away.
+    if (currentItem && currentItem->itemIsClone(*targetItem)) {
+        removeSwipeSnapshot();
+        return;
+    }
+
     SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
         | SnapshotRemovalTracker::MainFrameLoad
         | SnapshotRemovalTracker::SubresourceLoads