Bug 1370869 - Don't copy scope data in XDR decode r=shu
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 12 Jun 2017 10:43:46 +0100
changeset 412198 d54918df1a29caa779c5149014c5ddc3c79f3b94
parent 412197 9b6d3584c9b871d23b858d5728eae7d81dbd80b7
child 412199 0ec77a61f0f25ac4ad63a2bf35be8c3ebec339c6
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1370869
milestone55.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 1370869 - Don't copy scope data in XDR decode r=shu
js/public/RootingAPI.h
js/src/vm/Scope.cpp
js/src/vm/Scope.h
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -157,16 +157,20 @@ struct PersistentRootedMarker;
 // Assignment operators on a base class are hidden by the implicitly defined
 // operator= on the derived class. Thus, define the operator= directly on the
 // class as we would need to manually pass it through anyway.
 #define DECLARE_POINTER_ASSIGN_OPS(Wrapper, T)                                                    \
     Wrapper<T>& operator=(const T& p) {                                                           \
         set(p);                                                                                   \
         return *this;                                                                             \
     }                                                                                             \
+    Wrapper<T>& operator=(T&& p) {                                                                \
+        set(mozilla::Move(p));                                                                    \
+        return *this;                                                                             \
+    }                                                                                             \
     Wrapper<T>& operator=(const Wrapper<T>& other) {                                              \
         set(other.get());                                                                         \
         return *this;                                                                             \
     }                                                                                             \
 
 #define DELETE_ASSIGNMENT_OPS(Wrapper, T)                                                         \
     template <typename S> Wrapper<T>& operator=(S) = delete;                                      \
     Wrapper<T>& operator=(const Wrapper<T>&) = delete;
@@ -563,16 +567,19 @@ class MOZ_STACK_CLASS MutableHandle : pu
   private:
     // Disallow nullptr for overloading purposes.
     MutableHandle(decltype(nullptr)) = delete;
 
   public:
     void set(const T& v) {
         *ptr = v;
     }
+    void set(T&& v) {
+        *ptr = mozilla::Move(v);
+    }
 
     /*
      * This may be called only if the location of the T is guaranteed
      * to be marked (for some reason other than being a Rooted),
      * e.g., if it is guaranteed to be reachable from an implicit root.
      *
      * Create a MutableHandle from a raw location of a T.
      */
@@ -823,16 +830,19 @@ class MOZ_RAII Rooted : public js::Roote
 
     /*
      * This method is public for Rooted so that Codegen.py can use a Rooted
      * interchangeably with a MutableHandleValue.
      */
     void set(const T& value) {
         ptr = value;
     }
+    void set(T&& value) {
+        ptr = mozilla::Move(value);
+    }
 
     DECLARE_POINTER_CONSTREF_OPS(T);
     DECLARE_POINTER_ASSIGN_OPS(Rooted, T);
     DECLARE_NONPOINTER_ACCESSOR_METHODS(ptr);
     DECLARE_NONPOINTER_MUTABLE_ACCESSOR_METHODS(ptr);
 
   private:
     /*
@@ -1269,26 +1279,30 @@ namespace js {
 
 template <typename T, typename D, typename Container>
 class WrappedPtrOperations<UniquePtr<T, D>, Container>
 {
     const UniquePtr<T, D>& uniquePtr() const { return static_cast<const Container*>(this)->get(); }
 
   public:
     explicit operator bool() const { return !!uniquePtr(); }
+    T* get() const { return uniquePtr().get(); }
+    T* operator->() const { return get(); }
+    T& operator*() const { return *uniquePtr(); }
 };
 
 template <typename T, typename D, typename Container>
 class MutableWrappedPtrOperations<UniquePtr<T, D>, Container>
   : public WrappedPtrOperations<UniquePtr<T, D>, Container>
 {
     UniquePtr<T, D>& uniquePtr() { return static_cast<Container*>(this)->get(); }
 
   public:
     MOZ_MUST_USE typename UniquePtr<T, D>::Pointer release() { return uniquePtr().release(); }
+    void reset(T* ptr = T()) { uniquePtr().reset(ptr); }
 };
 
 namespace gc {
 
 template <typename T, typename TraceCallbacks>
 void
 CallTraceCallbackOnNonHeap(T* v, const TraceCallbacks& aCallbacks, const char* aName, void* aClosure)
 {
--- a/js/src/vm/Scope.cpp
+++ b/js/src/vm/Scope.cpp
@@ -171,19 +171,19 @@ CopyScopeData(JSContext* cx, Handle<type
     uint8_t* extra = reinterpret_cast<uint8_t*>(data.get()) + headerSize;
     uint8_t* extraCopy = copyBytes + headerSize;
 
     mozilla::PodCopy<uint8_t>(extraCopy, extra, extraSize);
     return UniquePtr<typename ConcreteScope::Data>(dataCopy);
 }
 
 template <typename ConcreteScope>
-static UniquePtr<typename ConcreteScope::Data>
-CopyScopeData(JSContext* cx, BindingIter& bi, Handle<typename ConcreteScope::Data*> data,
-              const Class* cls, uint32_t baseShapeFlags, MutableHandleShape envShape)
+static bool
+PrepareScopeData(JSContext* cx, BindingIter& bi, Handle<UniquePtr<typename ConcreteScope::Data>> data,
+                 const Class* cls, uint32_t baseShapeFlags, MutableHandleShape envShape)
 {
     // Copy a fresh BindingIter for use below.
     BindingIter freshBi(bi);
 
     // Iterate through all bindings. This counts the number of environment
     // slots needed and computes the maximum frame slot.
     while (bi)
         bi++;
@@ -191,20 +191,20 @@ CopyScopeData(JSContext* cx, BindingIter
 
     // Make a new environment shape if any environment slots were used.
     if (bi.nextEnvironmentSlot() == JSSLOT_FREE(cls)) {
         envShape.set(nullptr);
     } else {
         envShape.set(CreateEnvironmentShape(cx, freshBi, cls, bi.nextEnvironmentSlot(),
                                             baseShapeFlags));
         if (!envShape)
-            return nullptr;
+            return false;
     }
 
-    return CopyScopeData<ConcreteScope>(cx, data);
+    return true;
 }
 
 template <typename ConcreteScope>
 static UniquePtr<typename ConcreteScope::Data>
 NewEmptyScopeData(JSContext* cx, uint32_t length = 0)
 {
     uint8_t* bytes = cx->zone()->pod_calloc<uint8_t>(ConcreteScope::sizeOfData(length));
     if (!bytes)
@@ -261,52 +261,48 @@ DeleteScopeData(ConcreteScopeData* data)
     // Some scope Data classes have GCManagedDeletePolicy because then contain
     // GCPtrs. Dispose of them in the appropriate way.
     JS::DeletePolicy<ConcreteScopeData>()(data);
 }
 
 template <typename ConcreteScope, XDRMode mode>
 /* static */ bool
 Scope::XDRSizedBindingNames(XDRState<mode>* xdr, Handle<ConcreteScope*> scope,
-                            MutableHandle<typename ConcreteScope::Data*> data, uint32_t* lengthOut)
+                            MutableHandle<typename ConcreteScope::Data*> data)
 {
     MOZ_ASSERT(!data);
-    MOZ_ASSERT(!*lengthOut);
 
     JSContext* cx = xdr->cx();
 
     uint32_t length;
     if (mode == XDR_ENCODE)
         length = scope->data().length;
     if (!xdr->codeUint32(&length))
         return false;
 
     if (mode == XDR_ENCODE) {
         data.set(&scope->data());
     } else {
-        if (length) {
-            data.set(NewEmptyScopeData<ConcreteScope>(cx, length).release());
-            if (!data)
-                return false;
-            data->length = length;
-        }
+        data.set(NewEmptyScopeData<ConcreteScope>(cx, length).release());
+        if (!data)
+            return false;
+        data->length = length;
     }
 
     for (uint32_t i = 0; i < length; i++) {
         if (!XDRBindingName(xdr, &data->names[i])) {
             if (mode == XDR_DECODE) {
                 DeleteScopeData(data.get());
                 data.set(nullptr);
             }
 
             return false;
         }
     }
 
-    *lengthOut = length;
     return true;
 }
 
 /* static */ Scope*
 Scope::create(JSContext* cx, ScopeKind kind, HandleScope enclosing, HandleShape envShape)
 {
     Scope* scope = Allocate<Scope>(cx);
     if (scope)
@@ -517,36 +513,46 @@ LexicalScope::nextFrameSlot(Scope* scope
     }
     MOZ_CRASH("Not an enclosing intra-frame Scope");
 }
 
 /* static */ LexicalScope*
 LexicalScope::create(JSContext* cx, ScopeKind kind, Handle<Data*> data,
                      uint32_t firstFrameSlot, HandleScope enclosing)
 {
+    MOZ_ASSERT(data, "LexicalScopes should not be created if there are no bindings.");
+
+    // The data that's passed in is from the frontend and is LifoAlloc'd.
+    // Copy it now that we're creating a permanent VM scope.
+    Rooted<UniquePtr<Data>> copy(cx, CopyScopeData<LexicalScope>(cx, data));
+    if (!copy)
+        return nullptr;
+
+    return createWithData(cx, kind, &copy, firstFrameSlot, enclosing);
+}
+
+/* static */ LexicalScope*
+LexicalScope::createWithData(JSContext* cx, ScopeKind kind, MutableHandle<UniquePtr<Data>> data,
+                             uint32_t firstFrameSlot, HandleScope enclosing)
+{
     bool isNamedLambda = kind == ScopeKind::NamedLambda || kind == ScopeKind::StrictNamedLambda;
 
-    MOZ_ASSERT(data, "LexicalScopes should not be created if there are no bindings.");
     MOZ_ASSERT_IF(!isNamedLambda && firstFrameSlot != 0,
                   firstFrameSlot == nextFrameSlot(enclosing));
     MOZ_ASSERT_IF(isNamedLambda, firstFrameSlot == LOCALNO_LIMIT);
 
-    // The data that's passed in may be from the frontend and LifoAlloc'd.
-    // Copy it now that we're creating a permanent VM scope.
     RootedShape envShape(cx);
     BindingIter bi(*data, firstFrameSlot, isNamedLambda);
-    Rooted<UniquePtr<Data>> copy(cx,
-        CopyScopeData<LexicalScope>(cx, bi, data,
-                                    &LexicalEnvironmentObject::class_,
-                                    BaseShape::NOT_EXTENSIBLE | BaseShape::DELEGATE,
-                                    &envShape));
-    if (!copy)
+    if (!PrepareScopeData<LexicalScope>(cx, bi, data, &LexicalEnvironmentObject::class_,
+                                        BaseShape::NOT_EXTENSIBLE | BaseShape::DELEGATE, &envShape))
+    {
         return nullptr;
+    }
 
-    Scope* scope = Scope::create(cx, kind, enclosing, envShape, Move(copy.get()));
+    Scope* scope = Scope::create(cx, kind, enclosing, envShape, Move(data.get()));
     if (!scope)
         return nullptr;
     MOZ_ASSERT(scope->as<LexicalScope>().firstFrameSlot() == firstFrameSlot);
     return &scope->as<LexicalScope>();
 }
 
 /* static */ Shape*
 LexicalScope::getEmptyExtensibleEnvironmentShape(JSContext* cx)
@@ -558,45 +564,40 @@ LexicalScope::getEmptyExtensibleEnvironm
 template <XDRMode mode>
 /* static */ bool
 LexicalScope::XDR(XDRState<mode>* xdr, ScopeKind kind, HandleScope enclosing,
                   MutableHandleScope scope)
 {
     JSContext* cx = xdr->cx();
 
     Rooted<Data*> data(cx);
-    uint32_t length = 0;
-    if (!XDRSizedBindingNames<LexicalScope>(xdr, scope.as<LexicalScope>(), &data, &length))
+    if (!XDRSizedBindingNames<LexicalScope>(xdr, scope.as<LexicalScope>(), &data))
         return false;
 
     {
-        auto deleteOnLeave = MakeScopeExit([&data]() {
-            if (mode == XDR_DECODE)
-                DeleteScopeData(data.get());
-        });
+        Maybe<Rooted<UniquePtr<Data>>> uniqueData;
+        if (mode == XDR_DECODE)
+            uniqueData.emplace(cx, data);
 
         uint32_t firstFrameSlot;
         uint32_t nextFrameSlot;
         if (mode == XDR_ENCODE) {
             firstFrameSlot = scope->as<LexicalScope>().firstFrameSlot();
             nextFrameSlot = data->nextFrameSlot;
         }
 
         if (!xdr->codeUint32(&data->constStart))
             return false;
         if (!xdr->codeUint32(&firstFrameSlot))
             return false;
         if (!xdr->codeUint32(&nextFrameSlot))
             return false;
 
         if (mode == XDR_DECODE) {
-            if (!data)
-                return false;
-
-            scope.set(create(cx, kind, data, firstFrameSlot, enclosing));
+            scope.set(createWithData(cx, kind, &uniqueData.ref(), firstFrameSlot, enclosing));
             if (!scope)
                 return false;
 
             // nextFrameSlot is used only for this correctness check.
             MOZ_ASSERT(nextFrameSlot == scope->as<LexicalScope>().data().nextFrameSlot);
         }
     }
 
@@ -616,57 +617,62 @@ LexicalScope::XDR(XDRState<XDR_DECODE>* 
 static inline uint32_t
 FunctionScopeEnvShapeFlags(bool hasParameterExprs)
 {
     if (hasParameterExprs)
         return BaseShape::DELEGATE;
     return BaseShape::QUALIFIED_VAROBJ | BaseShape::DELEGATE;
 }
 
-/* static */ UniquePtr<FunctionScope::Data>
-FunctionScope::copyData(JSContext* cx, Handle<Data*> data,
-                        bool hasParameterExprs, MutableHandleShape envShape)
-{
-    if (data) {
-        BindingIter bi(*data, hasParameterExprs);
-        uint32_t shapeFlags = FunctionScopeEnvShapeFlags(hasParameterExprs);
-        return CopyScopeData<FunctionScope>(cx, bi, data,
-                                            &CallObject::class_,
-                                            shapeFlags, envShape);
-    }
-    return NewEmptyScopeData<FunctionScope>(cx);
-}
-
 Zone*
 FunctionScope::Data::zone() const
 {
     return canonicalFunction ? canonicalFunction->zone() : nullptr;
 }
 
 /* static */ FunctionScope*
-FunctionScope::create(JSContext* cx, Handle<Data*> data,
+FunctionScope::create(JSContext* cx, Handle<Data*> dataArg,
                       bool hasParameterExprs, bool needsEnvironment,
                       HandleFunction fun, HandleScope enclosing)
 {
+    // The data that's passed in is from the frontend and is LifoAlloc'd.
+    // Copy it now that we're creating a permanent VM scope.
+    Rooted<UniquePtr<Data>> data(cx, dataArg ? CopyScopeData<FunctionScope>(cx, dataArg)
+                                             : NewEmptyScopeData<FunctionScope>(cx));
+    if (!data)
+        return nullptr;
+
+    return createWithData(cx, &data, hasParameterExprs, needsEnvironment, fun, enclosing);
+}
+
+/* static */ FunctionScope*
+FunctionScope::createWithData(JSContext* cx, MutableHandle<UniquePtr<Data>> data,
+                              bool hasParameterExprs, bool needsEnvironment,
+                              HandleFunction fun, HandleScope enclosing)
+{
+    MOZ_ASSERT(data);
     MOZ_ASSERT(fun->isTenured());
 
     // FunctionScope::Data has GCManagedDeletePolicy because it contains a
-    // GCPtr. Destruction of |copy| below may trigger calls into the GC.
+    // GCPtr. Destruction of |data| below may trigger calls into the GC.
     Rooted<FunctionScope*> funScope(cx);
 
     {
-        // The data that's passed in may be from the frontend and LifoAlloc'd.
-        // Copy it now that we're creating a permanent VM scope.
         RootedShape envShape(cx);
-        Rooted<UniquePtr<Data>> copy(cx, copyData(cx, data, hasParameterExprs, &envShape));
-        if (!copy)
+
+        BindingIter bi(*data, hasParameterExprs);
+        uint32_t shapeFlags = FunctionScopeEnvShapeFlags(hasParameterExprs);
+        if (!PrepareScopeData<FunctionScope>(cx, bi, data, &CallObject::class_, shapeFlags,
+                                             &envShape))
+        {
             return nullptr;
+        }
 
-        copy->hasParameterExprs = hasParameterExprs;
-        copy->canonicalFunction.init(fun);
+        data->hasParameterExprs = hasParameterExprs;
+        data->canonicalFunction.init(fun);
 
         // An environment may be needed regardless of existence of any closed over
         // bindings:
         //   - Extensible scopes (i.e., due to direct eval)
         //   - Needing a home object
         //   - Being a derived class constructor
         //   - Being a generator
         if (!envShape && needsEnvironment) {
@@ -675,17 +681,17 @@ FunctionScope::create(JSContext* cx, Han
                 return nullptr;
         }
 
         Scope* scope = Scope::create(cx, ScopeKind::Function, enclosing, envShape);
         if (!scope)
             return nullptr;
 
         funScope = &scope->as<FunctionScope>();
-        funScope->initData(Move(copy.get()));
+        funScope->initData(Move(data.get()));
     }
 
     return funScope;
 }
 
 JSScript*
 FunctionScope::script() const
 {
@@ -746,66 +752,57 @@ FunctionScope::clone(JSContext* cx, Hand
 
 template <XDRMode mode>
 /* static */ bool
 FunctionScope::XDR(XDRState<mode>* xdr, HandleFunction fun, HandleScope enclosing,
                    MutableHandleScope scope)
 {
     JSContext* cx = xdr->cx();
     Rooted<Data*> data(cx);
-    uint32_t length = 0;
-    if (!XDRSizedBindingNames<FunctionScope>(xdr, scope.as<FunctionScope>(), &data, &length))
+    if (!XDRSizedBindingNames<FunctionScope>(xdr, scope.as<FunctionScope>(), &data))
         return false;
 
     {
-        auto deleteOnLeave = MakeScopeExit([&data]() {
-            if (mode == XDR_DECODE)
-                DeleteScopeData(data.get());
-        });
+        Maybe<Rooted<UniquePtr<Data>>> uniqueData;
+        if (mode == XDR_DECODE)
+            uniqueData.emplace(cx, data);
 
         uint8_t needsEnvironment;
         uint8_t hasParameterExprs;
-        uint16_t nonPositionalFormalStart;
-        uint16_t varStart;
         uint32_t nextFrameSlot;
         if (mode == XDR_ENCODE) {
             needsEnvironment = scope->hasEnvironment();
             hasParameterExprs = data->hasParameterExprs;
-            nonPositionalFormalStart = data->nonPositionalFormalStart;
-            varStart = data->varStart;
             nextFrameSlot = data->nextFrameSlot;
         }
         if (!xdr->codeUint8(&needsEnvironment))
             return false;
         if (!xdr->codeUint8(&hasParameterExprs))
             return false;
-        if (!xdr->codeUint16(&nonPositionalFormalStart))
+        if (!xdr->codeUint16(&data->nonPositionalFormalStart))
             return false;
-        if (!xdr->codeUint16(&varStart))
+        if (!xdr->codeUint16(&data->varStart))
             return false;
         if (!xdr->codeUint32(&nextFrameSlot))
             return false;
 
         if (mode == XDR_DECODE) {
-            MOZ_ASSERT(!length == !data);
-            if (length) {
-                data->nonPositionalFormalStart = nonPositionalFormalStart;
-                data->varStart = varStart;
-            } else {
-                MOZ_ASSERT(!nonPositionalFormalStart);
-                MOZ_ASSERT(!varStart);
+            if (!data->length) {
+                MOZ_ASSERT(!data->nonPositionalFormalStart);
+                MOZ_ASSERT(!data->varStart);
+                MOZ_ASSERT(!data->nextFrameSlot);
             }
 
-            scope.set(create(cx, data, hasParameterExprs, needsEnvironment, fun, enclosing));
+            scope.set(createWithData(cx, &uniqueData.ref(), hasParameterExprs, needsEnvironment, fun,
+                                     enclosing));
             if (!scope)
                 return false;
 
             // nextFrameSlot is used only for this correctness check.
             MOZ_ASSERT(nextFrameSlot == scope->as<FunctionScope>().data().nextFrameSlot);
-            MOZ_ASSERT_IF(!data, !nextFrameSlot);
         }
     }
 
     return true;
 }
 
 template
 /* static */ bool
@@ -815,55 +812,65 @@ FunctionScope::XDR(XDRState<XDR_ENCODE>*
 template
 /* static */ bool
 FunctionScope::XDR(XDRState<XDR_DECODE>* xdr, HandleFunction fun, HandleScope enclosing,
                    MutableHandleScope scope);
 
 static const uint32_t VarScopeEnvShapeFlags =
     BaseShape::QUALIFIED_VAROBJ | BaseShape::DELEGATE;
 
-/* static */ UniquePtr<VarScope::Data>
-VarScope::copyData(JSContext* cx, Handle<Data*> data, uint32_t firstFrameSlot,
-                   MutableHandleShape envShape)
+static UniquePtr<VarScope::Data>
+NewEmptyVarScopeData(JSContext* cx, uint32_t firstFrameSlot)
 {
-    if (data) {
-        BindingIter bi(*data, firstFrameSlot);
-        return CopyScopeData<VarScope>(cx, bi, data,
-                                       &VarEnvironmentObject::class_,
-                                       VarScopeEnvShapeFlags, envShape);
-    }
+    UniquePtr<VarScope::Data> data(NewEmptyScopeData<VarScope>(cx));
+    if (data)
+        data->nextFrameSlot = firstFrameSlot;
 
-    UniquePtr<Data> empty = NewEmptyScopeData<VarScope>(cx);
-    if (empty)
-        empty->nextFrameSlot = firstFrameSlot;
-    return empty;
+    return data;
 }
 
 /* static */ VarScope*
-VarScope::create(JSContext* cx, ScopeKind kind, Handle<Data*> data,
+VarScope::create(JSContext* cx, ScopeKind kind, Handle<Data*> dataArg,
                  uint32_t firstFrameSlot, bool needsEnvironment, HandleScope enclosing)
 {
-    // The data that's passed in may be from the frontend and LifoAlloc'd.
+    // The data that's passed in is from the frontend and is LifoAlloc'd.
     // Copy it now that we're creating a permanent VM scope.
+    Rooted<UniquePtr<Data>> data(cx, dataArg ? CopyScopeData<VarScope>(cx, dataArg)
+                                             : NewEmptyVarScopeData(cx, firstFrameSlot));
+    if (!data)
+        return nullptr;
+
+    return createWithData(cx, kind, &data, firstFrameSlot, needsEnvironment, enclosing);
+}
+
+/* static */ VarScope*
+VarScope::createWithData(JSContext* cx, ScopeKind kind, MutableHandle<UniquePtr<Data>> data,
+                         uint32_t firstFrameSlot, bool needsEnvironment, HandleScope enclosing)
+{
+    MOZ_ASSERT(data);
+
     RootedShape envShape(cx);
-    Rooted<UniquePtr<Data>> copy(cx, copyData(cx, data, firstFrameSlot, &envShape));
-    if (!copy)
+    BindingIter bi(*data, firstFrameSlot);
+    if (!PrepareScopeData<VarScope>(cx, bi, data, &VarEnvironmentObject::class_, VarScopeEnvShapeFlags,
+                                    &envShape))
+    {
         return nullptr;
+    }
 
     // An environment may be needed regardless of existence of any closed over
     // bindings:
     //   - Extensible scopes (i.e., due to direct eval)
     //   - Being a generator
     if (!envShape && needsEnvironment) {
         envShape = getEmptyEnvironmentShape(cx);
         if (!envShape)
             return nullptr;
     }
 
-    Scope* scope = Scope::create(cx, kind, enclosing, envShape, Move(copy.get()));
+    Scope* scope = Scope::create(cx, kind, enclosing, envShape, Move(data.get()));
     if (!scope)
         return nullptr;
     return &scope->as<VarScope>();
 }
 
 /* static */ Shape*
 VarScope::getEmptyEnvironmentShape(JSContext* cx)
 {
@@ -881,25 +888,23 @@ VarScope::firstFrameSlot() const
 
 template <XDRMode mode>
 /* static */ bool
 VarScope::XDR(XDRState<mode>* xdr, ScopeKind kind, HandleScope enclosing,
               MutableHandleScope scope)
 {
     JSContext* cx = xdr->cx();
     Rooted<Data*> data(cx);
-    uint32_t length = 0;
-    if (!XDRSizedBindingNames<VarScope>(xdr, scope.as<VarScope>(), &data, &length))
+    if (!XDRSizedBindingNames<VarScope>(xdr, scope.as<VarScope>(), &data))
         return false;
 
     {
-        auto deleteOnLeave = MakeScopeExit([&data]() {
-            if (mode == XDR_DECODE)
-                DeleteScopeData(data.get());
-        });
+        Maybe<Rooted<UniquePtr<Data>>> uniqueData;
+        if (mode == XDR_DECODE)
+            uniqueData.emplace(cx, data);
 
         uint8_t needsEnvironment;
         uint32_t firstFrameSlot;
         uint32_t nextFrameSlot;
         if (mode == XDR_ENCODE) {
             needsEnvironment = scope->hasEnvironment();
             firstFrameSlot = scope->as<VarScope>().firstFrameSlot();
             nextFrameSlot = data->nextFrameSlot;
@@ -907,18 +912,22 @@ VarScope::XDR(XDRState<mode>* xdr, Scope
         if (!xdr->codeUint8(&needsEnvironment))
             return false;
         if (!xdr->codeUint32(&firstFrameSlot))
             return false;
         if (!xdr->codeUint32(&nextFrameSlot))
             return false;
 
         if (mode == XDR_DECODE) {
-            MOZ_ASSERT(!length == !data);
-            scope.set(create(cx, kind, data, firstFrameSlot, needsEnvironment, enclosing));
+            if (!data->length) {
+                MOZ_ASSERT(!data->nextFrameSlot);
+            }
+
+            scope.set(createWithData(cx, kind, &uniqueData.ref(), firstFrameSlot, needsEnvironment,
+                                     enclosing));
             if (!scope)
                 return false;
 
             // nextFrameSlot is used only for this correctness check.
             MOZ_ASSERT(nextFrameSlot == scope->as<VarScope>().data().nextFrameSlot);
         }
     }
 
@@ -930,39 +939,39 @@ template
 VarScope::XDR(XDRState<XDR_ENCODE>* xdr, ScopeKind kind, HandleScope enclosing,
               MutableHandleScope scope);
 
 template
 /* static */ bool
 VarScope::XDR(XDRState<XDR_DECODE>* xdr, ScopeKind kind, HandleScope enclosing,
               MutableHandleScope scope);
 
-/* static */ UniquePtr<GlobalScope::Data>
-GlobalScope::copyData(JSContext *cx, Handle<Data*> data)
+/* static */ GlobalScope*
+GlobalScope::create(JSContext* cx, ScopeKind kind, Handle<Data*> dataArg)
 {
-    if (data) {
-        // The global scope has no environment shape. Its environment is the
-        // global lexical scope and the global object or non-syntactic objects
-        // created by embedding, all of which are not only extensible but may
-        // have names on them deleted.
-        return CopyScopeData<GlobalScope>(cx, data);
-    }
-    return NewEmptyScopeData<GlobalScope>(cx);
+    // The data that's passed in is from the frontend and is LifoAlloc'd.
+    // Copy it now that we're creating a permanent VM scope.
+    Rooted<UniquePtr<Data>> data(cx, dataArg ? CopyScopeData<GlobalScope>(cx, dataArg)
+                                             : NewEmptyScopeData<GlobalScope>(cx));
+    if (!data)
+        return nullptr;
+
+    return createWithData(cx, kind, &data);
 }
 
 /* static */ GlobalScope*
-GlobalScope::create(JSContext* cx, ScopeKind kind, Handle<Data*> data)
+GlobalScope::createWithData(JSContext* cx, ScopeKind kind, MutableHandle<UniquePtr<Data>> data)
 {
-    // The data that's passed in may be from the frontend and LifoAlloc'd.
-    // Copy it now that we're creating a permanent VM scope.
-    Rooted<UniquePtr<Data>> copy(cx, copyData(cx, data));
-    if (!copy)
-        return nullptr;
+    MOZ_ASSERT(data);
 
-    Scope* scope = Scope::create(cx, kind, nullptr, nullptr, Move(copy.get()));
+    // The global scope has no environment shape. Its environment is the
+    // global lexical scope and the global object or non-syntactic objects
+    // created by embedding, all of which are not only extensible but may
+    // have names on them deleted.
+    Scope* scope = Scope::create(cx, kind, nullptr, nullptr, Move(data.get()));
     if (!scope)
         return nullptr;
     return &scope->as<GlobalScope>();
 }
 
 /* static */ GlobalScope*
 GlobalScope::clone(JSContext* cx, Handle<GlobalScope*> scope, ScopeKind kind)
 {
@@ -980,54 +989,39 @@ GlobalScope::clone(JSContext* cx, Handle
 template <XDRMode mode>
 /* static */ bool
 GlobalScope::XDR(XDRState<mode>* xdr, ScopeKind kind, MutableHandleScope scope)
 {
     MOZ_ASSERT((mode == XDR_DECODE) == !scope);
 
     JSContext* cx = xdr->cx();
     Rooted<Data*> data(cx);
-    uint32_t length = 0;
-    if (!XDRSizedBindingNames<GlobalScope>(xdr, scope.as<GlobalScope>(), &data, &length))
+    if (!XDRSizedBindingNames<GlobalScope>(xdr, scope.as<GlobalScope>(), &data))
         return false;
 
     {
-        auto deleteOnLeave = MakeScopeExit([&data]() {
-            if (mode == XDR_DECODE)
-                DeleteScopeData(data.get());
-        });
+        Maybe<Rooted<UniquePtr<Data>>> uniqueData;
+        if (mode == XDR_DECODE)
+            uniqueData.emplace(cx, data);
 
-        uint32_t varStart;
-        uint32_t letStart;
-        uint32_t constStart;
-        if (mode == XDR_ENCODE) {
-            varStart = data->varStart;
-            letStart = data->letStart;
-            constStart = data->constStart;
-        }
-        if (!xdr->codeUint32(&varStart))
+        if (!xdr->codeUint32(&data->varStart))
             return false;
-        if (!xdr->codeUint32(&letStart))
+        if (!xdr->codeUint32(&data->letStart))
             return false;
-        if (!xdr->codeUint32(&constStart))
+        if (!xdr->codeUint32(&data->constStart))
             return false;
 
         if (mode == XDR_DECODE) {
-            MOZ_ASSERT(!length == !data);
-            if (length) {
-                data->varStart = varStart;
-                data->letStart = letStart;
-                data->constStart = constStart;
-            } else {
-                MOZ_ASSERT(!varStart);
-                MOZ_ASSERT(!letStart);
-                MOZ_ASSERT(!constStart);
+            if (!data->length) {
+                MOZ_ASSERT(!data->varStart);
+                MOZ_ASSERT(!data->letStart);
+                MOZ_ASSERT(!data->constStart);
             }
 
-            scope.set(create(cx, kind, data));
+            scope.set(createWithData(cx, kind, &uniqueData.ref()));
             if (!scope)
                 return false;
         }
     }
 
     return true;
 }
 
@@ -1044,52 +1038,55 @@ WithScope::create(JSContext* cx, HandleS
 {
     Scope* scope = Scope::create(cx, ScopeKind::With, enclosing, nullptr);
     return static_cast<WithScope*>(scope);
 }
 
 static const uint32_t EvalScopeEnvShapeFlags =
     BaseShape::QUALIFIED_VAROBJ | BaseShape::DELEGATE;
 
-/* static */ UniquePtr<EvalScope::Data>
-EvalScope::copyData(JSContext* cx, ScopeKind scopeKind, Handle<Data*> data,
-                    MutableHandleShape envShape)
+/* static */ EvalScope*
+EvalScope::create(JSContext* cx, ScopeKind scopeKind, Handle<Data*> dataArg,
+                  HandleScope enclosing)
 {
-    if (data) {
-        if (scopeKind == ScopeKind::StrictEval) {
-            BindingIter bi(*data, true);
-            return CopyScopeData<EvalScope>(cx, bi, data,
-                                            &VarEnvironmentObject::class_,
-                                            EvalScopeEnvShapeFlags, envShape);
-        }
-        return CopyScopeData<EvalScope>(cx, data);
-    }
-    return NewEmptyScopeData<EvalScope>(cx);
+    // The data that's passed in is from the frontend and is LifoAlloc'd.
+    // Copy it now that we're creating a permanent VM scope.
+    Rooted<UniquePtr<Data>> data(cx, dataArg ? CopyScopeData<EvalScope>(cx, dataArg)
+                                             : NewEmptyScopeData<EvalScope>(cx));
+    if (!data)
+        return nullptr;
+
+    return createWithData(cx, scopeKind, &data, enclosing);
 }
 
 /* static */ EvalScope*
-EvalScope::create(JSContext* cx, ScopeKind scopeKind, Handle<Data*> data,
-                  HandleScope enclosing)
+EvalScope::createWithData(JSContext* cx, ScopeKind scopeKind, MutableHandle<UniquePtr<Data>> data,
+                          HandleScope enclosing)
 {
-    // The data that's passed in may be from the frontend and LifoAlloc'd.
-    // Copy it now that we're creating a permanent VM scope.
+    MOZ_ASSERT(data);
+
     RootedShape envShape(cx);
-    Rooted<UniquePtr<Data>> copy(cx, copyData(cx, scopeKind, data, &envShape));
-    if (!copy)
-        return nullptr;
+    if (scopeKind == ScopeKind::StrictEval) {
+        BindingIter bi(*data, true);
+        if (!PrepareScopeData<EvalScope>(cx, bi, data, &VarEnvironmentObject::class_,
+                                         EvalScopeEnvShapeFlags, &envShape))
+        {
+            return nullptr;
+        }
+    }
 
     // Strict eval and direct eval in parameter expressions always get their own
     // var environment even if there are no bindings.
     if (!envShape && scopeKind == ScopeKind::StrictEval) {
         envShape = getEmptyEnvironmentShape(cx);
         if (!envShape)
             return nullptr;
     }
 
-    Scope* scope = Scope::create(cx, scopeKind, enclosing, envShape, Move(copy.get()));
+    Scope* scope = Scope::create(cx, scopeKind, enclosing, envShape, Move(data.get()));
     if (!scope)
         return nullptr;
     return &scope->as<EvalScope>();
 }
 
 /* static */ Scope*
 EvalScope::nearestVarScopeForDirectEval(Scope* scope)
 {
@@ -1119,28 +1116,28 @@ template <XDRMode mode>
 /* static */ bool
 EvalScope::XDR(XDRState<mode>* xdr, ScopeKind kind, HandleScope enclosing,
                MutableHandleScope scope)
 {
     JSContext* cx = xdr->cx();
     Rooted<Data*> data(cx);
 
     {
-        auto deleteOnLeave = MakeScopeExit([&data]() {
-            if (mode == XDR_DECODE)
-                DeleteScopeData(data.get());
-        });
+        Maybe<Rooted<UniquePtr<Data>>> uniqueData;
+        if (mode == XDR_DECODE)
+            uniqueData.emplace(cx, data);
 
-        uint32_t length = 0;
-        if (!XDRSizedBindingNames<EvalScope>(xdr, scope.as<EvalScope>(), &data, &length))
+        if (!XDRSizedBindingNames<EvalScope>(xdr, scope.as<EvalScope>(), &data))
             return false;
 
         if (mode == XDR_DECODE) {
-            MOZ_ASSERT(!length == !data);
-            scope.set(create(cx, kind, data, enclosing));
+            if (!data->length)
+                MOZ_ASSERT(!data->nextFrameSlot);
+
+            scope.set(createWithData(cx, kind, &uniqueData.ref(), enclosing));
             if (!scope)
                 return false;
         }
     }
 
     return true;
 }
 
@@ -1152,67 +1149,71 @@ EvalScope::XDR(XDRState<XDR_ENCODE>* xdr
 template
 /* static */ bool
 EvalScope::XDR(XDRState<XDR_DECODE>* xdr, ScopeKind kind, HandleScope enclosing,
                MutableHandleScope scope);
 
 static const uint32_t ModuleScopeEnvShapeFlags =
     BaseShape::NOT_EXTENSIBLE | BaseShape::QUALIFIED_VAROBJ | BaseShape::DELEGATE;
 
-/* static */ UniquePtr<ModuleScope::Data>
-ModuleScope::copyData(JSContext* cx, Handle<Data*> data, MutableHandleShape envShape)
-{
-    if (data) {
-        BindingIter bi(*data);
-        return CopyScopeData<ModuleScope>(cx, bi, data,
-                                          &ModuleEnvironmentObject::class_,
-                                          ModuleScopeEnvShapeFlags, envShape);
-    }
-    return NewEmptyScopeData<ModuleScope>(cx);
-}
-
 Zone*
 ModuleScope::Data::zone() const
 {
     return module ? module->zone() : nullptr;
 }
 
 /* static */ ModuleScope*
-ModuleScope::create(JSContext* cx, Handle<Data*> data,
+ModuleScope::create(JSContext* cx, Handle<Data*> dataArg,
                     HandleModuleObject module, HandleScope enclosing)
 {
+    Rooted<UniquePtr<Data>> data(cx, dataArg ? CopyScopeData<ModuleScope>(cx, dataArg)
+                                             : NewEmptyScopeData<ModuleScope>(cx));
+    if (!data)
+        return nullptr;
+
+    return createWithData(cx, &data, module, enclosing);
+}
+
+/* static */ ModuleScope*
+ModuleScope::createWithData(JSContext* cx, MutableHandle<UniquePtr<Data>> data,
+                            HandleModuleObject module, HandleScope enclosing)
+{
+    MOZ_ASSERT(data);
     MOZ_ASSERT(enclosing->is<GlobalScope>());
 
     // ModuleScope::Data has GCManagedDeletePolicy because it contains a
     // GCPtr. Destruction of |copy| below may trigger calls into the GC.
     Rooted<ModuleScope*> moduleScope(cx);
 
     {
-        // The data that's passed in may be from the frontend and LifoAlloc'd.
+        // The data that's passed in is from the frontend and is LifoAlloc'd.
         // Copy it now that we're creating a permanent VM scope.
         RootedShape envShape(cx);
-        Rooted<UniquePtr<Data>> copy(cx, copyData(cx, data, &envShape));
-        if (!copy)
+        BindingIter bi(*data);
+        if (!PrepareScopeData<ModuleScope>(cx, bi, data, &ModuleEnvironmentObject::class_,
+                                           ModuleScopeEnvShapeFlags, &envShape))
+        {
             return nullptr;
+        }
 
         // Modules always need an environment object for now.
         if (!envShape) {
             envShape = getEmptyEnvironmentShape(cx);
             if (!envShape)
                 return nullptr;
         }
 
         Scope* scope = Scope::create(cx, ScopeKind::Module, enclosing, envShape);
         if (!scope)
             return nullptr;
 
-        copy->module.init(module);
+        data->module.init(module);
 
         moduleScope = &scope->as<ModuleScope>();
-        moduleScope->initData(Move(copy.get()));
+        moduleScope->initData(Move(data.get()));
     }
 
     return moduleScope;
 }
 
 /* static */ Shape*
 ModuleScope::getEmptyEnvironmentShape(JSContext* cx)
 {
--- a/js/src/vm/Scope.h
+++ b/js/src/vm/Scope.h
@@ -243,18 +243,17 @@ class Scope : public js::gc::TenuredCell
                          HandleShape envShape);
 
     template <typename T, typename D>
     static Scope* create(JSContext* cx, ScopeKind kind, HandleScope enclosing,
                          HandleShape envShape, mozilla::UniquePtr<T, D> data);
 
     template <typename ConcreteScope, XDRMode mode>
     static bool XDRSizedBindingNames(XDRState<mode>* xdr, Handle<ConcreteScope*> scope,
-                                     MutableHandle<typename ConcreteScope::Data*> data,
-                                     uint32_t* lengthOut);
+                                     MutableHandle<typename ConcreteScope::Data*> data);
 
     Shape* maybeCloneEnvironmentShape(JSContext* cx);
 
     template <typename T, typename D>
     void initData(mozilla::UniquePtr<T, D> data) {
         MOZ_ASSERT(!data_);
         data_ = reinterpret_cast<uintptr_t>(data.release());
     }
@@ -393,17 +392,21 @@ class LexicalScope : public Scope
 
     static LexicalScope* create(JSContext* cx, ScopeKind kind, Handle<Data*> data,
                                 uint32_t firstFrameSlot, HandleScope enclosing);
 
     template <XDRMode mode>
     static bool XDR(XDRState<mode>* xdr, ScopeKind kind, HandleScope enclosing,
                     MutableHandleScope scope);
 
-  protected:
+  private:
+    static LexicalScope* createWithData(JSContext* cx, ScopeKind kind,
+                                        MutableHandle<UniquePtr<Data>> data,
+                                        uint32_t firstFrameSlot, HandleScope enclosing);
+
     Data& data() {
         return *reinterpret_cast<Data*>(data_);
     }
 
     const Data& data() const {
         return *reinterpret_cast<Data*>(data_);
     }
 
@@ -517,18 +520,19 @@ class FunctionScope : public Scope
     static FunctionScope* clone(JSContext* cx, Handle<FunctionScope*> scope, HandleFunction fun,
                                 HandleScope enclosing);
 
     template <XDRMode mode>
     static bool XDR(XDRState<mode>* xdr, HandleFunction fun, HandleScope enclosing,
                     MutableHandleScope scope);
 
   private:
-    static UniquePtr<Data> copyData(JSContext* cx, Handle<Data*> data,
-                                    bool hasParameterExprs, MutableHandleShape envShape);
+    static FunctionScope* createWithData(JSContext* cx, MutableHandle<UniquePtr<Data>> data,
+                                         bool hasParameterExprs, bool needsEnvironment,
+                                         HandleFunction fun, HandleScope enclosing);
 
     Data& data() {
         return *reinterpret_cast<Data*>(data_);
     }
 
     const Data& data() const {
         return *reinterpret_cast<Data*>(data_);
     }
@@ -614,18 +618,19 @@ class VarScope : public Scope
                             uint32_t firstFrameSlot, bool needsEnvironment,
                             HandleScope enclosing);
 
     template <XDRMode mode>
     static bool XDR(XDRState<mode>* xdr, ScopeKind kind, HandleScope enclosing,
                     MutableHandleScope scope);
 
   private:
-    static UniquePtr<Data> copyData(JSContext* cx, Handle<Data*> data,
-                                    uint32_t firstFrameSlot, MutableHandleShape envShape);
+    static VarScope* createWithData(JSContext* cx, ScopeKind kind, MutableHandle<UniquePtr<Data>> data,
+                                    uint32_t firstFrameSlot, bool needsEnvironment,
+                                    HandleScope enclosing);
 
     Data& data() {
         return *reinterpret_cast<Data*>(data_);
     }
 
     const Data& data() const {
         return *reinterpret_cast<Data*>(data_);
     }
@@ -710,17 +715,18 @@ class GlobalScope : public Scope
     }
 
     static GlobalScope* clone(JSContext* cx, Handle<GlobalScope*> scope, ScopeKind kind);
 
     template <XDRMode mode>
     static bool XDR(XDRState<mode>* xdr, ScopeKind kind, MutableHandleScope scope);
 
   private:
-    static UniquePtr<Data> copyData(JSContext* cx, Handle<Data*> data);
+    static GlobalScope* createWithData(JSContext* cx, ScopeKind kind,
+                                       MutableHandle<UniquePtr<Data>> data);
 
     Data& data() {
         return *reinterpret_cast<Data*>(data_);
     }
 
     const Data& data() const {
         return *reinterpret_cast<Data*>(data_);
     }
@@ -810,18 +816,18 @@ class EvalScope : public Scope
     static EvalScope* create(JSContext* cx, ScopeKind kind, Handle<Data*> data,
                              HandleScope enclosing);
 
     template <XDRMode mode>
     static bool XDR(XDRState<mode>* xdr, ScopeKind kind, HandleScope enclosing,
                     MutableHandleScope scope);
 
   private:
-    static UniquePtr<Data> copyData(JSContext* cx, ScopeKind scopeKind,
-                                    Handle<Data*> data, MutableHandleShape envShape);
+    static EvalScope* createWithData(JSContext* cx, ScopeKind kind, MutableHandle<UniquePtr<Data>> data,
+                                     HandleScope enclosing);
 
     Data& data() {
         return *reinterpret_cast<Data*>(data_);
     }
 
     const Data& data() const {
         return *reinterpret_cast<Data*>(data_);
     }
@@ -913,18 +919,18 @@ class ModuleScope : public Scope
         *names = data->names;
         *length = data->length;
     }
 
     static ModuleScope* create(JSContext* cx, Handle<Data*> data,
                                Handle<ModuleObject*> module, HandleScope enclosing);
 
   private:
-    static UniquePtr<Data> copyData(JSContext* cx, Handle<Data*> data,
-                                    MutableHandleShape envShape);
+    static ModuleScope* createWithData(JSContext* cx, MutableHandle<UniquePtr<Data>> data,
+                                       Handle<ModuleObject*> module, HandleScope enclosing);
 
     Data& data() {
         return *reinterpret_cast<Data*>(data_);
     }
 
     const Data& data() const {
         return *reinterpret_cast<Data*>(data_);
     }