Bug 1445662 - Ensure the keyboard map access is threadsafe. r?rhunt draft
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 14 Mar 2018 16:57:41 -0400
changeset 767603 dad4f9fe6f021624ed8bede25f093db66439f633
parent 767602 ccf21eada7beb4d14c59d451aa24b2cfea794806
child 767604 969b5dadb27639e76242e36c5508df373c46a918
push id102649
push userkgupta@mozilla.com
push dateWed, 14 Mar 2018 20:58:42 +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: LVVZLFxSuyj
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)