Bug 1445662 - Ensure the keyboard map access is threadsafe. r?rhunt draft
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 15 Mar 2018 15:10:38 -0400
changeset 768167 db883c2615760445bfe1f28f547b0a7bf7c556bb
parent 768133 748f4a1baef345a1db72774393b3e45fcd40a9a1
child 768168 94adbe204fd9d908bfede0aa1fa8657fd03c5a8e
push id102813
push userkgupta@mozilla.com
push dateThu, 15 Mar 2018 19:11:06 +0000
reviewersrhunt
bugs1445662
milestone61.0a1
Bug 1445662 - Ensure the keyboard map access is threadsafe. r?rhunt - The change in APZCTreeManagerParent is functionally a no-op because it only ever runs in the GPU process on the controller thread. But it allows moving the controller thread to some other thread. - The change in nsBaseWidget is a no-op for desktop platforms, because in the UI process the main thread is the controller thread. But on Android it moves the call from the main thread to the Java UI thread. MozReview-Commit-ID: DZYyPZA2yQE
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
gfx/layers/ipc/APZCTreeManagerParent.cpp
widget/nsBaseWidget.cpp
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -1873,16 +1873,18 @@ APZCTreeManager::ProcessTouchVelocity(ui
   if (mApzcForInputBlock) {
     mApzcForInputBlock->HandleTouchVelocity(aTimestampMs, aSpeedY);
   }
 }
 
 void
 APZCTreeManager::SetKeyboardMap(const KeyboardMap& aKeyboardMap)
 {
+  APZThreadUtils::AssertOnControllerThread();
+
   mKeyboardMap = aKeyboardMap;
 }
 
 void
 APZCTreeManager::ZoomToRect(const ScrollableLayerGuid& aGuid,
                             const CSSRect& aRect,
                             const uint32_t aFlags)
 {
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -694,16 +694,17 @@ private:
    * 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. */
   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.
    */
   FocusState mFocusState;
   /* This tracks the APZC that should receive all inputs for the current input event block.
    * This allows touch points to move outside the thing they started on, but still have the
--- a/gfx/layers/ipc/APZCTreeManagerParent.cpp
+++ b/gfx/layers/ipc/APZCTreeManagerParent.cpp
@@ -161,17 +161,21 @@ APZCTreeManagerParent::RecvReceiveKeyboa
   *aOutEvent = event;
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvSetKeyboardMap(const KeyboardMap& aKeyboardMap)
 {
-  mTreeManager->SetKeyboardMap(aKeyboardMap);
+  APZThreadUtils::RunOnControllerThread(NewRunnableMethod<KeyboardMap>(
+    "layers::IAPZCTreeManager::SetKeyboardMap",
+    mTreeManager,
+    &IAPZCTreeManager::SetKeyboardMap,
+    aKeyboardMap));
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvZoomToRect(
     const ScrollableLayerGuid& aGuid,
     const CSSRect& aRect,
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@@ -928,24 +928,31 @@ already_AddRefed<GeckoContentController>
 nsBaseWidget::CreateRootContentController()
 {
   RefPtr<GeckoContentController> controller = new ChromeProcessController(this, mAPZEventState, mAPZC);
   return controller.forget();
 }
 
 void nsBaseWidget::ConfigureAPZCTreeManager()
 {
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mAPZC);
 
   ConfigureAPZControllerThread();
 
   mAPZC->SetDPI(GetDPI());
 
   if (gfxPrefs::APZKeyboardEnabled()) {
-    mAPZC->SetKeyboardMap(nsXBLWindowKeyHandler::CollectKeyboardShortcuts());
+    KeyboardMap map = nsXBLWindowKeyHandler::CollectKeyboardShortcuts();
+    // On Android the main thread is not the controller thread
+    APZThreadUtils::RunOnControllerThread(NewRunnableMethod<KeyboardMap>(
+        "layers::IAPZCTreeManager::SetKeyboardMap",
+        mAPZC,
+        &IAPZCTreeManager::SetKeyboardMap,
+        map));
   }
 
   RefPtr<IAPZCTreeManager> treeManager = mAPZC;  // for capture by the lambdas
 
   ContentReceivedInputBlockCallback callback(
       [treeManager](const ScrollableLayerGuid& aGuid,
                     uint64_t aInputBlockId,
                     bool aPreventDefault)