Bug 1767933. Improvements to start and end of swipe gesture animation. r=hiro

The patch for bug 1762875 stops us from calling SwipeTracker::StartAnimation if we are performing a navigation (but still call it if we aren't performing a navigation). This also has the effect of not calling SwipeTracker::SwipeFinished since it is triggered from StartAnimation. This means we don't null out nsBaseWidget::mSwipeTracker from the mWidget.SwipeFinished() call. This means that when the next pan that we want to track as a swipe then when we call nsBaseWidget::TrackScrollEventAsSwipe it will find the existing mSwipeTracker and call mSwipeTracker->CancelSwipe(), which sends a MozSwipeGestureEnd event. It would be okay if we didn't send the MozSwipeGestureEnd event in the case of a succesful navigation, the MozSwipeGesture event can serve the same purpose, but to send a MozSwipeGestureEnd when the _next_ swipe gesture is started does not seem like a good idea.

I noticed this while writing the test for bug 1762875.

To fix it just call SwipeFinished immediately when we get a successful swipe navigation. There are two side effects to consider: nulling out mSwipeTracker on the widget, and sending the MozSwipeGestureEnd event.

Nulling out mSwipeTracker on the widget seems fine, it's only there to track ongoing swipes. Since the current swipe is finished we don't need it anymore. However it brought up an issue where existing tests in widget/tests/browser/browser_test_swipe_gesture.js started failing. A test would finished immediately after a successful swipe navigation, remove the tab, and move on to the next test before the fade out animation could run. If the fade out animation had finished it would have called removeBoxes and removed the dom elements for the visual swipe arrows. So they stick around instead. Removing the tab seems to have disconnected these elements from the DOM. The structure above the swipe elements goes up to the hbox created here https://searchfox.org/mozilla-central/rev/dd404f43c7198b1076fe5d7e05b1e6b1a03bdfeb/browser/base/content/tabbrowser.js#2182 I don't know the tabbrowser code but I'm guessing that gets removed on removing a tab. So in order to fix this we always remove and re-create the boxes when we start a new animation. It should be safe to always do this and remove the isAnimationRunning early exit because we don't call startAnimation when we get a MozSwipeGestureStart event, so we should always want to start fresh when we get that event.

Sending the MozSwipeGestureEnd event means we need to be a bit more careful in stopAnimation. We don't want to do anything in stopAnimation if we are already stopping animation because the fade out will handle it, so we add a early return so that another stopAnimation call (which can come from a MozSwipeGestureEnd) won't cut off the animation prematurely.

Differential Revision: https://phabricator.services.mozilla.com/D145545
This commit is contained in:
Timothy Nikkel 2022-05-06 05:55:52 +00:00
Родитель 3085038dd2
Коммит c2bb02d580
2 изменённых файлов: 13 добавлений и 16 удалений

Просмотреть файл

@ -677,24 +677,14 @@ var gHistorySwipeAnimation = {
* Whether we're dealing with a vertical swipe or not.
*/
startAnimation: function HSA_startAnimation() {
// If the animation is running but we are in the process of stopping it
// then we want to reset and restart the animation.
if (this.isAnimationRunning() && !this._isStoppingAnimation) {
return;
}
let createBoxes = true;
if (this._isStoppingAnimation) {
// Boxes already exist, just need to reset them. Reset the transition that was in the process of stopping.
this._prevBox.style.transition = "";
this._nextBox.style.transition = "";
createBoxes = false;
}
// old boxes can still be around (if completing fade out for example), we
// always want to remove them and recreate them because they can be
// attached to an old browser stack that's no longer in use.
this._removeBoxes();
this._isStoppingAnimation = false;
this._canGoBack = this.canGoBack();
this._canGoForward = this.canGoForward();
if (createBoxes && this.active) {
if (this.active) {
this._addBoxes();
}
this.updateAnimation(0);
@ -704,7 +694,7 @@ var gHistorySwipeAnimation = {
* Stops the swipe animation.
*/
stopAnimation: function HSA_stopAnimation() {
if (!this.isAnimationRunning()) {
if (!this.isAnimationRunning() || this._isStoppingAnimation) {
return;
}

Просмотреть файл

@ -132,6 +132,13 @@ nsEventStatus SwipeTracker::ProcessEvent(const PanGestureInput& aEvent) {
// Let's use same timestamp as previous event because this is caused by
// the preceding event.
SendSwipeEvent(eSwipeGesture, mSwipeDirection, 0.0, aEvent.mTimeStamp);
UnregisterFromRefreshDriver();
NS_DispatchToMainThread(
NS_NewRunnableFunction("SwipeTracker::SwipeFinished",
[swipeTracker = RefPtr<SwipeTracker>(this),
timeStamp = aEvent.mTimeStamp] {
swipeTracker->SwipeFinished(timeStamp);
}));
} else {
StartAnimating(0.0);
}