Bug 937129 - Fix some concurrency issues in APZCTM::HandleOverscroll. r=kats

This commit is contained in:
Botond Ballo 2013-11-11 12:06:33 -05:00
parent db75b2138e
commit acc75156c8
3 changed files with 20 additions and 5 deletions

View File

@ -584,7 +584,15 @@ APZCTreeManager::ClearTree()
void void
APZCTreeManager::HandleOverscroll(AsyncPanZoomController* aChild, ScreenPoint aStartPoint, ScreenPoint aEndPoint) APZCTreeManager::HandleOverscroll(AsyncPanZoomController* aChild, ScreenPoint aStartPoint, ScreenPoint aEndPoint)
{ {
AsyncPanZoomController* parent = aChild->GetParent(); nsRefPtr<AsyncPanZoomController> parent;
{
// The tree lock needs to be held while navigating from an apzc to its
// parent. We don't hold it any longer though because GetInputTransforms()
// does its own locking, and AttemptScroll() can call HandleOverscroll()
// recursively.
MonitorAutoLock lock(mTreeLock);
parent = aChild->GetParent();
}
if (parent == nullptr) if (parent == nullptr)
return; return;
@ -597,7 +605,7 @@ APZCTreeManager::HandleOverscroll(AsyncPanZoomController* aChild, ScreenPoint aS
ApplyTransform(&aEndPoint, transformToApzc.Inverse()); ApplyTransform(&aEndPoint, transformToApzc.Inverse());
// Convert start and end points to parent's transformed screen coordinates. // Convert start and end points to parent's transformed screen coordinates.
GetInputTransforms(parent, transformToApzc, transformToGecko); GetInputTransforms(parent.get(), transformToApzc, transformToGecko);
ApplyTransform(&aStartPoint, transformToApzc); ApplyTransform(&aStartPoint, transformToApzc);
ApplyTransform(&aEndPoint, transformToApzc); ApplyTransform(&aEndPoint, transformToApzc);

View File

@ -862,8 +862,14 @@ void AsyncPanZoomController::AttemptScroll(const ScreenPoint& aStartPoint,
} }
if (fabs(overscroll.x) > EPSILON || fabs(overscroll.y) > EPSILON) { if (fabs(overscroll.x) > EPSILON || fabs(overscroll.y) > EPSILON) {
// "+ overscroll" rather than "- overscroll" for the same reason as above. // Make a local copy of the tree manager pointer and check if it's not
mTreeManager->HandleOverscroll(this, aEndPoint + overscroll, aEndPoint); // null before calling HandleOverscroll(). This is necessary because
// Destroy(), which nulls out mTreeManager, could be called concurrently.
APZCTreeManager* treeManagerLocal = mTreeManager;
if (treeManagerLocal) {
// "+ overscroll" rather than "- overscroll" for the same reason as above.
treeManagerLocal->HandleOverscroll(this, aEndPoint + overscroll, aEndPoint);
}
} }
} }

View File

@ -13,6 +13,7 @@
#include "mozilla/Monitor.h" #include "mozilla/Monitor.h"
#include "mozilla/ReentrantMonitor.h" #include "mozilla/ReentrantMonitor.h"
#include "mozilla/RefPtr.h" #include "mozilla/RefPtr.h"
#include "mozilla/Atomics.h"
#include "InputData.h" #include "InputData.h"
#include "Axis.h" #include "Axis.h"
#include "TaskThrottler.h" #include "TaskThrottler.h"
@ -648,7 +649,7 @@ private:
// live on the main thread, we can't use the cycle collector with them. // live on the main thread, we can't use the cycle collector with them.
// The APZCTreeManager owns the lifetime of the APZCs, so nulling this // The APZCTreeManager owns the lifetime of the APZCs, so nulling this
// pointer out in Destroy() will prevent accessing deleted memory. // pointer out in Destroy() will prevent accessing deleted memory.
APZCTreeManager* mTreeManager; Atomic<APZCTreeManager*> mTreeManager;
nsRefPtr<AsyncPanZoomController> mLastChild; nsRefPtr<AsyncPanZoomController> mLastChild;
nsRefPtr<AsyncPanZoomController> mPrevSibling; nsRefPtr<AsyncPanZoomController> mPrevSibling;