Bug 1551735 - Ref count and document CompositionRecorder r=kats
authorBarret Rennie <barret@brennie.ca>
Fri, 31 May 2019 00:30:24 +0000
changeset 476292 0c837987a8297e2a1106d4711f6e0e6f2bbb2654
parent 476291 a9e0369e5f2263aff731ee59981b48ce77013d38
child 476293 caca3a7d4c2f4468bf5503cbde79576f06aa7a10
push id36090
push usernbeleuzu@mozilla.com
push dateFri, 31 May 2019 03:59:09 +0000
treeherdermozilla-central@63568b2a8178 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1551735
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1551735 - Ref count and document CompositionRecorder r=kats The CompositionRecorder was being stored as a UniquePtr on the CompositorBridgeParent, but was then passed to and stored on the LayerManger as a raw pointer. This has been updated to use a RefPtr. Differential Revision: https://phabricator.services.mozilla.com/D32230
gfx/layers/CompositionRecorder.h
gfx/layers/composite/LayerManagerComposite.h
gfx/layers/ipc/CompositorBridgeParent.cpp
gfx/layers/ipc/CompositorBridgeParent.h
--- a/gfx/layers/CompositionRecorder.h
+++ b/gfx/layers/CompositionRecorder.h
@@ -15,16 +15,19 @@
 namespace mozilla {
 
 namespace gfx {
 class DataSourceSurface;
 }
 
 namespace layers {
 
+/**
+ * A captured frame from a |LayerManager|.
+ */
 class RecordedFrame {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(RecordedFrame)
 
   // The resulting DataSourceSurface must not be kept alive beyond the lifetime
   // of the RecordedFrame object, since it may refer to data owned by the frame.
   virtual already_AddRefed<gfx::DataSourceSurface> GetSourceSurface() = 0;
   TimeStamp GetTimeStamp() { return mTimeStamp; }
@@ -33,28 +36,44 @@ class RecordedFrame {
   virtual ~RecordedFrame() = default;
   RecordedFrame(const TimeStamp& aTimeStamp) : mTimeStamp(aTimeStamp) {}
 
  private:
   TimeStamp mTimeStamp;
 };
 
 /**
+ * A recorder for composited frames.
  *
+ * This object collects frames sent to it by a |LayerManager| and writes them
+ * out as a series of images until recording has finished.
+ *
+ * If GPU-accelerated rendering is used, the frames will not be mapped into
+ * memory until |WriteCollectedFrames| is called.
  */
 class CompositionRecorder final {
+  NS_INLINE_DECL_REFCOUNTING(CompositionRecorder)
+
  public:
   explicit CompositionRecorder(TimeStamp aRecordingStart);
-  ~CompositionRecorder();
 
+  /**
+   * Record a composited frame.
+   */
   void RecordFrame(RecordedFrame* aFrame);
 
+  /**
+   * Write out the collected frames as a series of timestamped images.
+   */
   void WriteCollectedFrames();
 
+ protected:
+  ~CompositionRecorder();
+
  private:
   nsTArray<RefPtr<RecordedFrame>> mCollectedFrames;
   TimeStamp mRecordingStart;
 };
 
 }  // namespace layers
 }  // namespace mozilla
 
-#endif  // mozilla_layers_ProfilerScreenshots_h
+#endif  // mozilla_layers_CompositionRecorder_h
--- a/gfx/layers/composite/LayerManagerComposite.h
+++ b/gfx/layers/composite/LayerManagerComposite.h
@@ -14,16 +14,17 @@
 #include "Units.h"               // for ParentLayerIntRect
 #include "mozilla/Assertions.h"  // for MOZ_ASSERT, etc
 #include "mozilla/Attributes.h"  // for override
 #include "mozilla/RefPtr.h"      // for RefPtr, already_AddRefed
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/Point.h"  // for IntSize
 #include "mozilla/gfx/Rect.h"   // for Rect
 #include "mozilla/gfx/Types.h"  // for SurfaceFormat
+#include "mozilla/layers/CompositionRecorder.h"
 #include "mozilla/layers/CompositorTypes.h"
 #include "mozilla/layers/Effects.h"  // for EffectChain
 #include "mozilla/layers/LayersMessages.h"
 #include "mozilla/layers/LayersTypes.h"  // for LayersBackend, etc
 #include "mozilla/Maybe.h"               // for Maybe
 #include "mozilla/RefPtr.h"
 #include "mozilla/UniquePtr.h"
 #include "nsAString.h"
@@ -48,17 +49,16 @@ namespace gfx {
 class DrawTarget;
 }  // namespace gfx
 
 namespace layers {
 
 class CanvasLayerComposite;
 class ColorLayerComposite;
 class Compositor;
-class CompositionRecorder;
 class ContainerLayerComposite;
 class Diagnostics;
 struct EffectChain;
 class ImageLayer;
 class ImageLayerComposite;
 class LayerComposite;
 class RefLayerComposite;
 class PaintedLayerComposite;
@@ -194,34 +194,34 @@ class HostLayerManager : public LayerMan
   // async compositables.
   uint64_t GetCompositorBridgeID() const { return mCompositorBridgeID; }
   void SetCompositorBridgeID(uint64_t aID) {
     MOZ_ASSERT(mCompositorBridgeID == 0,
                "The compositor ID must be set only once.");
     mCompositorBridgeID = aID;
   }
 
-  void SetCompositionRecorder(CompositionRecorder* aRecorder) {
+  void SetCompositionRecorder(already_AddRefed<CompositionRecorder> aRecorder) {
     mCompositionRecorder = aRecorder;
   }
 
  protected:
   bool mDebugOverlayWantsNextFrame;
   nsTArray<ImageCompositeNotificationInfo> mImageCompositeNotifications;
   // Testing property. If hardware composer is supported, this will return
   // true if the last frame was deemed 'too complicated' to be rendered.
   float mWarningLevel;
   mozilla::TimeStamp mWarnTime;
   UniquePtr<Diagnostics> mDiagnostics;
   uint64_t mCompositorBridgeID;
 
   bool mWindowOverlayChanged;
   TimeDuration mLastPaintTime;
   TimeStamp mRenderStartTime;
-  CompositionRecorder* mCompositionRecorder = nullptr;
+  RefPtr<CompositionRecorder> mCompositionRecorder = nullptr;
 
   // Render time for the current composition.
   TimeStamp mCompositionTime;
 
   // When nonnull, during rendering, some compositable indicated that it will
   // change its rendering at this time. In order not to miss it, we composite
   // on every vsync until this time occurs (this is the latest such time).
   TimeStamp mCompositeUntilTime;
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -2604,28 +2604,28 @@ int32_t RecordContentFrameTime(
     return result;
   }
 
   return 0;
 }
 
 mozilla::ipc::IPCResult CompositorBridgeParent::RecvBeginRecording(
     const TimeStamp& aRecordingStart) {
-  mCompositionRecorder.reset(new CompositionRecorder(aRecordingStart));
+  mCompositionRecorder = new CompositionRecorder(aRecordingStart);
 
   if (mLayerManager) {
-    mLayerManager->SetCompositionRecorder(mCompositionRecorder.get());
+    mLayerManager->SetCompositionRecorder(do_AddRef(mCompositionRecorder));
   }
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult CompositorBridgeParent::RecvEndRecording() {
   if (mLayerManager) {
     mLayerManager->SetCompositionRecorder(nullptr);
   }
   mCompositionRecorder->WriteCollectedFrames();
-  mCompositionRecorder.reset(nullptr);
+  mCompositionRecorder = nullptr;
   return IPC_OK();
 }
 
 }  // namespace layers
 }  // namespace mozilla
--- a/gfx/layers/ipc/CompositorBridgeParent.h
+++ b/gfx/layers/ipc/CompositorBridgeParent.h
@@ -22,16 +22,17 @@
 #include "mozilla/Maybe.h"
 #include "mozilla/Monitor.h"    // for Monitor
 #include "mozilla/RefPtr.h"     // for RefPtr
 #include "mozilla/TimeStamp.h"  // for TimeStamp
 #include "mozilla/dom/ipc/IdType.h"
 #include "mozilla/gfx/Point.h"  // for IntSize
 #include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/ipc/SharedMemory.h"
+#include "mozilla/layers/CompositionRecorder.h"
 #include "mozilla/layers/CompositorController.h"
 #include "mozilla/layers/CompositorOptions.h"
 #include "mozilla/layers/CompositorVsyncSchedulerOwner.h"
 #include "mozilla/layers/GeckoContentController.h"
 #include "mozilla/layers/ISurfaceAllocator.h"  // for ShmemAllocator
 #include "mozilla/layers/LayersMessages.h"     // for TargetConfig
 #include "mozilla/layers/MetricsSharingController.h"
 #include "mozilla/layers/PCompositorBridgeParent.h"
@@ -67,17 +68,16 @@ class ProtocolFuzzerHelper;
 namespace layers {
 
 class APZCTreeManager;
 class APZCTreeManagerParent;
 class APZSampler;
 class APZUpdater;
 class AsyncCompositionManager;
 class AsyncImagePipelineManager;
-class CompositionRecorder;
 class Compositor;
 class CompositorAnimationStorage;
 class CompositorBridgeParent;
 class CompositorManagerParent;
 class CompositorVsyncScheduler;
 class HostLayerManager;
 class IAPZCTreeManager;
 class LayerTransactionParent;
@@ -762,17 +762,17 @@ class CompositorBridgeParent final : pub
   RefPtr<APZUpdater> mApzUpdater;
 
   RefPtr<CompositorVsyncScheduler> mCompositorScheduler;
   // This makes sure the compositorParent is not destroyed before receiving
   // confirmation that the channel is closed.
   // mSelfRef is cleared in DeferredDestroy which is scheduled by ActorDestroy.
   RefPtr<CompositorBridgeParent> mSelfRef;
   RefPtr<CompositorAnimationStorage> mAnimationStorage;
-  UniquePtr<CompositionRecorder> mCompositionRecorder;
+  RefPtr<CompositionRecorder> mCompositionRecorder;
 
   TimeDuration mPaintTime;
 
 #if defined(XP_WIN) || defined(MOZ_WIDGET_GTK)
   // cached plugin data used to reduce the number of updates we request.
   LayersId mLastPluginUpdateLayerTreeId;
   nsIntPoint mPluginsLayerOffset;
   nsIntRegion mPluginsLayerVisibleRegion;