Bug 1183235 - Keep the dochsell-specific markers inside a ObservedDocShell managed list instead of nsDocShell, r=smaug

docshell/base/nsDocShell.cpp

@@ -2937,7 +2937,6 @@ nsDocShell::SetRecordProfileTimelineMarkers(bool aValue)
     } else {
       TimelineConsumers::RemoveConsumer(this, mObserved);
       UnuseEntryScriptProfiling();
-      ClearProfileTimelineMarkers();
     }
   }
 
@@ -2968,13 +2967,23 @@ nsDocShell::PopProfileTimelineMarkers(
   SequenceRooter<mozilla::dom::ProfileTimelineMarker> rooter(
     aCx, &profileTimelineMarkers);
 
+  if (!IsObserved()) {
+    if (!ToJSValue(aCx, profileTimelineMarkers, aProfileTimelineMarkers)) {
+      JS_ClearPendingException(aCx);
+      return NS_ERROR_UNEXPECTED;
+    }
+    return NS_OK;
+  }
+
+  nsTArray<UniquePtr<TimelineMarker>>& markersStore = mObserved.get()->mTimelineMarkers;
+
   // If we see an unpaired START, we keep it around for the next call
   // to PopProfileTimelineMarkers.  We store the kept START objects in
   // this array.
   nsTArray<UniquePtr<TimelineMarker>> keptMarkers;
 
-  for (uint32_t i = 0; i < mProfileTimelineMarkers.Length(); ++i) {
+  for (uint32_t i = 0; i < markersStore.Length(); ++i) {
-    UniquePtr<TimelineMarker>& startPayload = mProfileTimelineMarkers[i];
+    UniquePtr<TimelineMarker>& startPayload = markersStore[i];
     const char* startMarkerName = startPayload->GetName();
 
     bool hasSeenPaintedLayer = false;
@@ -3010,8 +3019,8 @@ nsDocShell::PopProfileTimelineMarkers(
       // The assumption is that the devtools timeline flushes markers frequently
       // enough for the amount of markers to always be small enough that the
       // nested for loop isn't going to be a performance problem.
-      for (uint32_t j = i + 1; j < mProfileTimelineMarkers.Length(); ++j) {
+      for (uint32_t j = i + 1; j < markersStore.Length(); ++j) {
-        UniquePtr<TimelineMarker>& endPayload = mProfileTimelineMarkers[j];
+        UniquePtr<TimelineMarker>& endPayload = markersStore[j];
         const char* endMarkerName = endPayload->GetName();
 
         // Look for Layer markers to stream out paint markers.
@@ -3057,14 +3066,14 @@ nsDocShell::PopProfileTimelineMarkers(
 
       // If we did not see the corresponding END, keep the START.
       if (!hasSeenEnd) {
-        keptMarkers.AppendElement(Move(mProfileTimelineMarkers[i]));
+        keptMarkers.AppendElement(Move(markersStore[i]));
-        mProfileTimelineMarkers.RemoveElementAt(i);
+        markersStore.RemoveElementAt(i);
         --i;
       }
     }
   }
 
-  mProfileTimelineMarkers.SwapElements(keptMarkers);
+  markersStore.SwapElements(keptMarkers);
 
   if (!ToJSValue(aCx, profileTimelineMarkers, aProfileTimelineMarkers)) {
     JS_ClearPendingException(aCx);
@@ -3088,8 +3097,7 @@ nsDocShell::AddProfileTimelineMarker(const char* aName,
                                      TracingMetadata aMetaData)
 {
   if (IsObserved()) {
-    TimelineMarker* marker = new TimelineMarker(this, aName, aMetaData);
+    mObserved->AddMarker(aName, aMetaData);
-    mProfileTimelineMarkers.AppendElement(marker);
   }
 }
 
@@ -3097,7 +3105,7 @@ void
 nsDocShell::AddProfileTimelineMarker(UniquePtr<TimelineMarker>&& aMarker)
 {
   if (IsObserved()) {
-    mProfileTimelineMarkers.AppendElement(Move(aMarker));
+    mObserved->AddMarker(Move(aMarker));
   }
 }
 
@@ -3130,12 +3138,6 @@ nsDocShell::GetWindowDraggingAllowed(bool* aValue)
   return NS_OK;
 }
 
-void
-nsDocShell::ClearProfileTimelineMarkers()
-{
-  mProfileTimelineMarkers.Clear();
-}
-
 nsIDOMStorageManager*
 nsDocShell::TopSessionStorageManager()
 {

docshell/base/nsDocShell.h

@@ -977,11 +977,6 @@ private:
   // has been called without a matching NotifyRunToCompletionStop.
   uint32_t mJSRunToCompletionDepth;
 
-  nsTArray<mozilla::UniquePtr<TimelineMarker>> mProfileTimelineMarkers;
-
-  // Get rid of all the timeline markers accumulated so far
-  void ClearProfileTimelineMarkers();
-
   // Separate function to do the actual name (i.e. not _top, _self etc.)
   // searching for FindItemWithName.
   nsresult DoFindItemWithName(const char16_t* aName,

docshell/base/timeline/ObservedDocShell.cpp

@@ -6,10 +6,32 @@
 
 #include "ObservedDocShell.h"
 
+#include "TimelineMarker.h"
+#include "mozilla/Move.h"
+
 namespace mozilla {
 
 ObservedDocShell::ObservedDocShell(nsDocShell* aDocShell)
   : mDocShell(aDocShell)
 {}
 
+void
+ObservedDocShell::AddMarker(const char* aName, TracingMetadata aMetaData)
+{
+  TimelineMarker* marker = new TimelineMarker(mDocShell, aName, aMetaData);
+  mTimelineMarkers.AppendElement(marker);
+}
+
+void
+ObservedDocShell::AddMarker(UniquePtr<TimelineMarker>&& aMarker)
+{
+  mTimelineMarkers.AppendElement(Move(aMarker));
+}
+
+void
+ObservedDocShell::ClearMarkers()
+{
+  mTimelineMarkers.Clear();
+}
+
 } // namespace mozilla

docshell/base/timeline/ObservedDocShell.h

@@ -7,9 +7,12 @@
 #ifndef ObservedDocShell_h_
 #define ObservedDocShell_h_
 
+#include "GeckoProfiler.h"
+#include "nsTArray.h"
 #include "nsRefPtr.h"
 
 class nsDocShell;
+class TimelineMarker;
 
 namespace mozilla {
 
@@ -23,8 +26,16 @@ private:
   nsRefPtr<nsDocShell> mDocShell;
 
 public:
+  // FIXME: make this private once all marker-specific logic has been
+  // moved out of nsDocShell.
+  nsTArray<UniquePtr<TimelineMarker>> mTimelineMarkers;
+
   explicit ObservedDocShell(nsDocShell* aDocShell);
   nsDocShell* operator*() const { return mDocShell.get(); }
+
+  void AddMarker(const char* aName, TracingMetadata aMetaData);
+  void AddMarker(UniquePtr<TimelineMarker>&& aMarker);
+  void ClearMarkers();
 };
 
 } // namespace mozilla

docshell/base/timeline/TimelineConsumers.cpp

@@ -36,6 +36,7 @@ TimelineConsumers::RemoveConsumer(nsDocShell* aDocShell,
 {
   MOZ_ASSERT(aObservedPtr);
   sActiveConsumers--;
+  aObservedPtr.get()->ClearMarkers();
   aObservedPtr.get()->remove();
   aObservedPtr.reset(nullptr);
 }