Bug 1351920 - Remove the nsCString&& version of PROFILER_LABEL_DYNAMIC because it makes misleading promises about performance. r?njn draft
authorMarkus Stange <mstange@themasta.com>
Wed, 29 Mar 2017 21:47:51 -0400
changeset 554820 0c1c00abd486c3cb5d8631592dbf902622cda205
parent 554819 ea55f24e9b334324cea9f789e19a8736ece88a88
child 555278 a22479fbe96c56461a92481a9d50c783f79a0065
child 555623 d34b8907945a0233d599a47bc3e1801b06f4fb7c
push id52056
push userbmo:mstange@themasta.com
push dateSun, 02 Apr 2017 21:59:28 +0000
reviewersnjn
bugs1351920
milestone55.0a1
Bug 1351920 - Remove the nsCString&& version of PROFILER_LABEL_DYNAMIC because it makes misleading promises about performance. r?njn MozReview-Commit-ID: I4y5xnFyfUj
dom/events/EventListenerManager.cpp
layout/base/PresShell.cpp
tools/profiler/public/GeckoProfiler.h
--- a/dom/events/EventListenerManager.cpp
+++ b/dom/events/EventListenerManager.cpp
@@ -1270,19 +1270,20 @@ EventListenerManager::HandleEventInterna
             if (profiler_is_active()) {
 #ifdef MOZ_GECKO_PROFILER
               // Add a profiler label and a profiler marker for the actual
               // dispatch of the event.
               // This is a very hot code path, so we need to make sure not to
               // do this extra work when we're not profiling.
               nsAutoString typeStr;
               (*aDOMEvent)->GetType(typeStr);
+              NS_LossyConvertUTF16toASCII typeCStr(typeStr);
               PROFILER_LABEL_DYNAMIC("EventListenerManager", "HandleEventInternal",
                                      js::ProfileEntry::Category::EVENTS,
-                                     NS_LossyConvertUTF16toASCII(typeStr));
+                                     typeCStr.get());
               TimeStamp startTime = TimeStamp::Now();
 
               rv = HandleEventSubType(listener, *aDOMEvent, aCurrentTarget);
 
               TimeStamp endTime = TimeStamp::Now();
               uint16_t phase;
               (*aDOMEvent)->GetEventPhase(&phase);
               PROFILER_MARKER_PAYLOAD("DOMEvent",
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -6361,17 +6361,17 @@ PresShell::Paint(nsView*        aViewToP
 #ifdef MOZ_GECKO_PROFILER
   nsIURI* uri = mDocument->GetDocumentURI();
   nsIDocument* contentRoot = GetPrimaryContentDocument();
   if (contentRoot) {
     uri = contentRoot->GetDocumentURI();
   }
   nsCString uriString = uri ? uri->GetSpecOrDefault() : NS_LITERAL_CSTRING("N/A");
   PROFILER_LABEL_DYNAMIC("PresShell", "Paint",
-    js::ProfileEntry::Category::GRAPHICS, Move(uriString));
+    js::ProfileEntry::Category::GRAPHICS, uriString.get());
 #endif
 
   Maybe<js::AutoAssertNoContentJS> nojs;
 
   // On Android, Flash can call into content JS during painting, so we can't
   // assert there. However, we don't rely on this assertion on Android because
   // we don't paint while JS is running.
 #if !defined(MOZ_WIDGET_ANDROID)
@@ -9204,17 +9204,17 @@ PresShell::DoReflow(nsIFrame* target, bo
     nsSVGEffects::InvalidateDirectRenderingObservers(parent);
     parent = nsLayoutUtils::GetCrossDocParentFrame(parent);
   }
 
 #ifdef MOZ_GECKO_PROFILER
   nsIURI* uri = mDocument->GetDocumentURI();
   nsCString uriString = uri ? uri->GetSpecOrDefault() : NS_LITERAL_CSTRING("N/A");
   PROFILER_LABEL_DYNAMIC("PresShell", "DoReflow",
-    js::ProfileEntry::Category::GRAPHICS, Move(uriString));
+    js::ProfileEntry::Category::GRAPHICS, uriString.get());
 #endif
 
   nsDocShell* docShell = static_cast<nsDocShell*>(GetPresContext()->GetDocShell());
   RefPtr<TimelineConsumers> timelines = TimelineConsumers::Get();
   bool isTimelineRecording = timelines && timelines->HasConsumer(docShell);
 
   if (isTimelineRecording) {
     timelines->AddMarkerForDocShell(docShell, "Reflow", MarkerTracingType::START);
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -488,41 +488,25 @@ private:
 };
 
 class MOZ_RAII SamplerStackFrameDynamicRAII {
 public:
   SamplerStackFrameDynamicRAII(const char* aInfo,
     js::ProfileEntry::Category aCategory, uint32_t aLine,
     const char* aDynamicString)
   {
-    mHandle = Enter(aInfo, aCategory, aLine, aDynamicString);
-  }
-
-  // An alternative constructor that accepts an rvalue string and moves it
-  // into this object (without copying!).
-  SamplerStackFrameDynamicRAII(const char* aInfo,
-    js::ProfileEntry::Category aCategory, uint32_t aLine,
-    nsCString&& aDynamicString)
-    : mDynamicStorage(aDynamicString)
-  {
-    mHandle = Enter(aInfo, aCategory, aLine, mDynamicStorage.get());
+    mHandle = profiler_call_enter(aInfo, aCategory, this, true, aLine,
+                                  aDynamicString);
   }
 
   ~SamplerStackFrameDynamicRAII() {
     profiler_call_exit(mHandle);
   }
 
 private:
-  void* Enter(const char* aInfo, js::ProfileEntry::Category aCategory,
-              uint32_t aLine, const char* aDynamicString)
-  {
-    return profiler_call_enter(aInfo, aCategory, this, true, aLine, aDynamicString);
-  }
-
-  nsCString mDynamicStorage;
   void* mHandle;
 };
 
 } // namespace mozilla
 
 inline PseudoStack*
 profiler_get_pseudo_stack(void)
 {