Bug 1343977 - Ensure that synthetic mouseclicks generated from touch gestures don't reopen rollups. r=botond,enndeakin+6102

Synthetic mouseevents generated from touch gestures are dispatched asynchronously
from the touch events. This means that the nsAutoRollup that the widget puts on
the stack during the dispatching of the touch events is no longer in scope when
we get around to firing the mouse events. As a result, the mouse events can end
up reopening rollups that got closed by the touch events and that shouldn't be
reopened. We fix this by stashing the rollup that was in scope while the touch
events were being dispatched, and make sure we keep that in scope for the
synthetic mouse events.

MozReview-Commit-ID: HjteKHfAqvD

--HG--
extra : rebase_source : 46217b1ba610ef193963ccab454a6f584af61a2b

gfx/layers/apz/util/APZEventState.cpp

@@ -32,6 +32,7 @@
 #include "nsIScrollableFrame.h"
 #include "nsIScrollbarMediator.h"
 #include "mozilla/TouchEvents.h"
+#include "mozilla/widget/nsAutoRollup.h"
 
 #define APZES_LOG(...)
 // #define APZES_LOG(...) printf_stderr("APZES: " __VA_ARGS__)
@@ -129,19 +130,22 @@ public:
                             LayoutDevicePoint& aPoint,
                             Modifiers aModifiers,
                             int32_t aClickCount,
-                            nsITimer* aTimer)
+                            nsITimer* aTimer,
+                            RefPtr<nsIContent>& aTouchRollup)
     : mWidget(aWidget)
     , mPoint(aPoint)
     , mModifiers(aModifiers)
     , mClickCount(aClickCount)
     // Hold the reference count until we are called back.
     , mTimer(aTimer)
+    , mTouchRollup(aTouchRollup)
   {
   }
 
   NS_IMETHOD Notify(nsITimer*) override
   {
     if (nsCOMPtr<nsIWidget> widget = do_QueryReferent(mWidget)) {
+      widget::nsAutoRollup rollup(mTouchRollup.get());
       APZCCallbackHelper::FireSingleTapEvent(mPoint, mModifiers, mClickCount, widget);
     }
     mTimer = nullptr;
@@ -162,6 +166,7 @@ private:
   Modifiers mModifiers;
   int32_t mClickCount;
   nsCOMPtr<nsITimer> mTimer;
+  RefPtr<nsIContent> mTouchRollup;
 };
 
 NS_IMPL_ISUPPORTS(DelayedFireSingleTapEvent, nsITimerCallback)
@@ -176,6 +181,9 @@ APZEventState::ProcessSingleTap(const CSSPoint& aPoint,
   APZES_LOG("Handling single tap at %s on %s with %d\n",
     Stringify(aPoint).c_str(), Stringify(aGuid).c_str(), mTouchEndCancelled);
 
+  RefPtr<nsIContent> touchRollup = GetTouchRollup();
+  mTouchRollup = nullptr;
+
   nsCOMPtr<nsIWidget> widget = GetWidget();
   if (!widget) {
     return;
@@ -190,6 +198,7 @@ APZEventState::ProcessSingleTap(const CSSPoint& aPoint,
     // If the active element isn't visually affected by the :active style, we
     // have no need to wait the extra sActiveDurationMs to make the activation
     // visually obvious to the user.
+    widget::nsAutoRollup rollup(touchRollup.get());
     APZCCallbackHelper::FireSingleTapEvent(ldPoint, aModifiers, aClickCount, widget);
     return;
   }
@@ -197,7 +206,8 @@ APZEventState::ProcessSingleTap(const CSSPoint& aPoint,
   APZES_LOG("Active element uses style, scheduling timer for click event\n");
   nsCOMPtr<nsITimer> timer = do_CreateInstance(NS_TIMER_CONTRACTID);
   RefPtr<DelayedFireSingleTapEvent> callback =
-    new DelayedFireSingleTapEvent(mWidget, ldPoint, aModifiers, aClickCount, timer);
+    new DelayedFireSingleTapEvent(mWidget, ldPoint, aModifiers, aClickCount,
+        timer, touchRollup);
   nsresult rv = timer->InitWithCallback(callback,
                                         sActiveDurationMs,
                                         nsITimer::TYPE_ONE_SHOT);
@@ -322,6 +332,8 @@ APZEventState::ProcessTouchEvent(const WidgetTouchEvent& aEvent,
   switch (aEvent.mMessage) {
   case eTouchStart: {
     mTouchEndCancelled = false;
+    mTouchRollup = do_GetWeakReference(widget::nsAutoRollup::GetLastRollup());
+
     sentContentResponse = SendPendingTouchPreventedResponse(false);
     // sentContentResponse can be true here if we get two TOUCH_STARTs in a row
     // and just responded to the first one.
@@ -509,5 +521,12 @@ APZEventState::GetWidget() const
   return result.forget();
 }
 
+already_AddRefed<nsIContent>
+APZEventState::GetTouchRollup() const
+{
+  nsCOMPtr<nsIContent> result = do_QueryReferent(mTouchRollup);
+  return result.forget();
+}
+
 } // namespace layers
 } // namespace mozilla

gfx/layers/apz/util/APZEventState.h

@@ -20,6 +20,7 @@
 #include <functional>
 
 template <class> class nsCOMPtr;
+class nsIContent;
 class nsIDocument;
 class nsIPresShell;
 class nsIWidget;
@@ -86,6 +87,7 @@ private:
                              Modifiers aModifiers,
                              const nsCOMPtr<nsIWidget>& aWidget);
   already_AddRefed<nsIWidget> GetWidget() const;
+  already_AddRefed<nsIContent> GetTouchRollup() const;
 private:
   nsWeakPtr mWidget;
   RefPtr<ActiveElementManager> mActiveElementManager;
@@ -96,6 +98,22 @@ private:
   bool mEndTouchIsClick;
   bool mTouchEndCancelled;
   int32_t mLastTouchIdentifier;
+
+  // Because touch-triggered mouse events (e.g. mouse events from a tap
+  // gesture) happen asynchronously from the touch events themselves, we
+  // need to stash and replicate some of the state from the touch events
+  // to the mouse events. One piece of state is the rollup content, which
+  // is the content for which a popup window was recently closed. If we
+  // don't replicate this state properly during the mouse events, the
+  // synthetic click might reopen a popup window that was just closed by
+  // the touch event, which is undesirable. See also documentation in
+  // nsAutoRollup.h
+  // Note that in cases where we get multiple touch blocks interleaved with
+  // their single-tap event notifications, mTouchRollup may hold an incorrect
+  // value. This is kind of an edge case, and falls in the same category of
+  // problems as bug 1227241. I intend that fixing that bug will also take
+  // care of this potential problem.
+  nsWeakPtr mTouchRollup;
 };
 
 } // namespace layers

widget/nsAutoRollup.cpp

@@ -20,6 +20,14 @@ nsAutoRollup::nsAutoRollup()
   sCount++;
 }
 
+nsAutoRollup::nsAutoRollup(nsIContent* aRollup)
+{
+  MOZ_ASSERT(!sLastRollup);
+  mWasClear = true;
+  sCount++;
+  SetLastRollup(aRollup);
+}
+
 nsAutoRollup::~nsAutoRollup()
 {
   if (sLastRollup && mWasClear) {

widget/nsAutoRollup.h

@@ -31,6 +31,10 @@ public:
   nsAutoRollup();
   ~nsAutoRollup();
 
+  // Convenience constructor that creates a nsAutoRollup and also sets
+  // the last rollup.
+  explicit nsAutoRollup(nsIContent* aRollup);
+
   static void SetLastRollup(nsIContent* aLastRollup);
   // Return the popup that was last rolled up, or null if there isn't one.
   static nsIContent* GetLastRollup();