Bug 1537958 - Discard and reschedule visual viewport events if the prescontext changes. r=botond

It seems that in some scenarios, the lifetime of the visual viewport
object exceeds the lifetime of the prescontext. In particular, the
prescontext produced by GetPresContext() can be torn down and a new one
installed. This can leave the visual viewport in a wedged state where it
has scheduled events to be dispatched, but they will never actually be
dispatched, and then subsequent events will not get scheduled. This
patch checks to see if the prescontext has changed, and if so, discards
the old events and reschedules new ones.

Differential Revision: https://phabricator.services.mozilla.com/D26578

--HG--
extra : moz-landing-system : lando

dom/base/VisualViewport.cpp

@@ -149,13 +149,19 @@ nsPresContext* VisualViewport::GetPresContext() const {
 /* ================= Resize event handling ================= */
 
 void VisualViewport::PostResizeEvent() {
-  VVP_LOG("%p: PostResizeEvent\n", this);
-  if (mResizeEvent) {
+  VVP_LOG("%p: PostResizeEvent (pre-existing: %d)\n", this, !!mResizeEvent);
+  nsPresContext* presContext = GetPresContext();
+  if (mResizeEvent && mResizeEvent->HasPresContext(presContext)) {
     return;
   }
+  if (mResizeEvent) {
+    // prescontext changed, so discard the old resize event and queue a new one
+    mResizeEvent->Revoke();
+    mResizeEvent = nullptr;
+  }
 
   // The event constructor will register itself with the refresh driver.
-  if (nsPresContext* presContext = GetPresContext()) {
+  if (presContext) {
     mResizeEvent = new VisualViewportResizeEvent(this, presContext);
     VVP_LOG("%p: PostResizeEvent, created new event\n", this);
   }
@@ -164,10 +170,23 @@ void VisualViewport::PostResizeEvent() {
 VisualViewport::VisualViewportResizeEvent::VisualViewportResizeEvent(
     VisualViewport* aViewport, nsPresContext* aPresContext)
     : Runnable("VisualViewport::VisualViewportResizeEvent"),
-      mViewport(aViewport) {
+      mViewport(aViewport),
+      mPresContext(aPresContext) {
+  VVP_LOG("%p: Registering PostResize on %p %p\n", aViewport, aPresContext,
+          aPresContext->RefreshDriver());
   aPresContext->RefreshDriver()->PostVisualViewportResizeEvent(this);
 }
 
+bool VisualViewport::VisualViewportResizeEvent::HasPresContext(
+    nsPresContext* aContext) const {
+  return mPresContext.get() == aContext;
+}
+
+void VisualViewport::VisualViewportResizeEvent::Revoke() {
+  mViewport = nullptr;
+  mPresContext = nullptr;
+}
+
 NS_IMETHODIMP
 VisualViewport::VisualViewportResizeEvent::Run() {
   if (mViewport) {
@@ -197,14 +216,22 @@ void VisualViewport::FireResizeEvent() {
 
 void VisualViewport::PostScrollEvent(const nsPoint& aPrevVisualOffset,
                                      const nsPoint& aPrevLayoutOffset) {
-  VVP_LOG("%p: PostScrollEvent, prevRelativeOffset=%s\n", this,
-          ToString(aPrevVisualOffset - aPrevLayoutOffset).c_str());
-  if (mScrollEvent) {
+  VVP_LOG("%p: PostScrollEvent, prevRelativeOffset=%s (pre-existing: %d)\n",
+          this, ToString(aPrevVisualOffset - aPrevLayoutOffset).c_str(),
+          !!mScrollEvent);
+  nsPresContext* presContext = GetPresContext();
+  if (mScrollEvent && mScrollEvent->HasPresContext(presContext)) {
     return;
   }
 
+  if (mScrollEvent) {
+    // prescontext changed, so discard the old scroll event and queue a new one
+    mScrollEvent->Revoke();
+    mScrollEvent = nullptr;
+  }
+
   // The event constructor will register itself with the refresh driver.
-  if (nsPresContext* presContext = GetPresContext()) {
+  if (presContext) {
     mScrollEvent = new VisualViewportScrollEvent(
         this, presContext, aPrevVisualOffset, aPrevLayoutOffset);
     VVP_LOG("%p: PostScrollEvent, created new event\n", this);
@@ -216,11 +243,24 @@ VisualViewport::VisualViewportScrollEvent::VisualViewportScrollEvent(
     const nsPoint& aPrevVisualOffset, const nsPoint& aPrevLayoutOffset)
     : Runnable("VisualViewport::VisualViewportScrollEvent"),
       mViewport(aViewport),
+      mPresContext(aPresContext),
       mPrevVisualOffset(aPrevVisualOffset),
       mPrevLayoutOffset(aPrevLayoutOffset) {
+  VVP_LOG("%p: Registering PostScroll on %p %p\n", aViewport, aPresContext,
+          aPresContext->RefreshDriver());
   aPresContext->RefreshDriver()->PostVisualViewportScrollEvent(this);
 }
 
+bool VisualViewport::VisualViewportScrollEvent::HasPresContext(
+    nsPresContext* aContext) const {
+  return mPresContext.get() == aContext;
+}
+
+void VisualViewport::VisualViewportScrollEvent::Revoke() {
+  mViewport = nullptr;
+  mPresContext = nullptr;
+}
+
 NS_IMETHODIMP
 VisualViewport::VisualViewportScrollEvent::Run() {
   if (mViewport) {

dom/base/VisualViewport.h

@@ -8,6 +8,7 @@
 #define mozilla_dom_VisualViewport_h
 
 #include "mozilla/DOMEventTargetHelper.h"
+#include "mozilla/WeakPtr.h"
 #include "mozilla/dom/VisualViewportBinding.h"
 #include "Units.h"
 #include "nsIPresShell.h"
@@ -46,10 +47,12 @@ class VisualViewport final : public mozilla::DOMEventTargetHelper {
     NS_DECL_NSIRUNNABLE
     VisualViewportResizeEvent(VisualViewport* aViewport,
                               nsPresContext* aPresContext);
-    void Revoke() { mViewport = nullptr; }
+    bool HasPresContext(nsPresContext* aContext) const;
+    void Revoke();
 
    private:
     VisualViewport* mViewport;
+    WeakPtr<nsPresContext> mPresContext;
   };
 
   class VisualViewportScrollEvent : public Runnable {
@@ -59,12 +62,14 @@ class VisualViewport final : public mozilla::DOMEventTargetHelper {
                               nsPresContext* aPresContext,
                               const nsPoint& aPrevVisualOffset,
                               const nsPoint& aPrevLayoutOffset);
-    void Revoke() { mViewport = nullptr; }
+    bool HasPresContext(nsPresContext* aContext) const;
+    void Revoke();
     nsPoint PrevVisualOffset() const { return mPrevVisualOffset; }
     nsPoint PrevLayoutOffset() const { return mPrevLayoutOffset; }
 
    private:
     VisualViewport* mViewport;
+    WeakPtr<nsPresContext> mPresContext;
     // The VisualViewport "scroll" event is supposed to be fired only when the
     // *relative* offset between visual and layout viewport changes. The two
     // viewports are updated independently from each other, though, so the only