Bug 1310014 - Avoid capturing JS backtraces in TabChild::DidRequestComposite(); r=tromey
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 14 Oct 2016 15:01:49 -0400
changeset 318123 cb6b42803b04e80b7b1109813e535419d422e696
parent 318122 f2d64ec287a1c2d55e4b03209ea5ee392395b91d
child 318124 8769351b980ee57e0d0b4436808a2e272d5fe59b
push id30830
push usercbook@mozilla.com
push dateMon, 17 Oct 2016 09:19:24 +0000
treeherdermozilla-central@94b0fddf96b4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstromey
bugs1310014
milestone52.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 1310014 - Avoid capturing JS backtraces in TabChild::DidRequestComposite(); r=tromey
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
@@ -290,19 +290,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
@@ -3152,20 +3152,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());