Bug 1447299 - Bounce PAPZCTreeManager controller messages through the sampler thread. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 28 Mar 2018 14:57:01 -0400
changeset 774006 a9ca5449ccbd298969d74b62b97a5e9c7d751ed6
parent 774005 a750744bd2203f3e7711ea5f88ab2b1c5fa5c8bb
child 774007 66ca68292896c2a1bff42990b01abc5448103d63
push id104365
push userkgupta@mozilla.com
push dateWed, 28 Mar 2018 18:59:12 +0000
reviewersbotond
bugs1447299
milestone61.0a1
Bug 1447299 - Bounce PAPZCTreeManager controller messages through the sampler thread. r?botond The methods on PAPZCTreeManagerParent are always invoked on the compositor thread, which currently is always the same as the sampler thread. When it does RunOnControllerThread calls, there is an implicit ordering with respect to the sampler thread, because the controller thread messages are always dispatched *from* the sampler thread. When we move the sampler thread to be the render backend thread, we have to preseve this implicit ordering, but we have to make it more explicit. For example, if a content process sends a layers update followed by a SetTargetAPZC message, it expects that the APZ will process them in that order, as the SetTargetAPZC might refer to a scrollframe that was just layerized. Currently, both these messages arrive on the compositor thread; the layers update is processed directly as the compositor thread is the sampler thread, and then the SetTargetAPZC message is bounced to the controller thread. However, if the compositor thread is not the sampler thread, then we would bounce the layers update to the sampler thread, and bounce the SetTargetAPZC to the controller thread, introducing a race. If SetTargetAPZC runs first bad things happen. The solution in this patch is to bounce the SetTargetAPZC to the sampler thread as well, so that it gets bounced to the controller thread only after we have processed the layers update. This patch accomplishes that by introducing a method on APZSampler that does this "double bounce". MozReview-Commit-ID: KI9wQQ9PZT4
gfx/layers/apz/public/APZSampler.h
gfx/layers/apz/src/APZSampler.cpp
gfx/layers/ipc/APZCTreeManagerParent.cpp
--- a/gfx/layers/apz/public/APZSampler.h
+++ b/gfx/layers/apz/public/APZSampler.h
@@ -2,16 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_layers_APZSampler_h
 #define mozilla_layers_APZSampler_h
 
+#include "base/message_loop.h"
 #include "LayersTypes.h"
 #include "mozilla/layers/APZTestData.h"
 #include "mozilla/layers/AsyncCompositionManager.h" // for AsyncTransform
 #include "mozilla/Maybe.h"
 #include "nsTArray.h"
 #include "Units.h"
 
 namespace mozilla {
@@ -120,16 +121,26 @@ public:
    */
   void RunOnSamplerThread(already_AddRefed<Runnable> aTask);
 
   /**
    * Returns true if currently on the APZSampler's "sampler thread".
    */
   bool IsSamplerThread();
 
+  /**
+   * Dispatches the given task to the APZ "controller thread", but does it *from*
+   * the sampler thread. That is, if the thread on which this function is called
+   * is not the sampler thread, the task is first dispatched to the sampler thread.
+   * When the sampler thread runs it (or if this is called directly on the sampler
+   * thread), that is when the task gets dispatched to the controller thread.
+   * The controller thread then actually runs the task.
+   */
+  void RunOnControllerThread(already_AddRefed<Runnable> aTask);
+
 protected:
   virtual ~APZSampler();
 
 private:
   RefPtr<APZCTreeManager> mApz;
 };
 
 } // namespace layers
--- a/gfx/layers/apz/src/APZSampler.cpp
+++ b/gfx/layers/apz/src/APZSampler.cpp
@@ -248,10 +248,21 @@ APZSampler::RunOnSamplerThread(already_A
 }
 
 bool
 APZSampler::IsSamplerThread()
 {
   return CompositorThreadHolder::IsInCompositorThread();
 }
 
+void
+APZSampler::RunOnControllerThread(already_AddRefed<Runnable> aTask)
+{
+  MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
+
+  RunOnSamplerThread(NewRunnableFunction(
+      "APZSampler::RunOnControllerThread",
+      &APZThreadUtils::RunOnControllerThread,
+      Move(aTask)));
+}
+
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/ipc/APZCTreeManagerParent.cpp
+++ b/gfx/layers/ipc/APZCTreeManagerParent.cpp
@@ -38,17 +38,17 @@ APZCTreeManagerParent::ChildAdopted(RefP
   MOZ_ASSERT(aAPZSampler->HasTreeManager(aAPZCTreeManager));
   mTreeManager = Move(aAPZCTreeManager);
   mSampler = Move(aAPZSampler);
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvSetKeyboardMap(const KeyboardMap& aKeyboardMap)
 {
-  APZThreadUtils::RunOnControllerThread(NewRunnableMethod<KeyboardMap>(
+  mSampler->RunOnControllerThread(NewRunnableMethod<KeyboardMap>(
     "layers::IAPZCTreeManager::SetKeyboardMap",
     mTreeManager,
     &IAPZCTreeManager::SetKeyboardMap,
     aKeyboardMap));
 
   return IPC_OK();
 }
 
@@ -59,31 +59,31 @@ APZCTreeManagerParent::RecvZoomToRect(
     const uint32_t& aFlags)
 {
   if (aGuid.mLayersId != mLayersId) {
     // Guard against bad data from hijacked child processes
     NS_ERROR("Unexpected layers id in RecvZoomToRect; dropping message...");
     return IPC_FAIL_NO_REASON(this);
   }
 
-  APZThreadUtils::RunOnControllerThread(
+  mSampler->RunOnControllerThread(
     NewRunnableMethod<ScrollableLayerGuid, CSSRect, uint32_t>(
       "layers::IAPZCTreeManager::ZoomToRect",
       mTreeManager,
       &IAPZCTreeManager::ZoomToRect,
       aGuid, aRect, aFlags));
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvContentReceivedInputBlock(
     const uint64_t& aInputBlockId,
     const bool& aPreventDefault)
 {
-  APZThreadUtils::RunOnControllerThread(NewRunnableMethod<uint64_t, bool>(
+  mSampler->RunOnControllerThread(NewRunnableMethod<uint64_t, bool>(
     "layers::IAPZCTreeManager::ContentReceivedInputBlock",
     mTreeManager,
     &IAPZCTreeManager::ContentReceivedInputBlock,
     aInputBlockId,
     aPreventDefault));
 
   return IPC_OK();
 }
@@ -95,17 +95,17 @@ APZCTreeManagerParent::RecvSetTargetAPZC
 {
   for (size_t i = 0; i < aTargets.Length(); i++) {
     if (aTargets[i].mLayersId != mLayersId) {
       // Guard against bad data from hijacked child processes
       NS_ERROR("Unexpected layers id in RecvSetTargetAPZC; dropping message...");
       return IPC_FAIL_NO_REASON(this);
     }
   }
-  APZThreadUtils::RunOnControllerThread(
+  mSampler->RunOnControllerThread(
     NewRunnableMethod<uint64_t,
                       StoreCopyPassByRRef<nsTArray<ScrollableLayerGuid>>>(
       "layers::IAPZCTreeManager::SetTargetAPZC",
       mTreeManager,
       &IAPZCTreeManager::SetTargetAPZC,
       aInputBlockId,
       aTargets));
 
@@ -125,30 +125,30 @@ APZCTreeManagerParent::RecvUpdateZoomCon
 
   mTreeManager->UpdateZoomConstraints(aGuid, aConstraints);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvSetDPI(const float& aDpiValue)
 {
-  APZThreadUtils::RunOnControllerThread(NewRunnableMethod<float>(
+  mSampler->RunOnControllerThread(NewRunnableMethod<float>(
     "layers::IAPZCTreeManager::SetDPI",
     mTreeManager,
     &IAPZCTreeManager::SetDPI,
     aDpiValue));
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvSetAllowedTouchBehavior(
     const uint64_t& aInputBlockId,
     nsTArray<TouchBehaviorFlags>&& aValues)
 {
-  APZThreadUtils::RunOnControllerThread(
+  mSampler->RunOnControllerThread(
     NewRunnableMethod<uint64_t,
                       StoreCopyPassByRRef<nsTArray<TouchBehaviorFlags>>>(
       "layers::IAPZCTreeManager::SetAllowedTouchBehavior",
       mTreeManager,
       &IAPZCTreeManager::SetAllowedTouchBehavior,
       aInputBlockId,
       Move(aValues)));
 
@@ -161,17 +161,17 @@ APZCTreeManagerParent::RecvStartScrollba
     const AsyncDragMetrics& aDragMetrics)
 {
   if (aGuid.mLayersId != mLayersId) {
     // Guard against bad data from hijacked child processes
     NS_ERROR("Unexpected layers id in RecvStartScrollbarDrag; dropping message...");
     return IPC_FAIL_NO_REASON(this);
   }
 
-  APZThreadUtils::RunOnControllerThread(
+  mSampler->RunOnControllerThread(
     NewRunnableMethod<ScrollableLayerGuid, AsyncDragMetrics>(
       "layers::IAPZCTreeManager::StartScrollbarDrag",
       mTreeManager,
       &IAPZCTreeManager::StartScrollbarDrag,
       aGuid,
       aDragMetrics));
 
   return IPC_OK();
@@ -184,45 +184,45 @@ APZCTreeManagerParent::RecvStartAutoscro
 {
   // Unlike RecvStartScrollbarDrag(), this message comes from the parent
   // process (via nsBaseWidget::mAPZC) rather than from the child process
   // (via TabChild::mApzcTreeManager), so there is no need to check the
   // layers id against mLayersId (and in any case, it wouldn't match, because
   // mLayersId stores the parent process's layers id, while nsBaseWidget is
   // sending the child process's layers id).
 
-  APZThreadUtils::RunOnControllerThread(
+  mSampler->RunOnControllerThread(
       NewRunnableMethod<ScrollableLayerGuid, ScreenPoint>(
         "layers::IAPZCTreeManager::StartAutoscroll",
         mTreeManager,
         &IAPZCTreeManager::StartAutoscroll,
         aGuid, aAnchorLocation));
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvStopAutoscroll(const ScrollableLayerGuid& aGuid)
 {
   // See RecvStartAutoscroll() for why we don't check the layers id.
 
-  APZThreadUtils::RunOnControllerThread(
+  mSampler->RunOnControllerThread(
       NewRunnableMethod<ScrollableLayerGuid>(
         "layers::IAPZCTreeManager::StopAutoscroll",
         mTreeManager,
         &IAPZCTreeManager::StopAutoscroll,
         aGuid));
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvSetLongTapEnabled(const bool& aLongTapEnabled)
 {
-  APZThreadUtils::RunOnControllerThread(
+  mSampler->RunOnControllerThread(
       NewRunnableMethod<bool>(
         "layers::IAPZCTreeManager::SetLongTapEnabled",
         mTreeManager,
         &IAPZCTreeManager::SetLongTapEnabled,
         aLongTapEnabled));
 
   return IPC_OK();
 }