Bug 1310014 - Avoid capturing JS backtraces in TabChild::DidRequestComposite(); r=tromey, a=gchang
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 14 Oct 2016 15:01:49 -0400
changeset 356160 5dc1d929beee261acd75a2920442b01e224071f0
parent 356159 f081853cf8df8282e62f33494b95dd370afd399c
child 356161 8fa339af80eac59858312fac0685cebb5bdb167e
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstromey, gchang
bugs1310014
milestone51.0a2
Bug 1310014 - Avoid capturing JS backtraces in TabChild::DidRequestComposite(); r=tromey, a=gchang
docshell/base/nsDocShell.h
docshell/base/timeline/TimelineConsumers.cpp
docshell/base/timeline/TimelineConsumers.h
dom/ipc/TabChild.cpp
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -291,19 +291,20 @@ private:
   // It is necessary to allow adding a timeline marker wherever a docshell
   // instance is available. This operation happens frequently and needs to
   // be very fast, so instead of using a Map or having to search for some
   // docshell-specific markers storage, a pointer to an `ObservedDocShell` is
   // is stored on docshells directly.
   friend void mozilla::TimelineConsumers::AddConsumer(nsDocShell*);
   friend void mozilla::TimelineConsumers::RemoveConsumer(nsDocShell*);
   friend void mozilla::TimelineConsumers::AddMarkerForDocShell(
-    nsDocShell*, const char*, MarkerTracingType);
+    nsDocShell*, const char*, MarkerTracingType, MarkerStackRequest);
   friend void mozilla::TimelineConsumers::AddMarkerForDocShell(
-    nsDocShell*, const char*, const TimeStamp&, MarkerTracingType);
+    nsDocShell*, const char*, const TimeStamp&, MarkerTracingType,
+    MarkerStackRequest);
   friend void mozilla::TimelineConsumers::AddMarkerForDocShell(
     nsDocShell*, UniquePtr<AbstractTimelineMarker>&&);
   friend void mozilla::TimelineConsumers::PopMarkers(nsDocShell*,
     JSContext*, nsTArray<dom::ProfileTimelineMarker>&);
 
 public:
   // Tell the favicon service that aNewURI has the same favicon as aOldURI.
   static void CopyFavicon(nsIURI* aOldURI,
--- a/docshell/base/timeline/TimelineConsumers.cpp
+++ b/docshell/base/timeline/TimelineConsumers.cpp
@@ -168,63 +168,67 @@ TimelineConsumers::IsEmpty()
 {
   StaticMutexAutoLock lock(sMutex); // for `mActiveConsumers`.
   return mActiveConsumers == 0;
 }
 
 void
 TimelineConsumers::AddMarkerForDocShell(nsDocShell* aDocShell,
                                         const char* aName,
-                                        MarkerTracingType aTracingType)
+                                        MarkerTracingType aTracingType,
+                                        MarkerStackRequest aStackRequest)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (HasConsumer(aDocShell)) {
-    aDocShell->mObserved->AddMarker(Move(MakeUnique<TimelineMarker>(aName, aTracingType)));
+    aDocShell->mObserved->AddMarker(Move(MakeUnique<TimelineMarker>(aName, aTracingType, aStackRequest)));
   }
 }
 
 void
 TimelineConsumers::AddMarkerForDocShell(nsDocShell* aDocShell,
                                         const char* aName,
                                         const TimeStamp& aTime,
-                                        MarkerTracingType aTracingType)
+                                        MarkerTracingType aTracingType,
+                                        MarkerStackRequest aStackRequest)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (HasConsumer(aDocShell)) {
-    aDocShell->mObserved->AddMarker(Move(MakeUnique<TimelineMarker>(aName, aTime, aTracingType)));
+    aDocShell->mObserved->AddMarker(Move(MakeUnique<TimelineMarker>(aName, aTime, aTracingType, aStackRequest)));
   }
 }
 
 void
 TimelineConsumers::AddMarkerForDocShell(nsDocShell* aDocShell,
                                         UniquePtr<AbstractTimelineMarker>&& aMarker)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (HasConsumer(aDocShell)) {
     aDocShell->mObserved->AddMarker(Move(aMarker));
   }
 }
 
 void
 TimelineConsumers::AddMarkerForDocShell(nsIDocShell* aDocShell,
                                         const char* aName,
-                                        MarkerTracingType aTracingType)
+                                        MarkerTracingType aTracingType,
+                                        MarkerStackRequest aStackRequest)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  AddMarkerForDocShell(static_cast<nsDocShell*>(aDocShell), aName, aTracingType);
+  AddMarkerForDocShell(static_cast<nsDocShell*>(aDocShell), aName, aTracingType, aStackRequest);
 }
 
 void
 TimelineConsumers::AddMarkerForDocShell(nsIDocShell* aDocShell,
                                         const char* aName,
                                         const TimeStamp& aTime,
-                                        MarkerTracingType aTracingType)
+                                        MarkerTracingType aTracingType,
+                                        MarkerStackRequest aStackRequest)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  AddMarkerForDocShell(static_cast<nsDocShell*>(aDocShell), aName, aTime, aTracingType);
+  AddMarkerForDocShell(static_cast<nsDocShell*>(aDocShell), aName, aTime, aTracingType, aStackRequest);
 }
 
 void
 TimelineConsumers::AddMarkerForDocShell(nsIDocShell* aDocShell,
                                         UniquePtr<AbstractTimelineMarker>&& aMarker)
 {
   MOZ_ASSERT(NS_IsMainThread());
   AddMarkerForDocShell(static_cast<nsDocShell*>(aDocShell), Move(aMarker));
--- a/docshell/base/timeline/TimelineConsumers.h
+++ b/docshell/base/timeline/TimelineConsumers.h
@@ -66,29 +66,33 @@ public:
   // created unless that docshell is specifically being currently observed.
   // See nsIDocShell::recordProfileTimelineMarkers
 
   // These methods create a basic TimelineMarker from a name and some metadata,
   // relevant for a specific docshell.
   // Main thread only.
   void AddMarkerForDocShell(nsDocShell* aDocShell,
                             const char* aName,
-                            MarkerTracingType aTracingType);
+                            MarkerTracingType aTracingType,
+                            MarkerStackRequest aStackRequest = MarkerStackRequest::STACK);
   void AddMarkerForDocShell(nsIDocShell* aDocShell,
                             const char* aName,
-                            MarkerTracingType aTracingType);
+                            MarkerTracingType aTracingType,
+                            MarkerStackRequest aStackRequest = MarkerStackRequest::STACK);
 
   void AddMarkerForDocShell(nsDocShell* aDocShell,
                             const char* aName,
                             const TimeStamp& aTime,
-                            MarkerTracingType aTracingType);
+                            MarkerTracingType aTracingType,
+                            MarkerStackRequest aStackRequest = MarkerStackRequest::STACK);
   void AddMarkerForDocShell(nsIDocShell* aDocShell,
                             const char* aName,
                             const TimeStamp& aTime,
-                            MarkerTracingType aTracingType);
+                            MarkerTracingType aTracingType,
+                            MarkerStackRequest aStackRequest = MarkerStackRequest::STACK);
 
   // These methods register and receive ownership of an already created marker,
   // relevant for a specific docshell.
   // Main thread only.
   void AddMarkerForDocShell(nsDocShell* aDocShell,
                             UniquePtr<AbstractTimelineMarker>&& aMarker);
   void AddMarkerForDocShell(nsIDocShell* aDocShell,
                             UniquePtr<AbstractTimelineMarker>&& aMarker);
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -3119,20 +3119,26 @@ TabChild::DidRequestComposite(const Time
   if (!docShellComPtr) {
     return;
   }
 
   nsDocShell* docShell = static_cast<nsDocShell*>(docShellComPtr.get());
   RefPtr<TimelineConsumers> timelines = TimelineConsumers::Get();
 
   if (timelines && timelines->HasConsumer(docShell)) {
+    // Since we're assuming that it's impossible for content JS to directly
+    // trigger a synchronous paint, we can avoid capturing a stack trace here,
+    // which means we won't run into JS engine reentrancy issues like bug
+    // 1310014.
     timelines->AddMarkerForDocShell(docShell,
-      "CompositeForwardTransaction", aCompositeReqStart, MarkerTracingType::START);
+      "CompositeForwardTransaction", aCompositeReqStart,
+      MarkerTracingType::START, MarkerStackRequest::NO_STACK);
     timelines->AddMarkerForDocShell(docShell,
-      "CompositeForwardTransaction", aCompositeReqEnd, MarkerTracingType::END);
+      "CompositeForwardTransaction", aCompositeReqEnd,
+      MarkerTracingType::END, MarkerStackRequest::NO_STACK);
   }
 }
 
 void
 TabChild::ClearCachedResources()
 {
   MOZ_ASSERT(mPuppetWidget);
   MOZ_ASSERT(mPuppetWidget->GetLayerManager());