Bug 1445662 - Ensure UpdateZoomConstraints runs on the sampler thread. r?rhunt draft
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 14 Mar 2018 16:57:42 -0400
changeset 767607 5e63a1d7e9e5da8410243ecb8d75ec298547e633
parent 767606 0a18bb0b98ff2cbe16cf552e431d837dad1fd1ef
child 767608 ebcc25a043f6386347e0fe30165673884183411d
push id102649
push userkgupta@mozilla.com
push dateWed, 14 Mar 2018 20:58:42 +0000
reviewersrhunt
bugs1445662
milestone61.0a1
Bug 1445662 - Ensure UpdateZoomConstraints runs on the sampler thread. r?rhunt Without this patch, UpdateZoomConstraints can get called on: a) the compositor/sampler thread (over PAPZCTreeManager) b) the controller thread which is also the UI process main thread (on desktop platforms without a GPU process) c) the UI process main thread when it's *not* the controller thread (on Android). Instead of having to reason about all these scenarios separately, we can try to unify them a little bit by ensuring the function contents always run on the sampler thread, which is the thread that seems to make the most sense for it. MozReview-Commit-ID: 8V4WTNtST3d
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
gfx/layers/apz/util/APZThreadUtils.cpp
gfx/layers/apz/util/APZThreadUtils.h
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -1930,16 +1930,34 @@ APZCTreeManager::SetTargetAPZC(uint64_t 
   RefPtr<AsyncPanZoomController> apzc = GetTargetAPZC(aTarget);
   mInputQueue->SetConfirmedTargetApzc(aInputBlockId, apzc);
 }
 
 void
 APZCTreeManager::UpdateZoomConstraints(const ScrollableLayerGuid& aGuid,
                                        const Maybe<ZoomConstraints>& aConstraints)
 {
+  if (!APZThreadUtils::IsSamplerThread()) {
+    // This can happen if we're in the UI process and got a call directly from
+    // nsBaseWidget (as opposed to over PAPZCTreeManager). We want this function
+    // to run on the sampler thread, so bounce it over.
+    MOZ_ASSERT(XRE_IsParentProcess());
+
+    APZThreadUtils::RunOnSamplerThread(
+        NewRunnableMethod<ScrollableLayerGuid, Maybe<ZoomConstraints>>(
+            "APZCTreeManager::UpdateZoomConstraints",
+            this,
+            &APZCTreeManager::UpdateZoomConstraints,
+            aGuid,
+            aConstraints));
+    return;
+  }
+
+  APZThreadUtils::AssertOnSamplerThread();
+
   RecursiveMutexAutoLock lock(mTreeLock);
   RefPtr<HitTestingTreeNode> node = GetTargetNode(aGuid, nullptr);
   MOZ_ASSERT(!node || node->GetApzc()); // any node returned must have an APZC
 
   // Propagate the zoom constraints down to the subtree, stopping at APZCs
   // which have their own zoom constraints or are in a different layers id.
   if (aConstraints) {
     APZCTM_LOG("Recording constraints %s for guid %s\n",
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -686,22 +686,23 @@ private:
   /* Layers id for the root CompositorBridgeParent that owns this APZCTreeManager. */
   uint64_t mRootLayersId;
 
   /* Whenever walking or mutating the tree rooted at mRootNode, mTreeLock must be held.
    * This lock does not need to be held while manipulating a single APZC instance in
    * isolation (that is, if its tree pointers are not being accessed or mutated). The
    * lock also needs to be held when accessing the mRootNode instance variable, as that
    * is considered part of the APZC tree management state.
-   * Finally, the lock needs to be held when accessing mZoomConstraints.
    * IMPORTANT: See the note about lock ordering at the top of this file. */
   mutable mozilla::RecursiveMutex mTreeLock;
   RefPtr<HitTestingTreeNode> mRootNode;
+
   /* Holds the zoom constraints for scrollable layers, as determined by the
-   * the main-thread gecko code. */
+   * the main-thread gecko code. This can only be accessed on the sampler
+   * thread. */
   std::unordered_map<ScrollableLayerGuid, ZoomConstraints, ScrollableLayerGuidHash> mZoomConstraints;
   /* A list of keyboard shortcuts to use for translating keyboard inputs into
    * keyboard actions. This is gathered on the main thread from XBL bindings.
    * This must only be accessed on the controller thread.
    */
   KeyboardMap mKeyboardMap;
   /* This tracks the focus targets of chrome and content and whether we have
    * a current focus target or whether we are waiting for a new confirmation.
--- a/gfx/layers/apz/util/APZThreadUtils.cpp
+++ b/gfx/layers/apz/util/APZThreadUtils.cpp
@@ -38,24 +38,16 @@ APZThreadUtils::AssertOnControllerThread
   if (!GetThreadAssertionsEnabled()) {
     return;
   }
 
   MOZ_ASSERT(sControllerThread == MessageLoop::current());
 }
 
 /*static*/ void
-APZThreadUtils::AssertOnSamplerThread()
-{
-  if (GetThreadAssertionsEnabled()) {
-    MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
-  }
-}
-
-/*static*/ void
 APZThreadUtils::RunOnControllerThread(already_AddRefed<Runnable> aTask)
 {
   RefPtr<Runnable> task = aTask;
 
   if (!sControllerThread) {
     // Could happen on startup
     NS_WARNING("Dropping task posted to controller thread");
     return;
@@ -69,12 +61,45 @@ APZThreadUtils::RunOnControllerThread(al
 }
 
 /*static*/ bool
 APZThreadUtils::IsControllerThread()
 {
   return sControllerThread == MessageLoop::current();
 }
 
+/*static*/ void
+APZThreadUtils::AssertOnSamplerThread()
+{
+  if (GetThreadAssertionsEnabled()) {
+    MOZ_ASSERT(IsSamplerThread());
+  }
+}
+
+/*static*/ void
+APZThreadUtils::RunOnSamplerThread(already_AddRefed<Runnable> aTask)
+{
+  RefPtr<Runnable> task = aTask;
+
+  MessageLoop* loop = CompositorThreadHolder::Loop();
+  if (!loop) {
+    // Could happen during startup
+    NS_WARNING("Dropping task posted to sampler thread");
+    return;
+  }
+
+  if (IsSamplerThread()) {
+    task->Run();
+  } else {
+    loop->PostTask(task.forget());
+  }
+}
+
+/*static*/ bool
+APZThreadUtils::IsSamplerThread()
+{
+  return CompositorThreadHolder::IsInCompositorThread();
+}
+
 NS_IMPL_ISUPPORTS(GenericNamedTimerCallbackBase, nsITimerCallback, nsINamed)
 
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/apz/util/APZThreadUtils.h
+++ b/gfx/layers/apz/util/APZThreadUtils.h
@@ -36,33 +36,45 @@ public:
   /**
    * This can be used to assert that the current thread is the
    * controller/UI thread (on which input events are received).
    * This does nothing if thread assertions are disabled.
    */
   static void AssertOnControllerThread();
 
   /**
-   * This can be used to assert that the current thread is the
-   * sampler thread (which samples the async transform).
-   * This does nothing if thread assertions are disabled.
-   */
-  static void AssertOnSamplerThread();
-
-  /**
    * 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);
 
   /**
    * Returns true if currently on APZ "controller thread".
    */
   static bool IsControllerThread();
+
+  /**
+   * This can be used to assert that the current thread is the
+   * sampler thread (which samples the async transform).
+   * This does nothing if thread assertions are disabled.
+   */
+  static void AssertOnSamplerThread();
+
+  /**
+   * Runs the given task on the APZ "sampler thread" for this platform. If
+   * this function is called from the sampler thread itself then the task is
+   * run immediately without getting queued.
+   */
+  static void RunOnSamplerThread(already_AddRefed<Runnable> aTask);
+
+  /**
+   * Returns true if currently on the APZ "sampler thread".
+   */
+  static bool IsSamplerThread();
 };
 
 // A base class for GenericNamedTimerCallback<Function>.
 // This is necessary because NS_IMPL_ISUPPORTS doesn't work for a class
 // template.
 class GenericNamedTimerCallbackBase : public nsITimerCallback,
                                       public nsINamed
 {