Bug 1000633 - Avoid APZ data race in accessing mAnimation. r=kats, a=bajaj
authorBotond Ballo <botond@mozilla.com>
Fri, 25 Apr 2014 13:38:09 -0400
changeset 199339 406ff46c498b145fd560824a3633480bbd1bd418
parent 199338 ad2258fa2e1a44fb7f9a09ed829508225cef8551
child 199340 2c7882800cdd322de35efc4d06b324ce635f9f9e
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats, bajaj
bugs1000633
milestone31.0a2
Bug 1000633 - Avoid APZ data race in accessing mAnimation. r=kats, a=bajaj
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/AsyncPanZoomController.h
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -1597,36 +1597,44 @@ bool AsyncPanZoomController::SampleConte
                                                             ViewTransform* aNewTransform,
                                                             ScreenPoint& aScrollOffset) {
   // The eventual return value of this function. The compositor needs to know
   // whether or not to advance by a frame as soon as it can. For example, if a
   // fling is happening, it has to keep compositing so that the animation is
   // smooth. If an animation frame is requested, it is the compositor's
   // responsibility to schedule a composite.
   bool requestAnimationFrame = false;
+  Vector<Task*> deferredTasks;
 
   {
     ReentrantMonitorAutoEnter lock(mMonitor);
 
     requestAnimationFrame = UpdateAnimation(aSampleTime);
 
     aScrollOffset = mFrameMetrics.GetScrollOffset() * mFrameMetrics.GetZoom();
     *aNewTransform = GetCurrentAsyncTransform();
 
     LogRendertraceRect(GetGuid(), "viewport", "red",
       CSSRect(mFrameMetrics.GetScrollOffset(),
               ParentLayerSize(mFrameMetrics.mCompositionBounds.Size()) / mFrameMetrics.GetZoomToParent()));
 
     mCurrentAsyncScrollOffset = mFrameMetrics.GetScrollOffset();
+
+    // Get any deferred tasks queued up by mAnimation's Sample() (called by
+    // UpdateAnimation()). This needs to be done here since mAnimation can
+    // be destroyed by another thread when we release the monitor, but
+    // the tasks need to be executed after we release the monitor since they
+    // are allowed to call APZCTreeManager methods which can grab the tree lock. 
+    if (mAnimation) {
+      deferredTasks = mAnimation->TakeDeferredTasks();
+    }
   }
 
-  // Execute tasks queued up by mAnimation's Sample() (called by
-  // UpdateAnimation()) for execution after mMonitor has been released.
-  if (mAnimation) {
-    mAnimation->ExecuteDeferredTasks();
+  for (uint32_t i = 0; i < deferredTasks.length(); ++i) {
+    deferredTasks[i]->Run();
   }
 
   // Cancel the mAsyncScrollTimeoutTask because we will fire a
   // mozbrowserasyncscroll event or renew the mAsyncScrollTimeoutTask again.
   if (mAsyncScrollTimeoutTask) {
     mAsyncScrollTimeoutTask->Cancel();
     mAsyncScrollTimeoutTask = nullptr;
   }
--- a/gfx/layers/apz/src/AsyncPanZoomController.h
+++ b/gfx/layers/apz/src/AsyncPanZoomController.h
@@ -943,24 +943,24 @@ public:
                         TimeDuration::Forever())
     : mRepaintInterval(aRepaintInterval)
   { }
 
   virtual bool Sample(FrameMetrics& aFrameMetrics,
                       const TimeDuration& aDelta) = 0;
 
   /**
-   * Execute the tasks in |mDeferredTasks| in order. See |mDeferredTasks|
+   * Get the deferred tasks in |mDeferredTasks|. See |mDeferredTasks|
    * for more information.
+   * Clears |mDeferredTasks|.
    */
-  void ExecuteDeferredTasks() {
-    for (uint32_t i = 0; i < mDeferredTasks.length(); ++i) {
-      mDeferredTasks[i]->Run();
-    }
-    mDeferredTasks.clear();
+  Vector<Task*> TakeDeferredTasks() {
+    Vector<Task*> result;
+    mDeferredTasks.swap(result);
+    return result;
   }
 
   /**
    * Specifies how frequently (at most) we want to do repaints during the
    * animation sequence. TimeDuration::Forever() will cause it to only repaint
    * at the end of the animation.
    */
   TimeDuration mRepaintInterval;