From 1e2edc0ebbba6f0cf20985eb3dbf075cbb12a79f Mon Sep 17 00:00:00 2001 From: Dan Robertson Date: Sat, 10 Jun 2023 13:29:38 +0000 Subject: [PATCH] Bug 1815713 - Allow pull to refresh from subframes. r=botond,geckoview-reviewers,owlish When a subframe and all of its scroll handoff parents have no room to scroll up, provide a mechanism for the frontend to implement pull-to-refresh. When input targets a subframe that meets the above conditions: For eager results and content that has a related event handler that does not preventDefault, return input "unhandled". This allows pull to refresh to work on mobile when the upwards scroll occurs over a subframe. Differential Revision: https://phabricator.services.mozilla.com/D176559 --- gfx/layers/apz/public/APZInputBridge.h | 3 +- gfx/layers/apz/src/APZInputBridge.cpp | 28 ++++++- gfx/layers/apz/src/AsyncPanZoomController.cpp | 5 ++ gfx/layers/apz/src/AsyncPanZoomController.h | 3 + gfx/layers/apz/src/InputQueue.cpp | 14 +++- gfx/layers/apz/src/OverscrollHandoffState.cpp | 17 ++++ gfx/layers/apz/src/OverscrollHandoffState.h | 3 + .../assets/www/pull-to-refresh-subframe.html | 82 +++++++++++++++++++ .../assets/www/scroll-handoff.html | 7 +- .../mozilla/geckoview/test/BaseSessionTest.kt | 1 + .../geckoview/test/InputResultDetailTest.kt | 6 +- .../geckoview/test/PanZoomControllerTest.kt | 70 ++++++++++++++++ 12 files changed, 228 insertions(+), 11 deletions(-) create mode 100644 mobile/android/geckoview/src/androidTest/assets/www/pull-to-refresh-subframe.html diff --git a/gfx/layers/apz/public/APZInputBridge.h b/gfx/layers/apz/public/APZInputBridge.h index 07ed39548d7f..117e4b7bb27b 100644 --- a/gfx/layers/apz/public/APZInputBridge.h +++ b/gfx/layers/apz/public/APZInputBridge.h @@ -54,7 +54,8 @@ struct APZHandledResult { // an event handler using preventDefault() in the callback, so call sites of // this function should be responsible to set a proper |aPlace|. APZHandledResult(APZHandledPlace aPlace, - const AsyncPanZoomController* aTarget); + const AsyncPanZoomController* aTarget, + bool aPopulateDirectionsForUnhandled = false); APZHandledResult(APZHandledPlace aPlace, SideBits aScrollableDirections, ScrollDirections aOverscrollDirections) : mPlace(aPlace), diff --git a/gfx/layers/apz/src/APZInputBridge.cpp b/gfx/layers/apz/src/APZInputBridge.cpp index bce801a6d240..9b0f21d774a4 100644 --- a/gfx/layers/apz/src/APZInputBridge.cpp +++ b/gfx/layers/apz/src/APZInputBridge.cpp @@ -108,10 +108,27 @@ void APZEventResult::UpdateHandledResult( } if (aTarget && !aTarget->IsRootContent()) { - auto [result, rootApzc] = + // If the event targets a subframe but the subframe and its ancestors + // are all scrolled to the top, we want an upward swipe to allow + // triggering pull-to-refresh. + bool mayTriggerPullToRefresh = + aBlock.GetOverscrollHandoffChain()->ScrollingUpWillTriggerPullToRefresh( + aTarget); + if (mayTriggerPullToRefresh) { + // Similar to what is done for the dynamic toolbar, we need to ensure + // that if the input has the dispatch to content flag, we need to change + // the handled result to Nothing(), so that GeckoView can wait for the + // result. + mHandledResult = (aDispatchToContent) + ? Nothing() + : Some(APZHandledResult{APZHandledPlace::Unhandled, + aTarget, true}); + } + + auto [mayMoveDynamicToolbar, rootApzc] = aBlock.GetOverscrollHandoffChain()->ScrollingDownWillMoveDynamicToolbar( aTarget); - if (result) { + if (mayMoveDynamicToolbar) { MOZ_ASSERT(rootApzc && rootApzc->IsRootContent()); // The event is actually consumed by a non-root APZC but scroll // positions in all relevant APZCs are at the bottom edge, so if there's @@ -339,11 +356,16 @@ APZEventResult APZInputBridge::ReceiveInputEvent( } APZHandledResult::APZHandledResult(APZHandledPlace aPlace, - const AsyncPanZoomController* aTarget) + const AsyncPanZoomController* aTarget, + bool aPopulateDirectionsForUnhandled) : mPlace(aPlace) { MOZ_ASSERT(aTarget); switch (aPlace) { case APZHandledPlace::Unhandled: + if (aPopulateDirectionsForUnhandled) { + mScrollableDirections = aTarget->ScrollableDirections(); + mOverscrollDirections = aTarget->GetAllowedHandoffDirections(); + } break; case APZHandledPlace::HandledByContent: if (aTarget) { diff --git a/gfx/layers/apz/src/AsyncPanZoomController.cpp b/gfx/layers/apz/src/AsyncPanZoomController.cpp index 19062421aa0c..71ba4767f177 100644 --- a/gfx/layers/apz/src/AsyncPanZoomController.cpp +++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp @@ -2339,6 +2339,11 @@ bool AsyncPanZoomController::CanVerticalScrollWithDynamicToolbar() const { return mY.CanVerticalScrollWithDynamicToolbar(); } +bool AsyncPanZoomController::CanOverscrollUpwards() const { + RecursiveMutexAutoLock lock(mRecursiveMutex); + return !mY.CanScrollTo(eSideTop) && mY.OverscrollBehaviorAllowsHandoff(); +} + bool AsyncPanZoomController::CanScrollDownwards() const { RecursiveMutexAutoLock lock(mRecursiveMutex); return mY.CanScrollTo(eSideBottom); diff --git a/gfx/layers/apz/src/AsyncPanZoomController.h b/gfx/layers/apz/src/AsyncPanZoomController.h index 046a65ba08d6..e5b5eede8ce0 100644 --- a/gfx/layers/apz/src/AsyncPanZoomController.h +++ b/gfx/layers/apz/src/AsyncPanZoomController.h @@ -564,6 +564,9 @@ class AsyncPanZoomController { // Return true if there is room to scroll downwards. bool CanScrollDownwards() const; + // Return true if there is room to scroll upwards. + bool CanOverscrollUpwards() const; + /** * Convert a point on the scrollbar from this APZC's ParentLayer coordinates * to OuterCSS coordinates relative to the beginning of the scroll track. diff --git a/gfx/layers/apz/src/InputQueue.cpp b/gfx/layers/apz/src/InputQueue.cpp index 6cb58fe96bd3..f95dd1f102dc 100644 --- a/gfx/layers/apz/src/InputQueue.cpp +++ b/gfx/layers/apz/src/InputQueue.cpp @@ -952,9 +952,17 @@ static APZHandledResult GetHandledResultFor( : APZHandledResult{APZHandledPlace::Unhandled, aApzc}; } - auto [result, rootApzc] = aCurrentInputBlock.GetOverscrollHandoffChain() - ->ScrollingDownWillMoveDynamicToolbar(aApzc); - if (!result) { + bool mayTriggerPullToRefresh = + aCurrentInputBlock.GetOverscrollHandoffChain() + ->ScrollingUpWillTriggerPullToRefresh(aApzc); + if (mayTriggerPullToRefresh) { + return APZHandledResult{APZHandledPlace::Unhandled, aApzc, true}; + } + + auto [willMoveDynamicToolbar, rootApzc] = + aCurrentInputBlock.GetOverscrollHandoffChain() + ->ScrollingDownWillMoveDynamicToolbar(aApzc); + if (!willMoveDynamicToolbar) { return APZHandledResult{APZHandledPlace::HandledByContent, aApzc}; } diff --git a/gfx/layers/apz/src/OverscrollHandoffState.cpp b/gfx/layers/apz/src/OverscrollHandoffState.cpp index ed38f6fa7abc..564d65c0e417 100644 --- a/gfx/layers/apz/src/OverscrollHandoffState.cpp +++ b/gfx/layers/apz/src/OverscrollHandoffState.cpp @@ -224,5 +224,22 @@ OverscrollHandoffChain::ScrollingDownWillMoveDynamicToolbar( return {false, nullptr}; } +bool OverscrollHandoffChain::ScrollingUpWillTriggerPullToRefresh( + const AsyncPanZoomController* aApzc) const { + MOZ_ASSERT(aApzc && !aApzc->IsRootContent(), + "Should be used for non-root APZC"); + + for (uint32_t i = IndexOf(aApzc); i < Length(); i++) { + if (mChain[i]->IsRootContent()) { + return mChain[i]->CanOverscrollUpwards(); + } + + if (!mChain[i]->CanOverscrollUpwards()) { + return false; + } + } + return false; +} + } // namespace layers } // namespace mozilla diff --git a/gfx/layers/apz/src/OverscrollHandoffState.h b/gfx/layers/apz/src/OverscrollHandoffState.h index 90a22f259c85..28c89edd7448 100644 --- a/gfx/layers/apz/src/OverscrollHandoffState.h +++ b/gfx/layers/apz/src/OverscrollHandoffState.h @@ -121,6 +121,9 @@ class OverscrollHandoffChain { ScrollingDownWillMoveDynamicToolbar( const AsyncPanZoomController* aApzc) const; + bool ScrollingUpWillTriggerPullToRefresh( + const AsyncPanZoomController* aApzc) const; + private: std::vector> mChain; diff --git a/mobile/android/geckoview/src/androidTest/assets/www/pull-to-refresh-subframe.html b/mobile/android/geckoview/src/androidTest/assets/www/pull-to-refresh-subframe.html new file mode 100644 index 000000000000..d1a421c0a366 --- /dev/null +++ b/mobile/android/geckoview/src/androidTest/assets/www/pull-to-refresh-subframe.html @@ -0,0 +1,82 @@ + + + + + + + + +
+
+
+
+
+
+
+
+
+
+
+
+ + + diff --git a/mobile/android/geckoview/src/androidTest/assets/www/scroll-handoff.html b/mobile/android/geckoview/src/androidTest/assets/www/scroll-handoff.html index 98018f1a248b..c8f0fe9e9526 100644 --- a/mobile/android/geckoview/src/androidTest/assets/www/scroll-handoff.html +++ b/mobile/android/geckoview/src/androidTest/assets/www/scroll-handoff.html @@ -29,7 +29,12 @@
-
+
+ diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt index d2964aa54bd4..1fa747e52c7e 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt @@ -92,6 +92,7 @@ open class BaseSessionTest(noErrorCollector: Boolean = false) { const val IFRAME_UNKNOWN_PROTOCOL = "/assets/www/iframe_unknown_protocol.html" const val MEDIA_SESSION_DOM1_PATH = "/assets/www/media_session_dom1.html" const val MEDIA_SESSION_DEFAULT1_PATH = "/assets/www/media_session_default1.html" + const val PULL_TO_REFRESH_SUBFRAME_PATH = "/assets/www/pull-to-refresh-subframe.html" const val TOUCH_HTML_PATH = "/assets/www/touch.html" const val TOUCH_XORIGIN_HTML_PATH = "/assets/www/touch_xorigin.html" const val GETUSERMEDIA_XORIGIN_CONTAINER_HTML_PATH = "/assets/www/getusermedia_xorigin_container.html" diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/InputResultDetailTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/InputResultDetailTest.kt index cfe0bcaf12e1..bc20cb484af6 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/InputResultDetailTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/InputResultDetailTest.kt @@ -107,9 +107,9 @@ class InputResultDetailTest : BaseSessionTest() { // Since sendDownEvent() just sends a touch-down, APZ doesn't // yet know the direction, hence it allows scrolling in both // the pan-x and pan-y cases. - var expectedPlace = if (touchAction == "none" || (subframe && scrollable)) { + var expectedPlace = if (touchAction == "none") { PanZoomController.INPUT_RESULT_HANDLED_CONTENT - } else if (scrollable) { + } else if (scrollable && !subframe) { PanZoomController.INPUT_RESULT_HANDLED } else { PanZoomController.INPUT_RESULT_UNHANDLED @@ -283,7 +283,7 @@ class InputResultDetailTest : BaseSessionTest() { "handoff", value, PanZoomController.INPUT_RESULT_HANDLED_CONTENT, - PanZoomController.SCROLLABLE_FLAG_BOTTOM, + (PanZoomController.SCROLLABLE_FLAG_BOTTOM or PanZoomController.SCROLLABLE_FLAG_TOP), PanZoomController.OVERSCROLL_FLAG_VERTICAL, ) diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PanZoomControllerTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PanZoomControllerTest.kt index e40d04755819..ba4992ff8054 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PanZoomControllerTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PanZoomControllerTest.kt @@ -270,6 +270,44 @@ class PanZoomControllerTest : BaseSessionTest() { return result } + @WithDisplay(width = 100, height = 100) + @Test + fun pullToRefreshSubframe() { + setupDocument(PULL_TO_REFRESH_SUBFRAME_PATH) + + // No touch handler and no room to scroll up + var value = sessionRule.waitForResult(sendDownEvent(50f, 10f)) + assertThat( + "Touch when subframe has no room to scroll up should be unhandled", + value, + equalTo(PanZoomController.INPUT_RESULT_UNHANDLED), + ) + + // Touch handler with preventDefault + value = sessionRule.waitForResult(sendDownEvent(50f, 35f)) + assertThat( + "Touch when content handles the input should indicate so", + value, + equalTo(PanZoomController.INPUT_RESULT_HANDLED_CONTENT), + ) + + // Content with room to scroll up + value = sessionRule.waitForResult(sendDownEvent(50f, 60f)) + assertThat( + "Touch when subframe has room to scroll up should be handled by content", + value, + equalTo(PanZoomController.INPUT_RESULT_HANDLED_CONTENT), + ) + + // Touch handler without preventDefault and no room to scroll up + value = sessionRule.waitForResult(sendDownEvent(50f, 85f)) + assertThat( + "Touch no room up and not handled by content should be unhandled", + value, + equalTo(PanZoomController.INPUT_RESULT_UNHANDLED), + ) + } + @WithDisplay(width = 100, height = 100) @Test fun touchEventForResultWithStaticToolbar() { @@ -359,6 +397,22 @@ class PanZoomControllerTest : BaseSessionTest() { // There is a 100% height iframe which is scrollable. setupTouchEventDocument(IFRAME_100_PERCENT_HEIGHT_SCROLLABLE_HTML_PATH, withEventHandler) + + // Scroll down a bit to ensure the original tap cannot be the start of a + // pull to refresh gesture. + mainSession.evaluateJS( + """ + const iframe = document.querySelector('iframe'); + iframe.contentWindow.scrollTo({ + left: 0, + top: 50, + behavior: 'instant', + }); + """.trimIndent(), + ) + waitForScroll(scrollWaitTimeout) + mainSession.flushApzRepaints() + value = sessionRule.waitForResult(sendDownEvent(50f, 50f)) // The input result should be handled in the iframe content. assertThat( @@ -419,6 +473,22 @@ class PanZoomControllerTest : BaseSessionTest() { // There is a 98vh iframe which is scrollable. setupTouchEventDocument(IFRAME_98VH_SCROLLABLE_HTML_PATH, withEventHandler) + + // Scroll down a bit to ensure the original tap cannot be the start of a + // pull to refresh gesture. + mainSession.evaluateJS( + """ + const iframe = document.querySelector('iframe'); + iframe.contentWindow.scrollTo({ + left: 0, + top: 50, + behavior: 'instant', + }); + """.trimIndent(), + ) + waitForScroll(scrollWaitTimeout) + mainSession.flushApzRepaints() + value = sessionRule.waitForResult(sendDownEvent(50f, 50f)) // The input result should be handled in the iframe content initially. assertThat(