Bug 1635001 - P3. Fix APZ controller thread-safety access. r=kats

The APZ was keeping a raw pointer to the controller thread. This was a dangerous exercise.
This was okay on desktop, as the controller thread was the main thread and would have outlived everything else. On Android however it's the UI thread and it could get deleted before we received a last input event.

So we use a strong pointer instead to prevent the thread from being deleted and as such, we now needs to explicitly clear it on shutdown.

This requires the various methods in APZThreadUtils to be made thread-safe so that the controller thread can be shutdown mid-air.

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

gfx/layers/apz/util/APZThreadUtils.cpp

@@ -6,19 +6,25 @@
 
 #include "mozilla/layers/APZThreadUtils.h"
 
+#include "mozilla/ClearOnShutdown.h"
+#include "mozilla/StaticMutex.h"
+
 namespace mozilla {
 namespace layers {
 
 static bool sThreadAssertionsEnabled = true;
-static nsISerialEventTarget* sControllerThread;
+static StaticRefPtr<nsISerialEventTarget> sControllerThread;
+static StaticMutex sControllerThreadMutex;
 
 /*static*/
 void APZThreadUtils::SetThreadAssertionsEnabled(bool aEnabled) {
+  StaticMutexAutoLock lock(sControllerThreadMutex);
   sThreadAssertionsEnabled = aEnabled;
 }
 
 /*static*/
 bool APZThreadUtils::GetThreadAssertionsEnabled() {
+  StaticMutexAutoLock lock(sControllerThreadMutex);
   return sThreadAssertionsEnabled;
 }
 
@@ -26,51 +32,73 @@ bool APZThreadUtils::GetThreadAssertionsEnabled() {
 void APZThreadUtils::SetControllerThread(nsISerialEventTarget* aThread) {
   // We must either be setting the initial controller thread, or removing it,
   // or re-using an existing controller thread.
+  StaticMutexAutoLock lock(sControllerThreadMutex);
   MOZ_ASSERT(!sControllerThread || !aThread || sControllerThread == aThread);
+  if (aThread != sControllerThread) {
+    // This can only happen once, on startup.
     sControllerThread = aThread;
+    ClearOnShutdown(&sControllerThread);
+  }
 }
 
 /*static*/
 void APZThreadUtils::AssertOnControllerThread() {
+#if DEBUG
   if (!GetThreadAssertionsEnabled()) {
     return;
   }
-
-  MOZ_ASSERT(sControllerThread->IsOnCurrentThread());
+  StaticMutexAutoLock lock(sControllerThreadMutex);
+  MOZ_ASSERT(sControllerThread && sControllerThread->IsOnCurrentThread());
+#endif
 }
 
 /*static*/
 void APZThreadUtils::RunOnControllerThread(RefPtr<Runnable>&& aTask) {
+  RefPtr<nsISerialEventTarget> thread;
+  {
+    StaticMutexAutoLock lock(sControllerThreadMutex);
+    thread = sControllerThread;
+  }
   RefPtr<Runnable> task = std::move(aTask);
 
-  if (!sControllerThread) {
-    // Could happen on startup
+  if (!thread) {
+    // Could happen on startup or if Shutdown() got called.
     NS_WARNING("Dropping task posted to controller thread");
     return;
   }
 
-  if (sControllerThread->IsOnCurrentThread()) {
+  if (thread->IsOnCurrentThread()) {
     task->Run();
   } else {
-    sControllerThread->Dispatch(task.forget());
+    thread->Dispatch(task.forget());
   }
 }
 
 /*static*/
 bool APZThreadUtils::IsControllerThread() {
-  return sControllerThread == NS_GetCurrentThread();
+  StaticMutexAutoLock lock(sControllerThreadMutex);
+  return sControllerThread && sControllerThread->IsOnCurrentThread();
 }
 
 /*static*/
 void APZThreadUtils::DelayedDispatch(already_AddRefed<Runnable> aRunnable,
                                 int aDelayMs) {
-  MOZ_ASSERT(sControllerThread && sControllerThread->IsOnCurrentThread());
   MOZ_ASSERT(!XRE_IsContentProcess(),
              "ContentProcessController should only be used remotely.");
+  RefPtr<nsISerialEventTarget> thread;
+  {
+    StaticMutexAutoLock lock(sControllerThreadMutex);
+    thread = sControllerThread;
+  }
+  if (!thread) {
+    // Could happen on startup
+    NS_WARNING("Dropping task posted to controller thread");
+    return;
+  }
   if (aDelayMs) {
-    sControllerThread->DelayedDispatch(std::move(aRunnable), aDelayMs);
+    thread->DelayedDispatch(std::move(aRunnable), aDelayMs);
   } else {
-    sControllerThread->Dispatch(std::move(aRunnable));
+    thread->Dispatch(std::move(aRunnable));
   }
 }
 

gfx/layers/apz/util/APZThreadUtils.h

@@ -53,7 +53,6 @@ class APZThreadUtils {
   /**
    * Schedules a runnable to run on the controller thread at some time
    * in the future.
-   * This method must always be called on the controller thread.
    */
   static void DelayedDispatch(already_AddRefed<Runnable> aRunnable,
                               int aDelayMs);