- Revision
- 239720
- Author
- [email protected]
- Date
- 2019-01-07 20:27:00 -0800 (Mon, 07 Jan 2019)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/history/HistoryItem.cpp (239719 => 239720)
--- trunk/Source/WebCore/history/HistoryItem.cpp 2019-01-08 02:07:04 UTC (rev 239719)
+++ trunk/Source/WebCore/history/HistoryItem.cpp 2019-01-08 04:27:00 UTC (rev 239720)
@@ -372,6 +372,7 @@
// - 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;
Modified: trunk/Source/WebCore/loader/HistoryController.cpp (239719 => 239720)
--- trunk/Source/WebCore/loader/HistoryController.cpp 2019-01-08 02:07:04 UTC (rev 239719)
+++ trunk/Source/WebCore/loader/HistoryController.cpp 2019-01-08 04:27:00 UTC (rev 239720)
@@ -766,6 +766,7 @@
}
}
+// 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
Modified: trunk/Source/WebKit/ChangeLog (239719 => 239720)
--- trunk/Source/WebKit/ChangeLog 2019-01-08 02:07:04 UTC (rev 239719)
+++ trunk/Source/WebKit/ChangeLog 2019-01-08 04:27:00 UTC (rev 239720)
@@ -1,3 +1,33 @@
+2019-01-07 Chris Dumez <[email protected]>
+
+ 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 <[email protected]>
Leak of ScrollCompletionCallbackData (16 bytes) in com.apple.WebKit.WebContent running WebKit layout tests
Modified: trunk/Source/WebKit/Shared/WebBackForwardListItem.cpp (239719 => 239720)
--- trunk/Source/WebKit/Shared/WebBackForwardListItem.cpp 2019-01-08 02:07:04 UTC (rev 239719)
+++ trunk/Source/WebKit/Shared/WebBackForwardListItem.cpp 2019-01-08 04:27:00 UTC (rev 239720)
@@ -77,6 +77,16 @@
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 @@
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 @@
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)
Modified: trunk/Source/WebKit/Shared/WebBackForwardListItem.h (239719 => 239720)
--- trunk/Source/WebKit/Shared/WebBackForwardListItem.h 2019-01-08 02:07:04 UTC (rev 239719)
+++ trunk/Source/WebKit/Shared/WebBackForwardListItem.h 2019-01-08 04:27:00 UTC (rev 239720)
@@ -64,6 +64,7 @@
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(); }
Modified: trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm (239719 => 239720)
--- trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm 2019-01-08 02:07:04 UTC (rev 239719)
+++ trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm 2019-01-08 04:27:00 UTC (rev 239720)
@@ -300,6 +300,13 @@
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
Modified: trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm (239719 => 239720)
--- trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm 2019-01-08 02:07:04 UTC (rev 239719)
+++ trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm 2019-01-08 04:27:00 UTC (rev 239720)
@@ -743,6 +743,13 @@
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
