Wheel event widget: Harden logic an extra bit

This is quite an unlikely scenario, but not impossible.

It could be that the wheel widget is destroyed during
an update phase event. In that case, wheel_widget would
be a dangling pointer for any subsequent wheel event.
We protect against this with a QPointer.

However, that would mean that if the next wheel event
were to be an end phase event, that event would be lost.
So we go through the usual code path, except that we won't
set wheel_widget in the case of an end phase event.

Change-Id: I59a912b845dcc249e1edc60b4dc28bf308d807d9
Reviewed-by: Shawn Rutledge <shawn.rutledge@theqtcompany.com>

2 files changed, 53 insertions, 25 deletions

diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp
index b7de0d7a7e..b34380dbc3 100644
--- a/src/widgets/kernel/qapplication.cpp
+++ b/src/widgets/kernel/qapplication.cpp
@@ -417,7 +417,7 @@ QWidget *QApplicationPrivate::hidden_focus_widget = 0; // will get keyboard inpu
 QWidget *QApplicationPrivate::active_window = 0;        // toplevel with keyboard focus
 #ifndef QT_NO_WHEELEVENT
 int QApplicationPrivate::wheel_scroll_lines;   // number of lines to scroll
-QWidget *QApplicationPrivate::wheel_widget = Q_NULLPTR;
+QPointer<QWidget> QApplicationPrivate::wheel_widget;
 #endif
 bool qt_in_tab_key_event = false;
 int qt_antialiasing_threshold = -1;
@@ -3326,9 +3326,32 @@ bool QApplication::notify(QObject *receiver, QEvent *e)
             const bool spontaneous = wheel->spontaneous();
             const Qt::ScrollPhase phase = wheel->phase();
 
-            if (phase == Qt::NoScrollPhase || phase == Qt::ScrollBegin
-                || (phase == Qt::ScrollUpdate && !QApplicationPrivate::wheel_widget)) {
-
+            // Ideally, we should lock on a widget when it starts receiving wheel
+            // events. This avoids other widgets to start receiving those events
+            // as the mouse cursor hovers them. However, given the way common
+            // wheeled mice work, there's no certain way of connecting different
+            // wheel events as a stream. This results in the NoScrollPhase case,
+            // where we just send the event from the original receiver and up its
+            // hierarchy until the event gets accepted.
+            //
+            // In the case of more evolved input devices, like Apple's trackpad or
+            // Magic Mouse, we receive the scroll phase information. This helps us
+            // connect wheel events as a stream and therefore makes it easier to
+            // lock on the widget onto which the scrolling was initiated.
+            //
+            // We assume that, when supported, the phase cycle follows the pattern:
+            //
+            //         ScrollBegin (ScrollUpdate* ScrollEnd)+
+            //
+            // This means that we can have scrolling sequences (starting with ScrollBegin)
+            // or partial sequences (after a ScrollEnd and starting with ScrollUpdate).
+            // If wheel_widget is null because it was deleted, we also take the same
+            // code path as an initial sequence.
+            if (phase == Qt::NoScrollPhase || phase == Qt::ScrollBegin || !QApplicationPrivate::wheel_widget) {
+
+                // A system-generated ScrollBegin event starts a new user scrolling
+                // sequence, so we reset wheel_widget in case no one accepts the event
+                // or if we didn't get (or missed) a ScrollEnd previously.
                 if (spontaneous && phase == Qt::ScrollBegin)
                     QApplicationPrivate::wheel_widget = Q_NULLPTR;
 
@@ -3346,7 +3369,10 @@ bool QApplication::notify(QObject *receiver, QEvent *e)
                     res = d->notify_helper(w, &we);
                     eventAccepted = we.isAccepted();
                     if (res && eventAccepted) {
-                        if (spontaneous && phase != Qt::NoScrollPhase && QGuiApplicationPrivate::scrollNoPhaseAllowed)
+                        // A new scrolling sequence or partial sequence starts and w has accepted
+                        // the event. Therefore, we can set wheel_widget, but only if it's not
+                        // the end of a sequence.
+                        if (spontaneous && (phase == Qt::ScrollBegin || phase == Qt::ScrollUpdate) && QGuiApplicationPrivate::scrollNoPhaseAllowed)
                             QApplicationPrivate::wheel_widget = w;
                         break;
                     }
@@ -3357,25 +3383,27 @@ bool QApplication::notify(QObject *receiver, QEvent *e)
                     w = w->parentWidget();
                 }
                 wheel->setAccepted(eventAccepted);
-            } else if (QApplicationPrivate::wheel_widget) {
-                if (!spontaneous) {
-                    // wheel_widget may forward the wheel event to a delegate widget,
-                    // either directly or indirectly (e.g. QAbstractScrollArea will
-                    // forward to its QScrollBars through viewportEvent()). In that
-                    // case, the event will not be spontaneous but synthesized, so
-                    // we can send it straigth to the receiver.
-                    d->notify_helper(w, wheel);
-                } else {
-                    const QPoint &relpos = QApplicationPrivate::wheel_widget->mapFromGlobal(wheel->globalPos());
-                    QWheelEvent we(relpos, wheel->globalPos(), wheel->pixelDelta(), wheel->angleDelta(), wheel->delta(), wheel->orientation(), wheel->buttons(),
-                                   wheel->modifiers(), wheel->phase(), wheel->source());
-                    we.spont = true;
-                    we.ignore();
-                    d->notify_helper(QApplicationPrivate::wheel_widget, &we);
-                    wheel->setAccepted(we.isAccepted());
-                    if (phase == Qt::ScrollEnd)
-                        QApplicationPrivate::wheel_widget = Q_NULLPTR;
-                }
+            } else if (!spontaneous) {
+                // wheel_widget may forward the wheel event to a delegate widget,
+                // either directly or indirectly (e.g. QAbstractScrollArea will
+                // forward to its QScrollBars through viewportEvent()). In that
+                // case, the event will not be spontaneous but synthesized, so
+                // we can send it straight to the receiver.
+                d->notify_helper(w, wheel);
+            } else {
+                // The phase is either ScrollUpdate or ScrollEnd, and wheel_widget
+                // is set. Since it accepted the wheel event previously, we continue
+                // sending those events until we get a ScrollEnd, which signifies
+                // the end of the natural scrolling sequence.
+                const QPoint &relpos = QApplicationPrivate::wheel_widget->mapFromGlobal(wheel->globalPos());
+                QWheelEvent we(relpos, wheel->globalPos(), wheel->pixelDelta(), wheel->angleDelta(), wheel->delta(), wheel->orientation(), wheel->buttons(),
+                               wheel->modifiers(), wheel->phase(), wheel->source());
+                we.spont = true;
+                we.ignore();
+                d->notify_helper(QApplicationPrivate::wheel_widget, &we);
+                wheel->setAccepted(we.isAccepted());
+                if (phase == Qt::ScrollEnd)
+                    QApplicationPrivate::wheel_widget = Q_NULLPTR;
             }
         }
         break;
diff --git a/src/widgets/kernel/qapplication_p.h b/src/widgets/kernel/qapplication_p.h
index 832d37a329..4b3cf773dc 100644
--- a/src/widgets/kernel/qapplication_p.h
+++ b/src/widgets/kernel/qapplication_p.h
@@ -202,7 +202,7 @@ public:
     static QWidget *active_window;
 #ifndef QT_NO_WHEELEVENT
     static int  wheel_scroll_lines;
-    static QWidget *wheel_widget;
+    static QPointer<QWidget> wheel_widget;
 #endif
 
     static int enabledAnimations; // Combination of QPlatformTheme::UiEffect