Bug 1407458 - Use nsString for Observation filenames. r=erahm.

This avoids some unnecessary string copying.

--HG--
extra : rebase_source : a81a5e1612be2897a4285f5f395e7cb3ed006bc8

toolkit/components/telemetry/TelemetryIOInterposeObserver.cpp

@@ -47,10 +47,11 @@ void TelemetryIOInterposeObserver::Observe(Observation& aOb)
   }
 
   // Get the filename
-  const char16_t* filename = aOb.Filename();
+  nsAutoString filename;
+  aOb.Filename(filename);
 
   // Discard observations without filename
-  if (!filename) {
+  if (filename.IsEmpty()) {
     return;
   }
 
@@ -60,12 +61,11 @@ void TelemetryIOInterposeObserver::Observe(Observation& aOb)
   nsDefaultStringComparator comparator;
 #endif
   nsAutoString      processedName;
-  nsDependentString filenameStr(filename);
   uint32_t safeDirsLen = mSafeDirs.Length();
   for (uint32_t i = 0; i < safeDirsLen; ++i) {
-    if (StringBeginsWith(filenameStr, mSafeDirs[i].mPath, comparator)) {
+    if (StringBeginsWith(filename, mSafeDirs[i].mPath, comparator)) {
       processedName = mSafeDirs[i].mSubstName;
-      processedName += Substring(filenameStr, mSafeDirs[i].mPath.Length());
+      processedName += Substring(filename, mSafeDirs[i].mPath.Length());
       break;
     }
   }

tools/profiler/gecko/ProfilerIOInterposeObserver.cpp

@@ -16,14 +16,12 @@ void ProfilerIOInterposeObserver::Observe(Observation& aObservation)
 
   UniqueProfilerBacktrace stack = profiler_get_backtrace();
 
-  nsCString filename;
-  if (aObservation.Filename()) {
-    filename = NS_ConvertUTF16toUTF8(aObservation.Filename());
-  }
-
+  nsString filename;
+  aObservation.Filename(filename);
   profiler_add_marker(
     aObservation.ObservedOperationString(),
-    MakeUnique<IOMarkerPayload>(aObservation.Reference(), filename.get(),
+    MakeUnique<IOMarkerPayload>(aObservation.Reference(),
+                                NS_ConvertUTF16toUTF8(filename).get(),
                                 aObservation.Start(), aObservation.End(),
                                 Move(stack)));
 }

xpcom/build/IOInterposer.h

@@ -10,6 +10,7 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/GuardObjects.h"
 #include "mozilla/TimeStamp.h"
+#include "nsString.h"
 
 namespace mozilla {
 
@@ -94,8 +95,8 @@ public:
      */
     const char* Reference() const { return mReference; }
 
-    /** Request filename associated with the I/O operation, null if unknown */
-    virtual const char16_t* Filename() { return nullptr; }
+    /** Request filename associated with the I/O operation, empty if unknown */
+    virtual void Filename(nsAString& aString) { aString.Truncate(); }
 
     virtual ~Observation() {}
 

xpcom/build/MainThreadIOLogger.cpp

@@ -39,10 +39,7 @@ struct ObservationWithStack
     , mStack(aStack)
 #endif
   {
-    const char16_t* filename = aObs.Filename();
-    if (filename) {
-      mFilename = filename;
-    }
+    aObs.Filename(mFilename);
   }
 
   mozilla::IOInterposeObserver::Observation mObservation;

xpcom/build/PoisonIOInterposerMac.cpp

@@ -65,7 +65,6 @@ public:
                                        !IsDebugFile(aFd))
     , mFd(aFd)
     , mHasQueriedFilename(false)
-    , mFilename(nullptr)
   {
   }
 
@@ -76,49 +75,45 @@ public:
                                        IsValidWrite(aFd, aBuf, aCount))
     , mFd(aFd)
     , mHasQueriedFilename(false)
-    , mFilename(nullptr)
   {
   }
 
   // Custom implementation of IOInterposeObserver::Observation::Filename
-  const char16_t* Filename() override;
+  void Filename(nsAString& aFilename) override;
 
   ~MacIOAutoObservation()
   {
     Report();
-    if (mFilename) {
-      free(mFilename);
-      mFilename = nullptr;
-    }
   }
 
 private:
   int                 mFd;
   bool                mHasQueriedFilename;
-  char16_t*           mFilename;
+  nsString            mFilename;
   static const char*  sReference;
 };
 
 const char* MacIOAutoObservation::sReference = "PoisonIOInterposer";
 
 // Get filename for this observation
-const char16_t*
-MacIOAutoObservation::Filename()
+void
+MacIOAutoObservation::Filename(nsAString& aFilename)
 {
   // If mHasQueriedFilename is true, then we already have it
   if (mHasQueriedFilename) {
-    return mFilename;
+    aFilename = mFilename;
+    return;
   }
+
   char filename[MAXPATHLEN];
   if (fcntl(mFd, F_GETPATH, filename) != -1) {
-    mFilename = UTF8ToNewUnicode(nsDependentCString(filename));
+    mFilename = NS_ConvertUTF8toUTF16(filename);
   } else {
-    mFilename = nullptr;
+    mFilename.Truncate();
   }
   mHasQueriedFilename = true;
 
-  // Return filename
-  return mFilename;
+  aFilename = mFilename;
 }
 
 /****************************** Write Validation ******************************/

xpcom/build/PoisonIOInterposerWin.cpp

@@ -149,7 +149,6 @@ public:
         aOp, sReference, !IsDebugFile(reinterpret_cast<intptr_t>(aFileHandle)))
     , mFileHandle(aFileHandle)
     , mHasQueriedFilename(false)
-    , mFilename(nullptr)
   {
     if (mShouldReport) {
       mOffset.QuadPart = aOffset ? aOffset->QuadPart : 0;
@@ -160,12 +159,11 @@ public:
     : IOInterposeObserver::Observation(aOp, sReference)
     , mFileHandle(nullptr)
     , mHasQueriedFilename(false)
-    , mFilename(nullptr)
   {
     if (mShouldReport) {
       nsAutoString dosPath;
       if (NtPathToDosPath(aFilename, dosPath)) {
-        mFilename = ToNewUnicode(dosPath);
+        mFilename = dosPath;
         mHasQueriedFilename = true;
       }
       mOffset.QuadPart = 0;
@@ -173,46 +171,39 @@ public:
   }
 
   // Custom implementation of IOInterposeObserver::Observation::Filename
-  const char16_t* Filename() override;
+  void Filename(nsAString& aFilename) override;
 
   ~WinIOAutoObservation()
   {
     Report();
-    if (mFilename) {
-      MOZ_ASSERT(mHasQueriedFilename);
-      free(mFilename);
-      mFilename = nullptr;
-    }
   }
 
 private:
   HANDLE              mFileHandle;
   LARGE_INTEGER       mOffset;
   bool                mHasQueriedFilename;
-  char16_t*           mFilename;
+  nsString            mFilename;
   static const char*  sReference;
 };
 
 const char* WinIOAutoObservation::sReference = "PoisonIOInterposer";
 
 // Get filename for this observation
-const char16_t*
-WinIOAutoObservation::Filename()
+void
+WinIOAutoObservation::Filename(nsAString& aFilename)
 {
   // If mHasQueriedFilename is true, then filename is already stored in mFilename
   if (mHasQueriedFilename) {
-    return mFilename;
+    aFilename = mFilename;
   }
 
-  nsAutoString utf16Filename;
-  if (HandleToFilename(mFileHandle, mOffset, utf16Filename)) {
-    // Heap allocate with leakable memory
-    mFilename = ToNewUnicode(utf16Filename);
+  nsAutoString filename;
+  if (HandleToFilename(mFileHandle, mOffset, filename)) {
+    mFilename = filename;
   }
   mHasQueriedFilename = true;
 
-  // Return filename
-  return mFilename;
+  aFilename = mFilename;
 }
 
 /*************************** IO Interposing Methods ***************************/