Bug 1465350 - Use UniquePtr instead of ScopedJSFreePtr for JSErrorReporter. r=Waldo
authorAndré Bargull <andre.bargull@gmail.com>
Wed, 13 Jun 2018 09:47:21 -0700
changeset 422800 6350b1a6097e821b836c7a83924259f3b64d0b70
parent 422799 1d498636e0d5b5090691cb05dd7c0af09b4c2949
child 422801 3e478d973ae21a40cd4e871fbb889d76753ca56c
push id34148
push userccoroiu@mozilla.com
push dateSun, 17 Jun 2018 09:46:48 +0000
treeherdermozilla-central@c40cc0a89bc7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1465350
milestone62.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 1465350 - Use UniquePtr instead of ScopedJSFreePtr for JSErrorReporter. r=Waldo
js/src/jsapi.cpp
js/src/jsexn.cpp
js/src/jsexn.h
js/src/vm/ErrorObject.cpp
js/src/vm/ErrorObject.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -7088,17 +7088,17 @@ JSErrorNotes::copy(JSContext* cx)
 {
     auto copiedNotes = MakeUnique<JSErrorNotes>();
     if (!copiedNotes) {
         ReportOutOfMemory(cx);
         return nullptr;
     }
 
     for (auto&& note : *this) {
-        js::UniquePtr<JSErrorNotes::Note> copied(CopyErrorNote(cx, note.get()));
+        UniquePtr<JSErrorNotes::Note> copied = CopyErrorNote(cx, note.get());
         if (!copied)
             return nullptr;
 
         if (!copiedNotes->notes_.append(std::move(copied)))
             return nullptr;
     }
 
     return copiedNotes;
--- a/js/src/jsexn.cpp
+++ b/js/src/jsexn.cpp
@@ -9,25 +9,27 @@
  */
 
 #include "jsexn.h"
 
 #include "mozilla/ScopeExit.h"
 #include "mozilla/Sprintf.h"
 
 #include <string.h>
+#include <utility>
 
 #include "jsapi.h"
 #include "jsnum.h"
 #include "jstypes.h"
 #include "jsutil.h"
 
 #include "gc/FreeOp.h"
 #include "gc/Marking.h"
 #include "js/CharacterEncoding.h"
+#include "js/UniquePtr.h"
 #include "js/Wrapper.h"
 #include "util/StringBuffer.h"
 #include "vm/ErrorObject.h"
 #include "vm/GlobalObject.h"
 #include "vm/JSContext.h"
 #include "vm/JSFunction.h"
 #include "vm/JSObject.h"
 #include "vm/JSScript.h"
@@ -262,17 +264,17 @@ CopyExtraData(JSContext* cx, uint8_t** c
 
 bool
 CopyExtraData(JSContext* cx, uint8_t** cursor, JSErrorNotes::Note* copy, JSErrorNotes::Note* report)
 {
     return true;
 }
 
 template <typename T>
-static T*
+static UniquePtr<T>
 CopyErrorHelper(JSContext* cx, T* report)
 {
     /*
      * We use a single malloc block to make a deep copy of JSErrorReport or
      * JSErrorNotes::Note, except JSErrorNotes linked from JSErrorReport with
      * the following layout:
      *   JSErrorReport or JSErrorNotes::Note
      *   char array with characters for message_
@@ -293,54 +295,51 @@ CopyErrorHelper(JSContext* cx, T* report
      * The mallocSize can not overflow since it represents the sum of the
      * sizes of already allocated objects.
      */
     size_t mallocSize = sizeof(T) + messageSize + filenameSize + ExtraMallocSize(report);
     uint8_t* cursor = cx->pod_calloc<uint8_t>(mallocSize);
     if (!cursor)
         return nullptr;
 
-    T* copy = new (cursor) T();
+    UniquePtr<T> copy(new (cursor) T());
     cursor += sizeof(T);
 
     if (report->message()) {
         copy->initBorrowedMessage((const char*)cursor);
         js_memcpy(cursor, report->message().c_str(), messageSize);
         cursor += messageSize;
     }
 
     if (report->filename) {
         copy->filename = (const char*)cursor;
         js_memcpy(cursor, report->filename, filenameSize);
         cursor += filenameSize;
     }
 
-    if (!CopyExtraData(cx, &cursor, copy, report)) {
-        /* js_delete calls destructor for T and js_free for pod_calloc. */
-        js_delete(copy);
+    if (!CopyExtraData(cx, &cursor, copy.get(), report))
         return nullptr;
-    }
 
-    MOZ_ASSERT(cursor == (uint8_t*)copy + mallocSize);
+    MOZ_ASSERT(cursor == (uint8_t*)copy.get() + mallocSize);
 
     /* Copy non-pointer members. */
     copy->lineno = report->lineno;
     copy->column = report->column;
     copy->errorNumber = report->errorNumber;
 
     return copy;
 }
 
-JSErrorNotes::Note*
+UniquePtr<JSErrorNotes::Note>
 js::CopyErrorNote(JSContext* cx, JSErrorNotes::Note* note)
 {
     return CopyErrorHelper(cx, note);
 }
 
-JSErrorReport*
+UniquePtr<JSErrorReport>
 js::CopyErrorReport(JSContext* cx, JSErrorReport* report)
 {
     return CopyErrorHelper(cx, report);
 }
 
 struct SuppressErrorsGuard
 {
     JSContext* cx;
@@ -681,22 +680,22 @@ js::ErrorToException(JSContext* cx, JSEr
 
     uint32_t lineNumber = reportp->lineno;
     uint32_t columnNumber = reportp->column;
 
     RootedObject stack(cx);
     if (!CaptureStack(cx, &stack))
         return;
 
-    js::ScopedJSFreePtr<JSErrorReport> report(CopyErrorReport(cx, reportp));
+    UniquePtr<JSErrorReport> report = CopyErrorReport(cx, reportp);
     if (!report)
         return;
 
-    RootedObject errObject(cx, ErrorObject::create(cx, exnType, stack, fileName,
-                                                   lineNumber, columnNumber, &report, messageStr));
+    RootedObject errObject(cx, ErrorObject::create(cx, exnType, stack, fileName, lineNumber,
+                                                   columnNumber, std::move(report), messageStr));
     if (!errObject)
         return;
 
     // Throw it.
     cx->setPendingException(ObjectValue(*errObject));
 
     // Flag the error report passed in to indicate an exception was raised.
     reportp->flags |= JSREPORT_EXCEPTION;
@@ -985,17 +984,17 @@ ErrorReport::populateUncaughtExceptionRe
     toStringResult_ = ownedReport.message();
     reportp = &ownedReport;
     return true;
 }
 
 JSObject*
 js::CopyErrorObject(JSContext* cx, Handle<ErrorObject*> err)
 {
-    js::ScopedJSFreePtr<JSErrorReport> copyReport;
+    UniquePtr<JSErrorReport> copyReport;
     if (JSErrorReport* errorReport = err->getErrorReport()) {
         copyReport = CopyErrorReport(cx, errorReport);
         if (!copyReport)
             return nullptr;
     }
 
     RootedString message(cx, err->getMessage());
     if (message && !cx->compartment()->wrap(cx, &message))
@@ -1007,34 +1006,36 @@ js::CopyErrorObject(JSContext* cx, Handl
     if (!cx->compartment()->wrap(cx, &stack))
         return nullptr;
     uint32_t lineNumber = err->lineNumber();
     uint32_t columnNumber = err->columnNumber();
     JSExnType errorType = err->type();
 
     // Create the Error object.
     return ErrorObject::create(cx, errorType, stack, fileName,
-                               lineNumber, columnNumber, &copyReport, message);
+                               lineNumber, columnNumber, std::move(copyReport), message);
 }
 
 JS_PUBLIC_API(bool)
 JS::CreateError(JSContext* cx, JSExnType type, HandleObject stack, HandleString fileName,
                     uint32_t lineNumber, uint32_t columnNumber, JSErrorReport* report,
                     HandleString message, MutableHandleValue rval)
 {
     assertSameCompartment(cx, stack, fileName, message);
     AssertObjectIsSavedFrameOrWrapper(cx, stack);
 
-    js::ScopedJSFreePtr<JSErrorReport> rep;
-    if (report)
+    js::UniquePtr<JSErrorReport> rep;
+    if (report) {
         rep = CopyErrorReport(cx, report);
+        if (!rep)
+            return false;
+    }
 
-    RootedObject obj(cx,
-        js::ErrorObject::create(cx, type, stack, fileName,
-                                lineNumber, columnNumber, &rep, message));
+    JSObject* obj = js::ErrorObject::create(cx, type, stack, fileName, lineNumber, columnNumber,
+                                            std::move(rep), message);
     if (!obj)
         return false;
 
     rval.setObject(*obj);
     return true;
 }
 
 const char*
--- a/js/src/jsexn.h
+++ b/js/src/jsexn.h
@@ -9,25 +9,26 @@
  */
 
 #ifndef jsexn_h
 #define jsexn_h
 
 #include "jsapi.h"
 #include "NamespaceImports.h"
 
+#include "js/UniquePtr.h"
 #include "vm/JSContext.h"
 
 namespace js {
 class ErrorObject;
 
-JSErrorNotes::Note*
+UniquePtr<JSErrorNotes::Note>
 CopyErrorNote(JSContext* cx, JSErrorNotes::Note* note);
 
-JSErrorReport*
+UniquePtr<JSErrorReport>
 CopyErrorReport(JSContext* cx, JSErrorReport* report);
 
 JSString*
 ComputeStackString(JSContext* cx);
 
 /*
  * Given a JSErrorReport, check to see if there is an exception associated with
  * the error number.  If there is, then create an appropriate exception object,
--- a/js/src/vm/ErrorObject.cpp
+++ b/js/src/vm/ErrorObject.cpp
@@ -4,16 +4,18 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "vm/ErrorObject-inl.h"
 
 #include "mozilla/Range.h"
 
+#include <utility>
+
 #include "jsexn.h"
 
 #include "js/CallArgs.h"
 #include "js/CharacterEncoding.h"
 #include "vm/GlobalObject.h"
 #include "vm/SelfHosting.h"
 #include "vm/StringType.h"
 
@@ -33,17 +35,17 @@ js::ErrorObject::assignInitialShape(JSCo
         return nullptr;
     if (!NativeObject::addDataProperty(cx, obj, cx->names().lineNumber, LINENUMBER_SLOT, 0))
         return nullptr;
     return NativeObject::addDataProperty(cx, obj, cx->names().columnNumber, COLUMNNUMBER_SLOT, 0);
 }
 
 /* static */ bool
 js::ErrorObject::init(JSContext* cx, Handle<ErrorObject*> obj, JSExnType type,
-                      ScopedJSFreePtr<JSErrorReport>* errorReport, HandleString fileName,
+                      UniquePtr<JSErrorReport> errorReport, HandleString fileName,
                       HandleObject stack, uint32_t lineNumber, uint32_t columnNumber,
                       HandleString message)
 {
     AssertObjectIsSavedFrameOrWrapper(cx, stack);
     assertSameCompartment(cx, obj, stack);
 
     // Null out early in case of error, for exn_finalize's sake.
     obj->initReservedSlot(ERROR_REPORT_SLOT, PrivateValue(nullptr));
@@ -67,33 +69,33 @@ js::ErrorObject::init(JSContext* cx, Han
     MOZ_ASSERT(obj->lookupPure(NameToId(cx->names().lineNumber))->slot() == LINENUMBER_SLOT);
     MOZ_ASSERT(obj->lookupPure(NameToId(cx->names().columnNumber))->slot() ==
                COLUMNNUMBER_SLOT);
     MOZ_ASSERT_IF(message,
                   obj->lookupPure(NameToId(cx->names().message))->slot() == MESSAGE_SLOT);
 
     MOZ_ASSERT(JSEXN_ERR <= type && type < JSEXN_LIMIT);
 
-    JSErrorReport* report = errorReport ? errorReport->forget() : nullptr;
+    JSErrorReport* report = errorReport.release();
     obj->initReservedSlot(EXNTYPE_SLOT, Int32Value(type));
     obj->initReservedSlot(STACK_SLOT, ObjectOrNullValue(stack));
     obj->setReservedSlot(ERROR_REPORT_SLOT, PrivateValue(report));
     obj->initReservedSlot(FILENAME_SLOT, StringValue(fileName));
     obj->initReservedSlot(LINENUMBER_SLOT, Int32Value(lineNumber));
     obj->initReservedSlot(COLUMNNUMBER_SLOT, Int32Value(columnNumber));
     if (message)
         obj->setSlotWithType(cx, messageShape, StringValue(message));
 
     return true;
 }
 
 /* static */ ErrorObject*
 js::ErrorObject::create(JSContext* cx, JSExnType errorType, HandleObject stack,
                         HandleString fileName, uint32_t lineNumber, uint32_t columnNumber,
-                        ScopedJSFreePtr<JSErrorReport>* report, HandleString message,
+                        UniquePtr<JSErrorReport> report, HandleString message,
                         HandleObject protoArg /* = nullptr */)
 {
     AssertObjectIsSavedFrameOrWrapper(cx, stack);
 
     RootedObject proto(cx, protoArg);
     if (!proto) {
         proto = GlobalObject::getOrCreateCustomErrorPrototype(cx, cx->global(), errorType);
         if (!proto)
@@ -104,17 +106,17 @@ js::ErrorObject::create(JSContext* cx, J
     {
         const Class* clasp = ErrorObject::classForType(errorType);
         JSObject* obj = NewObjectWithGivenProto(cx, clasp, proto);
         if (!obj)
             return nullptr;
         errObject = &obj->as<ErrorObject>();
     }
 
-    if (!ErrorObject::init(cx, errObject, errorType, report, fileName, stack,
+    if (!ErrorObject::init(cx, errObject, errorType, std::move(report), fileName, stack,
                            lineNumber, columnNumber, message))
     {
         return nullptr;
     }
 
     return errObject;
 }
 
@@ -151,21 +153,21 @@ js::ErrorObject::getOrCreateErrorReport(
         return nullptr;
 
     UniqueChars utf8 = StringToNewUTF8CharsZ(cx, *message);
     if (!utf8)
         return nullptr;
     report.initOwnedMessage(utf8.release());
 
     // Cache and return.
-    JSErrorReport* copy = CopyErrorReport(cx, &report);
+    UniquePtr<JSErrorReport> copy = CopyErrorReport(cx, &report);
     if (!copy)
         return nullptr;
-    setReservedSlot(ERROR_REPORT_SLOT, PrivateValue(copy));
-    return copy;
+    setReservedSlot(ERROR_REPORT_SLOT, PrivateValue(copy.get()));
+    return copy.release();
 }
 
 static bool
 FindErrorInstanceOrPrototype(JSContext* cx, HandleObject obj, MutableHandleObject result)
 {
     // Walk up the prototype chain until we find an error object instance or
     // prototype object. This allows code like:
     //  Object.create(Error.prototype).stack
--- a/js/src/vm/ErrorObject.h
+++ b/js/src/vm/ErrorObject.h
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef vm_ErrorObject_h_
 #define vm_ErrorObject_h_
 
 #include "mozilla/ArrayUtils.h"
 
+#include "js/UniquePtr.h"
 #include "vm/NativeObject.h"
 #include "vm/SavedStacks.h"
 #include "vm/Shape.h"
 
 namespace js {
 
 /*
  * Initialize the exception constructor/prototype hierarchy.
@@ -30,17 +31,17 @@ class ErrorObject : public NativeObject
     createConstructor(JSContext* cx, JSProtoKey key);
 
     /* For access to createProto. */
     friend JSObject*
     js::InitExceptionClasses(JSContext* cx, HandleObject global);
 
     static bool
     init(JSContext* cx, Handle<ErrorObject*> obj, JSExnType type,
-         ScopedJSFreePtr<JSErrorReport>* errorReport, HandleString fileName, HandleObject stack,
+         UniquePtr<JSErrorReport> errorReport, HandleString fileName, HandleObject stack,
          uint32_t lineNumber, uint32_t columnNumber, HandleString message);
 
     static const ClassSpec classSpecs[JSEXN_ERROR_LIMIT];
     static const Class protoClasses[JSEXN_ERROR_LIMIT];
 
   protected:
     static const uint32_t EXNTYPE_SLOT          = 0;
     static const uint32_t STACK_SLOT            = EXNTYPE_SLOT + 1;
@@ -65,17 +66,17 @@ class ErrorObject : public NativeObject
     }
 
     // Create an error of the given type corresponding to the provided location
     // info.  If |message| is non-null, then the error will have a .message
     // property with that value; otherwise the error will have no .message
     // property.
     static ErrorObject*
     create(JSContext* cx, JSExnType type, HandleObject stack, HandleString fileName,
-           uint32_t lineNumber, uint32_t columnNumber, ScopedJSFreePtr<JSErrorReport>* report,
+           uint32_t lineNumber, uint32_t columnNumber, UniquePtr<JSErrorReport> report,
            HandleString message, HandleObject proto = nullptr);
 
     /*
      * Assign the initial error shape to the empty object.  (This shape does
      * *not* include .message, which must be added separately if needed; see
      * ErrorObject::init.)
      */
     static Shape*