Bug 1343977 - Ensure that synthetic mouseclicks generated from touch gestures don't reopen rollups. r=botond,enndeakin+6102
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 13 Mar 2017 10:44:59 -0400
changeset 347618 40479d157fc874df89dd7b7f959f761319aece49
parent 347617 8dcb0cd0ee3699057a2c491e496280f90f9838cf
child 347619 b57650ab9be87d34dbb63e36afe8b266240ee92a
push id31501
push usercbook@mozilla.com
push dateWed, 15 Mar 2017 12:01:05 +0000
treeherdermozilla-central@0cc9dced786c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond, enndeakin
bugs1343977
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
gfx/layers/apz/util/APZEventState.cpp
gfx/layers/apz/util/APZEventState.h
widget/nsAutoRollup.cpp
widget/nsAutoRollup.h
--- a/gfx/layers/apz/util/APZEventState.cpp
+++ b/gfx/layers/apz/util/APZEventState.cpp
@@ -27,16 +27,17 @@
 #include "nsLayoutUtils.h"
 #include "nsQueryFrame.h"
 #include "TouchManager.h"
 #include "nsIDOMMouseEvent.h"
 #include "nsLayoutUtils.h"
 #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__)
 
 // Static helper functions
 namespace {
 
 int32_t
@@ -124,29 +125,32 @@ class DelayedFireSingleTapEvent final : 
 {
 public:
   NS_DECL_ISUPPORTS
 
   DelayedFireSingleTapEvent(nsWeakPtr aWidget,
                             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;
     return NS_OK;
   }
 
   void ClearTimer() {
     mTimer = nullptr;
@@ -157,52 +161,58 @@ private:
   {
   }
 
   nsWeakPtr mWidget;
   LayoutDevicePoint mPoint;
   Modifiers mModifiers;
   int32_t mClickCount;
   nsCOMPtr<nsITimer> mTimer;
+  RefPtr<nsIContent> mTouchRollup;
 };
 
 NS_IMPL_ISUPPORTS(DelayedFireSingleTapEvent, nsITimerCallback)
 
 void
 APZEventState::ProcessSingleTap(const CSSPoint& aPoint,
                                 const CSSToLayoutDeviceScale& aScale,
                                 Modifiers aModifiers,
                                 const ScrollableLayerGuid& aGuid,
                                 int32_t aClickCount)
 {
   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;
   }
 
   if (mTouchEndCancelled) {
     return;
   }
 
   LayoutDevicePoint ldPoint = aPoint * aScale;
   if (!mActiveElementManager->ActiveElementUsesStyle()) {
     // 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;
   }
 
   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);
   if (NS_FAILED(rv)) {
     // Make |callback| not hold the timer, so they will both be destructed when
     // we leave the scope of this function.
     callback->ClearTimer();
   }
@@ -317,16 +327,18 @@ APZEventState::ProcessTouchEvent(const W
   }
 
   bool isTouchPrevented = aContentResponse == nsEventStatus_eConsumeNoDefault;
   bool sentContentResponse = false;
   APZES_LOG("Handling event type %d\n", aEvent.mMessage);
   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.
 
     // We're about to send a response back to APZ, but we should only do it
     // for events that went through APZ (which should be all of them).
     MOZ_ASSERT(aEvent.mFlags.mHandledByAPZ);
 
@@ -504,10 +516,17 @@ APZEventState::SendPendingTouchPrevented
 
 already_AddRefed<nsIWidget>
 APZEventState::GetWidget() const
 {
   nsCOMPtr<nsIWidget> result = do_QueryReferent(mWidget);
   return result.forget();
 }
 
+already_AddRefed<nsIContent>
+APZEventState::GetTouchRollup() const
+{
+  nsCOMPtr<nsIContent> result = do_QueryReferent(mTouchRollup);
+  return result.forget();
+}
+
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/apz/util/APZEventState.h
+++ b/gfx/layers/apz/util/APZEventState.h
@@ -15,16 +15,17 @@
 #include "mozilla/RefPtr.h"
 #include "nsCOMPtr.h"
 #include "nsISupportsImpl.h"  // for NS_INLINE_DECL_REFCOUNTING
 #include "nsIWeakReferenceUtils.h"  // for nsWeakPtr
 
 #include <functional>
 
 template <class> class nsCOMPtr;
+class nsIContent;
 class nsIDocument;
 class nsIPresShell;
 class nsIWidget;
 
 namespace mozilla {
 namespace layers {
 
 class ActiveElementManager;
@@ -81,24 +82,41 @@ private:
   ~APZEventState();
   bool SendPendingTouchPreventedResponse(bool aPreventDefault);
   bool FireContextmenuEvents(const nsCOMPtr<nsIPresShell>& aPresShell,
                              const CSSPoint& aPoint,
                              const CSSToLayoutDeviceScale& aScale,
                              Modifiers aModifiers,
                              const nsCOMPtr<nsIWidget>& aWidget);
   already_AddRefed<nsIWidget> GetWidget() const;
+  already_AddRefed<nsIContent> GetTouchRollup() const;
 private:
   nsWeakPtr mWidget;
   RefPtr<ActiveElementManager> mActiveElementManager;
   ContentReceivedInputBlockCallback mContentReceivedInputBlockCallback;
   bool mPendingTouchPreventedResponse;
   ScrollableLayerGuid mPendingTouchPreventedGuid;
   uint64_t mPendingTouchPreventedBlockId;
   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
 } // namespace mozilla
 
 #endif /* mozilla_layers_APZEventState_h */
--- a/widget/nsAutoRollup.cpp
+++ b/widget/nsAutoRollup.cpp
@@ -15,16 +15,24 @@ nsAutoRollup::nsAutoRollup()
 {
   // remember if sLastRollup was null, and only clear it upon destruction
   // if so. This prevents recursive usage of nsAutoRollup from clearing
   // sLastRollup when it shouldn't.
   mWasClear = !sLastRollup;
   sCount++;
 }
 
+nsAutoRollup::nsAutoRollup(nsIContent* aRollup)
+{
+  MOZ_ASSERT(!sLastRollup);
+  mWasClear = true;
+  sCount++;
+  SetLastRollup(aRollup);
+}
+
 nsAutoRollup::~nsAutoRollup()
 {
   if (sLastRollup && mWasClear) {
     sLastRollup = nullptr;
   }
   sCount--;
 }
 
--- a/widget/nsAutoRollup.h
+++ b/widget/nsAutoRollup.h
@@ -26,16 +26,20 @@ namespace widget {
 // As sLastRollup is static, it can be retrieved by calling
 // nsAutoRollup::GetLastRollup.
 class MOZ_RAII nsAutoRollup
 {
 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();
 
 private:
   // Whether sLastRollup was clear when this nsAutoRollup
   // was created.
   bool mWasClear;