Bug 1542800 - Clear out any leftover updater tasks after the ClearTree task is run. r=botond a=pascalc
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 08 Apr 2019 20:04:14 +0000
changeset 526162 e54cf3b64083747317a9675b587b1975dfa49607
parent 526161 d9a704ef462353c6427f316faad23af8bd76832d
child 526163 110f7d795cb30c61edbe2710eca96abe74ccd238
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond, pascalc
bugs1542800
milestone67.0
Bug 1542800 - Clear out any leftover updater tasks after the ClearTree task is run. r=botond a=pascalc This patch ensures that once the ClearTree task is run on the updater thread, we throw away any other remaining tasks in the queue, as they will never get run. (Note that the ClearTree task removes the APZUpdater instance from the global map, and so it becomes "unfindable", meaning the ProcessQueue will never run again on that instance). Differential Revision: https://phabricator.services.mozilla.com/D26541
gfx/layers/apz/public/APZUpdater.h
gfx/layers/apz/src/APZUpdater.cpp
--- a/gfx/layers/apz/public/APZUpdater.h
+++ b/gfx/layers/apz/public/APZUpdater.h
@@ -150,16 +150,17 @@ class APZUpdater {
   bool UsingWebRenderUpdaterThread() const;
   static already_AddRefed<APZUpdater> GetUpdater(
       const wr::WrWindowId& aWindowId);
 
   void ProcessQueue();
 
  private:
   RefPtr<APZCTreeManager> mApz;
+  bool mDestroyed;
   bool mIsUsingWebRender;
 
   // Map from layers id to WebRenderScrollData. This can only be touched on
   // the updater thread.
   std::unordered_map<LayersId, WebRenderScrollData, LayersId::HashFn>
       mScrollData;
 
   // Stores epoch state for a particular layers id. This structure is only
--- a/gfx/layers/apz/src/APZUpdater.cpp
+++ b/gfx/layers/apz/src/APZUpdater.cpp
@@ -21,16 +21,17 @@ namespace layers {
 
 StaticMutex APZUpdater::sWindowIdLock;
 StaticAutoPtr<std::unordered_map<uint64_t, APZUpdater*>>
     APZUpdater::sWindowIdMap;
 
 APZUpdater::APZUpdater(const RefPtr<APZCTreeManager>& aApz,
                        bool aIsUsingWebRender)
     : mApz(aApz),
+      mDestroyed(false),
       mIsUsingWebRender(aIsUsingWebRender),
       mThreadIdLock("APZUpdater::ThreadIdLock"),
       mQueueLock("APZUpdater::QueueLock") {
   MOZ_ASSERT(aApz);
   mApz->SetUpdater(this);
 }
 
 APZUpdater::~APZUpdater() {
@@ -129,16 +130,17 @@ void APZUpdater::ProcessPendingTasks(con
 }
 
 void APZUpdater::ClearTree(LayersId aRootLayersId) {
   MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
   RefPtr<APZUpdater> self = this;
   RunOnUpdaterThread(aRootLayersId,
                      NS_NewRunnableFunction("APZUpdater::ClearTree", [=]() {
                        self->mApz->ClearTree();
+                       self->mDestroyed = true;
 
                        // Once ClearTree is called on the APZCTreeManager, we
                        // are in a shutdown phase. After this point it's ok if
                        // WebRender cannot get a hold of the updater via the
                        // window id, and it's a good point to remove the mapping
                        // and avoid leaving a dangling pointer to this object.
                        StaticMutexAutoLock lock(sWindowIdLock);
                        if (self->mWindowId) {
@@ -421,16 +423,18 @@ already_AddRefed<APZUpdater> APZUpdater:
     if (it != sWindowIdMap->end()) {
       updater = it->second;
     }
   }
   return updater.forget();
 }
 
 void APZUpdater::ProcessQueue() {
+  MOZ_ASSERT(!mDestroyed);
+
   {  // scope lock to check for emptiness
     MutexAutoLock lock(mQueueLock);
     if (mUpdaterQueue.empty()) {
       return;
     }
   }
 
   std::deque<QueuedTask> blockedTasks;
@@ -461,16 +465,33 @@ void APZUpdater::ProcessQueue() {
       // If this task is blocked, put it into the blockedTasks queue that
       // we will replace mUpdaterQueue with
       blockedTasks.push_back(task);
     } else {
       // Run and discard the task
       task.mRunnable->Run();
     }
   }
+
+  if (mDestroyed) {
+    // If we get here, then we must have just run the ClearTree task for
+    // this updater. There might be tasks in the queue from content subtrees
+    // of this window that are blocked due to stale epochs. This can happen
+    // if the tasks were queued after the root pipeline was removed in
+    // WebRender, which prevents scene builds (and therefore prevents us
+    // from getting updated epochs via CompleteSceneSwap). See bug 1465658
+    // comment 43 for some more context.
+    // To avoid leaking these tasks, we discard the contents of the queue.
+    // This happens during window shutdown so if we don't run the tasks it's
+    // not going to matter much.
+    MutexAutoLock lock(mQueueLock);
+    if (!mUpdaterQueue.empty()) {
+      mUpdaterQueue.clear();
+    }
+  }
 }
 
 APZUpdater::EpochState::EpochState() : mRequired{0}, mIsRoot(false) {}
 
 bool APZUpdater::EpochState::IsBlocked() const {
   // The root is a special case because we basically assume it is "visible"
   // even before it is built for the first time. This is because building the
   // scene automatically makes it visible, and we need to make sure the APZ