Bug 1249389 - part 4 - make StartupCache::GetBuffer take a UniquePtr outparam; r=erahm
authorNathan Froyd <froydnj@mozilla.com>
Thu, 18 Feb 2016 12:26:28 -0500
changeset 285450 50332bf18a2fd8f6be9b69fb0e2482da8d528987
parent 285449 1f79d41d9098733aefe7da16c690ec4624ce4e7b
child 285451 bcfb36dfee0fea8de1555b658b6b04d30ccad263
push id30030
push usercbook@mozilla.com
push dateThu, 25 Feb 2016 10:58:04 +0000
treeherderautoland@c1e0d1890cfe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1249389
milestone47.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 1249389 - part 4 - make StartupCache::GetBuffer take a UniquePtr outparam; r=erahm This change eliminates a number of nsAutoArrayPtr usages, as well as making the pattern GetBuffer() -> NewObjectInputStreamFromBuffer more pleasant.
dom/xbl/nsXBLDocumentInfo.cpp
dom/xul/nsXULPrototypeCache.cpp
gfx/thebes/gfxFT2FontList.cpp
js/xpconnect/loader/mozJSLoaderUtils.cpp
modules/libjar/nsZipArchive.h
startupcache/StartupCache.cpp
startupcache/StartupCache.h
--- a/dom/xbl/nsXBLDocumentInfo.cpp
+++ b/dom/xbl/nsXBLDocumentInfo.cpp
@@ -193,26 +193,25 @@ nsXBLDocumentInfo::ReadPrototypeBindings
 
   nsAutoCString spec(kXBLCachePrefix);
   nsresult rv = PathifyURI(aURI, spec);
   NS_ENSURE_SUCCESS(rv, rv);
 
   StartupCache* startupCache = StartupCache::GetSingleton();
   NS_ENSURE_TRUE(startupCache, NS_ERROR_FAILURE);
 
-  nsAutoArrayPtr<char> buf;
+  UniquePtr<char[]> buf;
   uint32_t len;
-  rv = startupCache->GetBuffer(spec.get(), getter_Transfers(buf), &len);
+  rv = startupCache->GetBuffer(spec.get(), &buf, &len);
   // GetBuffer will fail if the binding is not in the cache.
   if (NS_FAILED(rv))
     return rv;
 
   nsCOMPtr<nsIObjectInputStream> stream;
-  rv = NewObjectInputStreamFromBuffer(UniquePtr<char[]>(buf.forget()),
-                                      len, getter_AddRefs(stream));
+  rv = NewObjectInputStreamFromBuffer(Move(buf), len, getter_AddRefs(stream));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // The file compatibility.ini stores the build id. This is checked in
   // nsAppRunner.cpp and will delete the cache if a different build is
   // present. However, we check that the version matches here to be safe. 
   uint32_t version;
   rv = stream->Read32(&version);
   NS_ENSURE_SUCCESS(rv, rv);
--- a/dom/xul/nsXULPrototypeCache.cpp
+++ b/dom/xul/nsXULPrototypeCache.cpp
@@ -329,29 +329,28 @@ nsXULPrototypeCache::WritePrototype(nsXU
 nsresult
 nsXULPrototypeCache::GetInputStream(nsIURI* uri, nsIObjectInputStream** stream)
 {
     nsAutoCString spec(kXULCachePrefix);
     nsresult rv = PathifyURI(uri, spec);
     if (NS_FAILED(rv))
         return NS_ERROR_NOT_AVAILABLE;
 
-    nsAutoArrayPtr<char> buf;
+    UniquePtr<char[]> buf;
     uint32_t len;
     nsCOMPtr<nsIObjectInputStream> ois;
     StartupCache* sc = StartupCache::GetSingleton();
     if (!sc)
         return NS_ERROR_NOT_AVAILABLE;
 
-    rv = sc->GetBuffer(spec.get(), getter_Transfers(buf), &len);
+    rv = sc->GetBuffer(spec.get(), &buf, &len);
     if (NS_FAILED(rv))
         return NS_ERROR_NOT_AVAILABLE;
 
-    rv = NewObjectInputStreamFromBuffer(UniquePtr<char[]>(buf.forget()),
-                                        len, getter_AddRefs(ois));
+    rv = NewObjectInputStreamFromBuffer(Move(buf), len, getter_AddRefs(ois));
     NS_ENSURE_SUCCESS(rv, rv);
 
     mInputStreamTable.Put(uri, ois);
 
     ois.forget(stream);
     return NS_OK;
 }
 
@@ -431,22 +430,22 @@ nsXULPrototypeCache::HasData(nsIURI* uri
         return NS_OK;
     }
     nsAutoCString spec(kXULCachePrefix);
     nsresult rv = PathifyURI(uri, spec);
     if (NS_FAILED(rv)) {
         *exists = false;
         return NS_OK;
     }
-    nsAutoArrayPtr<char> buf;
+    UniquePtr<char[]> buf;
     uint32_t len;
     StartupCache* sc = StartupCache::GetSingleton();
-    if (sc)
-        rv = sc->GetBuffer(spec.get(), getter_Transfers(buf), &len);
-    else {
+    if (sc) {
+        rv = sc->GetBuffer(spec.get(), &buf, &len);
+    } else {
         *exists = false;
         return NS_OK;
     }
     *exists = NS_SUCCEEDED(rv);
     return NS_OK;
 }
 
 nsresult
@@ -487,25 +486,24 @@ nsXULPrototypeCache::BeginCaching(nsIURI
         = do_GetService(NS_CHROMEREGISTRY_CONTRACTID, &rv);
     nsAutoCString locale;
     rv = chromeReg->GetSelectedLocale(package, locale);
     if (NS_FAILED(rv))
         return rv;
 
     nsAutoCString fileChromePath, fileLocale;
 
-    nsAutoArrayPtr<char> buf;
+    UniquePtr<char[]> buf;
     uint32_t len, amtRead;
     nsCOMPtr<nsIObjectInputStream> objectInput;
 
-    rv = startupCache->GetBuffer(kXULCacheInfoKey, getter_Transfers(buf),
-                                 &len);
+    rv = startupCache->GetBuffer(kXULCacheInfoKey, &buf, &len);
     if (NS_SUCCEEDED(rv))
-        rv = NewObjectInputStreamFromBuffer(UniquePtr<char[]>(buf.forget()),
-                                            len, getter_AddRefs(objectInput));
+        rv = NewObjectInputStreamFromBuffer(Move(buf), len,
+                                            getter_AddRefs(objectInput));
 
     if (NS_SUCCEEDED(rv)) {
         rv = objectInput->ReadCString(fileLocale);
         tmp = objectInput->ReadCString(fileChromePath);
         if (NS_FAILED(tmp)) {
           rv = tmp;
         }
         if (NS_FAILED(rv) ||
@@ -553,20 +551,20 @@ nsXULPrototypeCache::BeginCaching(nsIURI
               if (len64 <= UINT32_MAX)
                 len = (uint32_t)len64;
               else
                 rv = NS_ERROR_FILE_TOO_BIG;
             }
         }
 
         if (NS_SUCCEEDED(rv)) {
-            buf = new char[len];
-            rv = inputStream->Read(buf, len, &amtRead);
+            buf = MakeUnique<char[]>(len);
+            rv = inputStream->Read(buf.get(), len, &amtRead);
             if (NS_SUCCEEDED(rv) && len == amtRead)
-                rv = startupCache->PutBuffer(kXULCacheInfoKey, buf, len);
+              rv = startupCache->PutBuffer(kXULCacheInfoKey, buf.get(), len);
             else {
                 rv = NS_ERROR_UNEXPECTED;
             }
         }
 
         // Failed again, just bail.
         if (NS_FAILED(rv)) {
             startupCache->InvalidateCache();
--- a/gfx/thebes/gfxFT2FontList.cpp
+++ b/gfx/thebes/gfxFT2FontList.cpp
@@ -662,17 +662,17 @@ public:
     }
 
     void Init()
     {
         if (!mCache) {
             return;
         }
         uint32_t size;
-        char* buf;
+        UniquePtr<char[]> buf;
         if (NS_FAILED(mCache->GetBuffer(CACHE_KEY, &buf, &size))) {
             return;
         }
 
         LOG(("got: %s from the cache", nsDependentCString(buf, size).get()));
 
         const char* beginning = buf;
         const char* end = strchr(beginning, ';');
@@ -704,19 +704,16 @@ public:
                 // entries from the startupcache are marked "non-existing"
                 // until we have confirmed that the file still exists
                 mapEntry->mFileExists = false;
             }
 
             beginning = end + 1;
             end = strchr(beginning, ';');
         }
-
-        // Should we use free() or delete[] here? See bug 684700.
-        free(buf);
     }
 
     virtual void
     GetInfoForFile(const nsCString& aFileName, nsCString& aFaceList,
                    uint32_t *aTimestamp, uint32_t *aFilesize)
     {
         auto entry = static_cast<FNCMapEntry*>(mMap.Search(aFileName.get()));
         if (entry) {
@@ -948,28 +945,28 @@ gfxFT2FontList::AppendFacesFromFontFile(
 
 void
 gfxFT2FontList::FindFontsInOmnijar(FontNameCache *aCache)
 {
     bool jarChanged = false;
 
     mozilla::scache::StartupCache* cache =
         mozilla::scache::StartupCache::GetSingleton();
-    char *cachedModifiedTimeBuf;
+    UniquePtr<char[]> cachedModifiedTimeBuf;
     uint32_t longSize;
     int64_t jarModifiedTime;
     if (cache &&
         NS_SUCCEEDED(cache->GetBuffer(JAR_LAST_MODIFED_TIME,
                                       &cachedModifiedTimeBuf,
                                       &longSize)) &&
         longSize == sizeof(int64_t))
     {
         nsCOMPtr<nsIFile> jarFile = Omnijar::GetPath(Omnijar::Type::GRE);
         jarFile->GetLastModifiedTime(&jarModifiedTime);
-        if (jarModifiedTime > *(int64_t*)cachedModifiedTimeBuf) {
+        if (jarModifiedTime > *(int64_t*)cachedModifiedTimeBuf.get()) {
             jarChanged = true;
         }
     }
 
     static const char* sJarSearchPaths[] = {
         "res/fonts/*.ttf$",
     };
     RefPtr<nsZipArchive> reader = Omnijar::GetReader(Omnijar::Type::GRE);
--- a/js/xpconnect/loader/mozJSLoaderUtils.cpp
+++ b/js/xpconnect/loader/mozJSLoaderUtils.cpp
@@ -10,32 +10,32 @@
 #include "jsfriendapi.h"
 
 #include "nsJSPrincipals.h"
 
 #include "mozilla/scache/StartupCache.h"
 
 using namespace JS;
 using namespace mozilla::scache;
+using mozilla::UniquePtr;
 
 // We only serialize scripts with system principals. So we don't serialize the
 // principals when writing a script. Instead, when reading it back, we set the
 // principals to the system principals.
 nsresult
 ReadCachedScript(StartupCache* cache, nsACString& uri, JSContext* cx,
                  nsIPrincipal* systemPrincipal, MutableHandleScript scriptp)
 {
-    nsAutoArrayPtr<char> buf;
+    UniquePtr<char[]> buf;
     uint32_t len;
-    nsresult rv = cache->GetBuffer(PromiseFlatCString(uri).get(),
-                                   getter_Transfers(buf), &len);
+    nsresult rv = cache->GetBuffer(PromiseFlatCString(uri).get(), &buf, &len);
     if (NS_FAILED(rv))
         return rv; // don't warn since NOT_AVAILABLE is an ok error
 
-    scriptp.set(JS_DecodeScript(cx, buf, len));
+    scriptp.set(JS_DecodeScript(cx, buf.get(), len));
     if (!scriptp)
         return NS_ERROR_OUT_OF_MEMORY;
     return NS_OK;
 }
 
 nsresult
 ReadCachedFunction(StartupCache* cache, nsACString& uri, JSContext* cx,
                    nsIPrincipal* systemPrincipal, JSFunction** functionp)
--- a/modules/libjar/nsZipArchive.h
+++ b/modules/libjar/nsZipArchive.h
@@ -368,26 +368,26 @@ public:
     return Buffer();
   }
 
   /**
    * Relinquish ownership of zip member if compressed.
    * Copy member into a new buffer if uncompressed.
    * @return a buffer with whole zip member. It is caller's responsibility to free() it.
    */
-  T* Forget() {
+  mozilla::UniquePtr<T[]> Forget() {
     if (!mReturnBuf)
       return nullptr;
     // In uncompressed mmap case, give up buffer
     if (mAutoBuf.get() == mReturnBuf) {
       mReturnBuf = nullptr;
-      return (T*) mAutoBuf.release();
+      return mozilla::UniquePtr<T[]>(reinterpret_cast<T*>(mAutoBuf.release()));
     }
-    T *ret = (T*) malloc(Length());
-    memcpy(ret, mReturnBuf, Length());
+    auto ret = mozilla::MakeUnique<T[]>(Length());
+    memcpy(ret.get(), mReturnBuf, Length());
     mReturnBuf = nullptr;
     return ret;
   }
 };
 
 class nsZipHandle final
 {
 friend class nsZipArchive;
--- a/startupcache/StartupCache.cpp
+++ b/startupcache/StartupCache.cpp
@@ -279,17 +279,17 @@ StartupCache::LoadArchive(enum Telemetri
 
   return rv;
 }
 
 namespace {
 
 nsresult
 GetBufferFromZipArchive(nsZipArchive *zip, bool doCRC, const char* id,
-                        char** outbuf, uint32_t* length)
+                        UniquePtr<char[]>* outbuf, uint32_t* length)
 {
   if (!zip)
     return NS_ERROR_NOT_AVAILABLE;
 
   nsZipItemPtr<char> zipItem(zip, id, doCRC);
   if (!zipItem)
     return NS_ERROR_NOT_AVAILABLE;
 
@@ -298,30 +298,30 @@ GetBufferFromZipArchive(nsZipArchive *zi
   return NS_OK;
 }
 
 } /* anonymous namespace */
 
 // NOTE: this will not find a new entry until it has been written to disk!
 // Consumer should take ownership of the resulting buffer.
 nsresult
-StartupCache::GetBuffer(const char* id, char** outbuf, uint32_t* length) 
+StartupCache::GetBuffer(const char* id, UniquePtr<char[]>* outbuf, uint32_t* length) 
 {
   PROFILER_LABEL_FUNC(js::ProfileEntry::Category::OTHER);
 
   NS_ASSERTION(NS_IsMainThread(), "Startup cache only available on main thread");
 
   WaitOnWriteThread();
   if (!mStartupWriteInitiated) {
     CacheEntry* entry; 
     nsDependentCString idStr(id);
     mTable.Get(idStr, &entry);
     if (entry) {
-      *outbuf = new char[entry->size];
-      memcpy(*outbuf, entry->data, entry->size);
+      *outbuf = MakeUnique<char[]>(entry->size);
+      memcpy(outbuf->get(), entry->data, entry->size);
       *length = entry->size;
       return NS_OK;
     }
   }
 
   nsresult rv = GetBufferFromZipArchive(mArchive, true, id, outbuf, length);
   if (NS_SUCCEEDED(rv))
     return rv;
@@ -749,17 +749,20 @@ StartupCacheWrapper* StartupCacheWrapper
 
 nsresult 
 StartupCacheWrapper::GetBuffer(const char* id, char** outbuf, uint32_t* length) 
 {
   StartupCache* sc = StartupCache::GetSingleton();
   if (!sc) {
     return NS_ERROR_NOT_INITIALIZED;
   }
-  return sc->GetBuffer(id, outbuf, length);
+  UniquePtr<char[]> buf;
+  nsresult rv = sc->GetBuffer(id, &buf, length);
+  *outbuf = buf.release();
+  return rv;
 }
 
 nsresult
 StartupCacheWrapper::PutBuffer(const char* id, const char* inbuf, uint32_t length) 
 {
   StartupCache* sc = StartupCache::GetSingleton();
   if (!sc) {
     return NS_ERROR_NOT_INITIALIZED;
--- a/startupcache/StartupCache.h
+++ b/startupcache/StartupCache.h
@@ -15,16 +15,17 @@
 #include "nsIMemoryReporter.h"
 #include "nsIObserverService.h"
 #include "nsIObserver.h"
 #include "nsIOutputStream.h"
 #include "nsIFile.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/StaticPtr.h"
+#include "mozilla/UniquePtr.h"
 
 /**
  * The StartupCache is a persistent cache of simple key-value pairs,
  * where the keys are null-terminated c-strings and the values are 
  * arbitrary data, passed as a (char*, size) tuple. 
  *
  * Clients should use the GetSingleton() static method to access the cache. It 
  * will be available from the end of XPCOM init (NS_InitXPCOM3 in XPCOMInit.cpp), 
@@ -107,17 +108,17 @@ friend class StartupCacheWrapper;
 
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIMEMORYREPORTER
 
   // StartupCache methods. See above comments for a more detailed description.
 
   // Returns a buffer that was previously stored, caller takes ownership.
-  nsresult GetBuffer(const char* id, char** outbuf, uint32_t* length);
+  nsresult GetBuffer(const char* id, UniquePtr<char[]>* outbuf, uint32_t* length);
 
   // Stores a buffer. Caller keeps ownership, we make a copy.
   nsresult PutBuffer(const char* id, const char* inbuf, uint32_t length);
 
   // Removes the cache file.
   void InvalidateCache();
 
   // Signal that data should not be loaded from the cache file