Bug 1240622 - Guard mCheckerboardEvent with a mutex to avoid races. r=botond

--HG--
extra : commitid : 9LLumoyfORc

gfx/layers/apz/src/AsyncPanZoomController.cpp

@@ -862,7 +862,8 @@ AsyncPanZoomController::AsyncPanZoomController(uint64_t aLayersId,
      mInputQueue(aInputQueue),
      mAPZCId(sAsyncPanZoomControllerCount++),
      mSharedLock(nullptr),
-     mAsyncTransformAppliedToContent(false)
+     mAsyncTransformAppliedToContent(false),
+     mCheckerboardEventLock("APZCBELock")
 {
   if (aGestures == USE_GESTURE_DETECTOR) {
     mGestureEventListener = new GestureEventListener(this);
@@ -2871,13 +2872,16 @@ AsyncPanZoomController::RequestContentRepaint(const FrameMetrics& aFrameMetrics,
   }
 
   APZC_LOG_FM(aFrameMetrics, "%p requesting content repaint", this);
-  if (mCheckerboardEvent && mCheckerboardEvent->IsRecordingTrace()) {
-    std::stringstream info;
-    info << " velocity " << aVelocity;
-    std::string str = info.str();
-    mCheckerboardEvent->UpdateRendertraceProperty(
-        CheckerboardEvent::RequestedDisplayPort, GetDisplayPortRect(aFrameMetrics),
-        str);
+  { // scope lock
+    MutexAutoLock lock(mCheckerboardEventLock);
+    if (mCheckerboardEvent && mCheckerboardEvent->IsRecordingTrace()) {
+      std::stringstream info;
+      info << " velocity " << aVelocity;
+      std::string str = info.str();
+      mCheckerboardEvent->UpdateRendertraceProperty(
+          CheckerboardEvent::RequestedDisplayPort, GetDisplayPortRect(aFrameMetrics),
+          str);
+    }
   }
 
   controller->RequestContentRepaint(aFrameMetrics);
@@ -2981,11 +2985,14 @@ bool AsyncPanZoomController::AdvanceAnimations(const TimeStamp& aSampleTime)
 
     requestAnimationFrame = UpdateAnimation(aSampleTime, &deferredTasks);
 
-    if (mCheckerboardEvent) {
-      mCheckerboardEvent->UpdateRendertraceProperty(
-          CheckerboardEvent::UserVisible,
-          CSSRect(mFrameMetrics.GetScrollOffset(),
-                  mFrameMetrics.CalculateCompositedSizeInCssPixels()));
+    { // scope lock
+      MutexAutoLock lock(mCheckerboardEventLock);
+      if (mCheckerboardEvent) {
+        mCheckerboardEvent->UpdateRendertraceProperty(
+            CheckerboardEvent::UserVisible,
+            CSSRect(mFrameMetrics.GetScrollOffset(),
+                    mFrameMetrics.CalculateCompositedSizeInCssPixels()));
+      }
     }
   }
 
@@ -3106,10 +3113,12 @@ AsyncPanZoomController::ReportCheckerboard(const TimeStamp& aSampleTime)
 
   bool recordTrace = gfxPrefs::APZRecordCheckerboarding();
   bool forTelemetry = Telemetry::CanRecordExtended();
+  uint32_t magnitude = GetCheckerboardMagnitude();
+
+  MutexAutoLock lock(mCheckerboardEventLock);
   if (!mCheckerboardEvent && (recordTrace || forTelemetry)) {
     mCheckerboardEvent = MakeUnique<CheckerboardEvent>(recordTrace);
   }
-  uint32_t magnitude = GetCheckerboardMagnitude();
   if (magnitude) {
     mPotentialCheckerboardTracker.CheckerboardSeen();
   }
@@ -3168,36 +3177,39 @@ void AsyncPanZoomController::NotifyLayersUpdated(const FrameMetrics& aLayerMetri
   APZC_LOG_FM(aLayerMetrics, "%p got a NotifyLayersUpdated with aIsFirstPaint=%d, aThisLayerTreeUpdated=%d",
     this, aIsFirstPaint, aThisLayerTreeUpdated);
 
-  if (mCheckerboardEvent && mCheckerboardEvent->IsRecordingTrace()) {
-    std::string str;
-    if (aThisLayerTreeUpdated) {
-      if (!aLayerMetrics.GetPaintRequestTime().IsNull()) {
-        // Note that we might get the paint request time as non-null, but with
-        // aThisLayerTreeUpdated false. That can happen if we get a layer transaction
-        // from a different process right after we get the layer transaction with
-        // aThisLayerTreeUpdated == true. In this case we want to ignore the
-        // paint request time because it was already dumped in the previous layer
-        // transaction.
-        TimeDuration paintTime = TimeStamp::Now() - aLayerMetrics.GetPaintRequestTime();
-        std::stringstream info;
-        info << " painttime " << paintTime.ToMilliseconds();
-        str = info.str();
-      } else {
-        // This might be indicative of a wasted paint particularly if it happens
-        // during a checkerboard event.
-        str = " (this layertree updated)";
+  { // scope lock
+    MutexAutoLock lock(mCheckerboardEventLock);
+    if (mCheckerboardEvent && mCheckerboardEvent->IsRecordingTrace()) {
+      std::string str;
+      if (aThisLayerTreeUpdated) {
+        if (!aLayerMetrics.GetPaintRequestTime().IsNull()) {
+          // Note that we might get the paint request time as non-null, but with
+          // aThisLayerTreeUpdated false. That can happen if we get a layer transaction
+          // from a different process right after we get the layer transaction with
+          // aThisLayerTreeUpdated == true. In this case we want to ignore the
+          // paint request time because it was already dumped in the previous layer
+          // transaction.
+          TimeDuration paintTime = TimeStamp::Now() - aLayerMetrics.GetPaintRequestTime();
+          std::stringstream info;
+          info << " painttime " << paintTime.ToMilliseconds();
+          str = info.str();
+        } else {
+          // This might be indicative of a wasted paint particularly if it happens
+          // during a checkerboard event.
+          str = " (this layertree updated)";
+        }
       }
-    }
-    mCheckerboardEvent->UpdateRendertraceProperty(
-        CheckerboardEvent::Page, aLayerMetrics.GetScrollableRect());
-    mCheckerboardEvent->UpdateRendertraceProperty(
-        CheckerboardEvent::PaintedDisplayPort,
-        aLayerMetrics.GetDisplayPort() + aLayerMetrics.GetScrollOffset(),
-        str);
-    if (!aLayerMetrics.GetCriticalDisplayPort().IsEmpty()) {
       mCheckerboardEvent->UpdateRendertraceProperty(
-          CheckerboardEvent::PaintedCriticalDisplayPort,
-          aLayerMetrics.GetCriticalDisplayPort() + aLayerMetrics.GetScrollOffset());
+          CheckerboardEvent::Page, aLayerMetrics.GetScrollableRect());
+      mCheckerboardEvent->UpdateRendertraceProperty(
+          CheckerboardEvent::PaintedDisplayPort,
+          aLayerMetrics.GetDisplayPort() + aLayerMetrics.GetScrollOffset(),
+          str);
+      if (!aLayerMetrics.GetCriticalDisplayPort().IsEmpty()) {
+        mCheckerboardEvent->UpdateRendertraceProperty(
+            CheckerboardEvent::PaintedCriticalDisplayPort,
+            aLayerMetrics.GetCriticalDisplayPort() + aLayerMetrics.GetScrollOffset());
+      }
     }
   }
 

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -1102,6 +1102,8 @@ private:
    * recording.
    */
 private:
+  // Mutex protecting mCheckerboardEvent
+  Mutex mCheckerboardEventLock;
   // This is created when this APZC instance is first included as part of a
   // composite. If a checkerboard event takes place, this is destroyed at the
   // end of the event, and a new one is created on the next composite.