Bug 1501793 - Fix a shutdown assert in APZUpdater::~APZUpdater. r=kats
authorAndrew Osmond <aosmond@mozilla.com>
Wed, 24 Oct 2018 15:40:29 -0400
changeset 491327 b304191da6b7922375b78f6b3ea27245833a2ab6
parent 491326 9c5d6ad21391c871733e2cb4aeb5840a7833e112
child 491328 25132db0240fecdcebdf68946ff77a78f0bdcece
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerskats
bugs1501793
milestone65.0a1
Bug 1501793 - Fix a shutdown assert in APZUpdater::~APZUpdater. r=kats When APZUpdater::mUpdaterQueue still contains entries, and it is destroyed, we may trip an assert due to destroying an already_AddRefed object without taking its contents into a RefPtr. This is because APZUpdater::RunOnControllerThread would pass an already_AddRefed object directly into NewRunnableFunction as the object, instead of a RefPtr object. This caused templated object to store an already_AddRefed object as a result, and when we wanted to drop the object, it complained. Storing it as a RefPtr should cause everything to be freed normally. Differential Revision: https://phabricator.services.mozilla.com/D9702
gfx/layers/apz/src/APZUpdater.cpp
gfx/layers/apz/util/APZThreadUtils.cpp
gfx/layers/apz/util/APZThreadUtils.h
--- a/gfx/layers/apz/src/APZUpdater.cpp
+++ b/gfx/layers/apz/src/APZUpdater.cpp
@@ -441,20 +441,22 @@ APZUpdater::IsUpdaterThread() const
   return CompositorThreadHolder::IsInCompositorThread();
 }
 
 void
 APZUpdater::RunOnControllerThread(LayersId aLayersId, already_AddRefed<Runnable> aTask)
 {
   MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
 
+  RefPtr<Runnable> task = aTask;
+
   RunOnUpdaterThread(aLayersId, NewRunnableFunction(
       "APZUpdater::RunOnControllerThread",
       &APZThreadUtils::RunOnControllerThread,
-      std::move(aTask)));
+      std::move(task)));
 }
 
 bool
 APZUpdater::UsingWebRenderUpdaterThread() const
 {
   return mIsUsingWebRender;
 }
 
--- a/gfx/layers/apz/util/APZThreadUtils.cpp
+++ b/gfx/layers/apz/util/APZThreadUtils.cpp
@@ -36,19 +36,19 @@ APZThreadUtils::AssertOnControllerThread
   if (!GetThreadAssertionsEnabled()) {
     return;
   }
 
   MOZ_ASSERT(sControllerThread == MessageLoop::current());
 }
 
 /*static*/ void
-APZThreadUtils::RunOnControllerThread(already_AddRefed<Runnable> aTask)
+APZThreadUtils::RunOnControllerThread(RefPtr<Runnable>&& aTask)
 {
-  RefPtr<Runnable> task = aTask;
+  RefPtr<Runnable> task = std::move(aTask);
 
   if (!sControllerThread) {
     // Could happen on startup
     NS_WARNING("Dropping task posted to controller thread");
     return;
   }
 
   if (sControllerThread == MessageLoop::current()) {
--- a/gfx/layers/apz/util/APZThreadUtils.h
+++ b/gfx/layers/apz/util/APZThreadUtils.h
@@ -40,17 +40,17 @@ public:
    */
   static void AssertOnControllerThread();
 
   /**
    * Run the given task on the APZ "controller thread" for this platform. If
    * this function is called from the controller thread itself then the task is
    * run immediately without getting queued.
    */
-  static void RunOnControllerThread(already_AddRefed<Runnable> aTask);
+  static void RunOnControllerThread(RefPtr<Runnable>&& aTask);
 
   /**
    * Returns true if currently on APZ "controller thread".
    */
   static bool IsControllerThread();
 };
 
 // A base class for GenericNamedTimerCallback<Function>.