# HG changeset patch # User Kartikaya Gupta # Date 1527710091 14400 # Node ID 85d13ab240face6ef86b27f7fc22e8b255fb7b72 # Parent bec42921ea20f68747c06d72bef90672c4201941 Bug 1457590 - Use the HitTestingTreeNodeAutoLock. r=botond This updates the GetTargetAPZC function to produce a HitTestingTreeNodeAutoLock out-param instead of a RefPtr, to ensure that the node can be used safely in calling functions. It then propagates that change outward as needed, which covers all the scrollbar dragging code. MozReview-Commit-ID: 43K4eSECb4E diff --git a/gfx/layers/apz/src/APZCTreeManager.cpp b/gfx/layers/apz/src/APZCTreeManager.cpp --- a/gfx/layers/apz/src/APZCTreeManager.cpp +++ b/gfx/layers/apz/src/APZCTreeManager.cpp @@ -1179,20 +1179,20 @@ APZCTreeManager::ReceiveInputEvent(Input if (startsDrag) { // If this is the start of a drag we need to unambiguously know if it's // going to land on a scrollbar or not. We can't apply an untransform // here without knowing that, so we need to ensure the untransform is // a no-op. FlushRepaintsToClearScreenToGeckoTransform(); } - RefPtr hitScrollbarNode = nullptr; + HitTestingTreeNodeAutoLock hitScrollbarNode; RefPtr apzc = GetTargetAPZC(mouseInput.mOrigin, &hitResult, &hitScrollbarNode); - bool hitScrollbar = hitScrollbarNode; + bool hitScrollbar = (bool)hitScrollbarNode; // When the mouse is outside the window we still want to handle dragging // but we won't find an APZC. Fallback to root APZC then. { // scope lock RecursiveMutexAutoLock lock(mTreeLock); if (!apzc && mRootNode) { apzc = mRootNode->GetApzc(); } @@ -1218,17 +1218,17 @@ APZCTreeManager::ReceiveInputEvent(Input } result = mInputQueue->ReceiveInputEvent( apzc, confFlags, mouseInput, aOutInputBlockId); // If we're starting an async scrollbar drag if (apzDragEnabled && startsDrag && hitScrollbarNode && hitScrollbarNode->IsScrollThumbNode() && hitScrollbarNode->GetScrollbarData().mThumbIsAsyncDraggable) { - SetupScrollbarDrag(mouseInput, hitScrollbarNode.get(), apzc.get()); + SetupScrollbarDrag(mouseInput, hitScrollbarNode, apzc.get()); } if (result == nsEventStatus_eConsumeDoDefault) { // This input event is part of a drag block, so whether or not it is // directed at a scrollbar depends on whether the drag block started // on a scrollbar. hitScrollbar = mInputQueue->IsDragOnScrollbar(hitScrollbar); } @@ -1527,17 +1527,17 @@ ConvertToTouchBehavior(CompositorHitTest } return result; } already_AddRefed APZCTreeManager::GetTouchInputBlockAPZC(const MultiTouchInput& aEvent, nsTArray* aOutTouchBehaviors, CompositorHitTestInfo* aOutHitResult, - RefPtr* aOutHitScrollbarNode) + HitTestingTreeNodeAutoLock* aOutHitScrollbarNode) { RefPtr apzc; if (aEvent.mTouches.Length() == 0) { return apzc.forget(); } FlushRepaintsToClearScreenToGeckoTransform(); @@ -1551,17 +1551,17 @@ APZCTreeManager::GetTouchInputBlockAPZC( RefPtr apzc2 = GetTargetAPZC(aEvent.mTouches[i].mScreenPoint, &hitResult); if (aOutTouchBehaviors) { aOutTouchBehaviors->AppendElement(ConvertToTouchBehavior(hitResult)); } apzc = GetMultitouchTarget(apzc, apzc2); APZCTM_LOG("Using APZC %p as the root APZC for multi-touch\n", apzc.get()); // A multi-touch gesture will not be a scrollbar drag, even if the // first touch point happened to hit a scrollbar. - *aOutHitScrollbarNode = nullptr; + aOutHitScrollbarNode->Clear(); } if (aOutHitResult) { // XXX we should probably be combining the hit results from the different // touch points somehow, instead of just using the last one. *aOutHitResult = hitResult; } return apzc.forget(); @@ -1569,17 +1569,17 @@ APZCTreeManager::GetTouchInputBlockAPZC( nsEventStatus APZCTreeManager::ProcessTouchInput(MultiTouchInput& aInput, ScrollableLayerGuid* aOutTargetGuid, uint64_t* aOutInputBlockId) { aInput.mHandledByAPZ = true; nsTArray touchBehaviors; - RefPtr hitScrollbarNode = nullptr; + HitTestingTreeNodeAutoLock hitScrollbarNode; if (aInput.mType == MultiTouchInput::MULTITOUCH_START) { // If we are panned into overscroll and a second finger goes down, // ignore that second touch point completely. The touch-start for it is // dropped completely; subsequent touch events until the touch-end for it // will have this touch point filtered out. // (By contrast, if we're in overscroll but not panning, such as after // putting two fingers down during an overscroll animation, we process the // second touch and proceed to pinch.) @@ -1616,17 +1616,17 @@ APZCTreeManager::ProcessTouchInput(Multi } } else if (mApzcForInputBlock) { APZCTM_LOG("Re-using APZC %p as continuation of event block\n", mApzcForInputBlock.get()); } nsEventStatus result = nsEventStatus_eIgnore; if (mInScrollbarTouchDrag) { - result = ProcessTouchInputForScrollbarDrag(aInput, hitScrollbarNode.get(), + result = ProcessTouchInputForScrollbarDrag(aInput, hitScrollbarNode, aOutTargetGuid, aOutInputBlockId); } else { // If we receive a touch-cancel, it means all touches are finished, so we // can stop ignoring any that we were ignoring. if (aInput.mType == MultiTouchInput::MULTITOUCH_CANCEL) { mRetainedTouchIdentifier = -1; } @@ -1712,17 +1712,17 @@ MultiTouchTypeToMouseType(MultiTouchInpu return MouseInput::MOUSE_UP; } MOZ_ASSERT_UNREACHABLE("Invalid multi-touch type"); return MouseInput::MOUSE_NONE; } nsEventStatus APZCTreeManager::ProcessTouchInputForScrollbarDrag(MultiTouchInput& aTouchInput, - const HitTestingTreeNode* aScrollThumbNode, + const HitTestingTreeNodeAutoLock& aScrollThumbNode, ScrollableLayerGuid* aOutTargetGuid, uint64_t* aOutInputBlockId) { MOZ_ASSERT(mRetainedTouchIdentifier == -1); MOZ_ASSERT(mApzcForInputBlock); MOZ_ASSERT(aTouchInput.mTouches.Length() == 1); // Synthesize a mouse event based on the touch event, so that we can @@ -1764,17 +1764,17 @@ APZCTreeManager::ProcessTouchInputForScr // additional explanation. aOutTargetGuid->mScrollId = FrameMetrics::NULL_SCROLL_ID; return result; } void APZCTreeManager::SetupScrollbarDrag(MouseInput& aMouseInput, - const HitTestingTreeNode* aScrollThumbNode, + const HitTestingTreeNodeAutoLock& aScrollThumbNode, AsyncPanZoomController* aApzc) { DragBlockState* dragBlock = mInputQueue->GetCurrentDragBlock(); if (!dragBlock) { return; } const ScrollbarData& thumbData = aScrollThumbNode->GetScrollbarData(); @@ -1802,17 +1802,17 @@ APZCTreeManager::SetupScrollbarDrag(Mous // the scroll track. Now get it relative to the thumb. // ScrollThumbData::mThumbStart stores the offset of the thumb // relative to the scroll track at the time of the last paint. // Since that paint, the thumb may have acquired an async transform // due to async scrolling, so look that up and apply it. LayerToParentLayerMatrix4x4 thumbTransform; { RecursiveMutexAutoLock lock(mTreeLock); - thumbTransform = ComputeTransformForNode(aScrollThumbNode); + thumbTransform = ComputeTransformForNode(aScrollThumbNode.Get(lock)); } // Only consider the translation, since we do not support both // zooming and scrollbar dragging on any platform. CSSCoord thumbStart = thumbData.mThumbStart + ((*thumbData.mDirection == ScrollDirection::eHorizontal) ? thumbTransform._41 : thumbTransform._42); dragStart -= thumbStart; @@ -2382,34 +2382,35 @@ APZCTreeManager::GetTargetNode(const Scr } ); return target.forget(); } already_AddRefed APZCTreeManager::GetTargetAPZC(const ScreenPoint& aPoint, CompositorHitTestInfo* aOutHitResult, - RefPtr* aOutScrollbarNode) + HitTestingTreeNodeAutoLock* aOutScrollbarNode) { RecursiveMutexAutoLock lock(mTreeLock); CompositorHitTestInfo hitResult = CompositorHitTestInfo::eInvisibleToHitTest; HitTestingTreeNode* scrollbarNode = nullptr; RefPtr target; if (gfx::gfxVars::UseWebRender() && gfxPrefs::WebRenderHitTest()) { target = GetAPZCAtPointWR(aPoint, &hitResult, &scrollbarNode); } else { target = GetAPZCAtPoint(mRootNode, aPoint, &hitResult, &scrollbarNode); } if (aOutHitResult) { *aOutHitResult = hitResult; } - if (aOutScrollbarNode) { - *aOutScrollbarNode = scrollbarNode; + if (aOutScrollbarNode && scrollbarNode) { + RefPtr scrollbarRef = scrollbarNode; + aOutScrollbarNode->Initialize(lock, scrollbarRef.forget(), mTreeLock); } return target.forget(); } already_AddRefed APZCTreeManager::GetAPZCAtPointWR(const ScreenPoint& aHitTestPoint, CompositorHitTestInfo* aOutHitResult, HitTestingTreeNode** aOutScrollbarNode) diff --git a/gfx/layers/apz/src/APZCTreeManager.h b/gfx/layers/apz/src/APZCTreeManager.h --- a/gfx/layers/apz/src/APZCTreeManager.h +++ b/gfx/layers/apz/src/APZCTreeManager.h @@ -51,16 +51,17 @@ class CompositorBridgeParent; class OverscrollHandoffChain; struct OverscrollHandoffState; class FocusTarget; struct FlingHandoffState; class LayerMetricsWrapper; class InputQueue; class GeckoContentController; class HitTestingTreeNode; +class HitTestingTreeNodeAutoLock; class WebRenderScrollDataWrapper; struct AncestorTransform; struct ScrollThumbData; /** * ****************** NOTE ON LOCK ORDERING IN APZ ************************** * * There are two main kinds of locks used by APZ: APZCTreeManager::mTreeLock @@ -579,17 +580,17 @@ public: lock the tree of APZCs while they find the right one, and then return an addref'd pointer to it. This allows caller code to just use the target APZC without worrying about it going away. These are public for testing code and generally should not be used by other production code. */ RefPtr GetRootNode() const; already_AddRefed GetTargetAPZC(const ScreenPoint& aPoint, gfx::CompositorHitTestInfo* aOutHitResult, - RefPtr* aOutScrollbarNode = nullptr); + HitTestingTreeNodeAutoLock* aOutScrollbarNode = nullptr); already_AddRefed GetTargetAPZC(const LayersId& aLayersId, const FrameMetrics::ViewID& aScrollId); ScreenToParentLayerMatrix4x4 GetScreenToApzcTransform(const AsyncPanZoomController *aApzc) const; ParentLayerToScreenMatrix4x4 GetApzcToGeckoTransform(const AsyncPanZoomController *aApzc) const; ScreenPoint GetCurrentMousePosition() const; /** * Process touch velocity. @@ -647,17 +648,17 @@ private: * potentially start a scrollbar drag), and a scrollbar node was hit, * that scrollbar node, otherwise nullptr. * * @return The APZC that was hit. */ already_AddRefed GetTouchInputBlockAPZC(const MultiTouchInput& aEvent, nsTArray* aOutTouchBehaviors, gfx::CompositorHitTestInfo* aOutHitResult, - RefPtr* aOutHitScrollbarNode); + HitTestingTreeNodeAutoLock* aOutHitScrollbarNode); nsEventStatus ProcessTouchInput(MultiTouchInput& aInput, ScrollableLayerGuid* aOutTargetGuid, uint64_t* aOutInputBlockId); /** * Given a mouse-down event that hit a scroll thumb node, set up APZ * dragging of the scroll thumb. * * Must be called after the mouse event has been sent to InputQueue. @@ -665,34 +666,34 @@ private: * @param aMouseInput The mouse-down event. * @param aScrollThumbNode Tthe scroll thumb node that was hit. * @param aApzc * The APZC for the scroll frame scrolled by the scroll thumb, if that * scroll frame is layerized. (A thumb can be layerized without its * target scroll frame being layerized.) Otherwise, an enclosing APZC. */ void SetupScrollbarDrag(MouseInput& aMouseInput, - const HitTestingTreeNode* aScrollThumbNode, + const HitTestingTreeNodeAutoLock& aScrollThumbNode, AsyncPanZoomController* aApzc); /** * Process a touch event that's part of a scrollbar touch-drag gesture. * * @param aInput The touch event. * @param aScrollThumbNode * If this is the touch-start event, the node representing the scroll * thumb we are starting to drag. Otherwise nullptr. * @param aOutTargetGuid * The guid of the APZC for the scroll frame whose scroll thumb is * being dragged. * @param aOutInputBlockId * The ID of the input block for the touch-drag gesture. * @return See ReceiveInputEvent() for what the return value means. */ nsEventStatus ProcessTouchInputForScrollbarDrag(MultiTouchInput& aInput, - const HitTestingTreeNode* aScrollThumbNode, + const HitTestingTreeNodeAutoLock& aScrollThumbNode, ScrollableLayerGuid* aOutTargetGuid, uint64_t* aOutInputBlockId); void FlushRepaintsToClearScreenToGeckoTransform(); already_AddRefed RecycleOrCreateNode(const RecursiveMutexAutoLock& aProofOfTreeLock, TreeBuildingState& aState, AsyncPanZoomController* aApzc, LayersId aLayersId); diff --git a/gfx/layers/apz/src/HitTestingTreeNode.h b/gfx/layers/apz/src/HitTestingTreeNode.h --- a/gfx/layers/apz/src/HitTestingTreeNode.h +++ b/gfx/layers/apz/src/HitTestingTreeNode.h @@ -224,16 +224,26 @@ public: HitTestingTreeNodeAutoLock(HitTestingTreeNodeAutoLock&&) = delete; ~HitTestingTreeNodeAutoLock(); void Initialize(const RecursiveMutexAutoLock& aProofOfTreeLock, already_AddRefed aNode, RecursiveMutex& aTreeMutex); void Clear(); + // Convenience operators to simplify the using code. + explicit operator bool() const { return !!mNode; } + bool operator!() const { return !mNode; } + HitTestingTreeNode* operator->() const { return mNode.get(); } + + // Allow getting back a raw pointer to the node, but only inside the scope + // of the tree lock. The caller is responsible for ensuring that they do not + // use the raw pointer outside that scope. + HitTestingTreeNode* Get(mozilla::RecursiveMutexAutoLock& aProofOfTreeLock) const { return mNode.get(); } + private: RefPtr mNode; RecursiveMutex* mTreeMutex; }; } // namespace layers } // namespace mozilla