Bug 1298568 - Ensure Scopes that can have data always have non-null data on clone. (r=Waldo)

js/public/GCPolicyAPI.h

@@ -144,10 +144,13 @@ struct GCPolicy<mozilla::UniquePtr<T, D>>
 {
     static mozilla::UniquePtr<T,D> initial() { return mozilla::UniquePtr<T,D>(); }
     static void trace(JSTracer* trc, mozilla::UniquePtr<T,D>* tp, const char* name) {
-        GCPolicy<T>::trace(trc, tp->get(), name);
+        if (tp->get())
+            GCPolicy<T>::trace(trc, tp->get(), name);
     }
     static bool needsSweep(mozilla::UniquePtr<T,D>* tp) {
-        return GCPolicy<T>::needsSweep(tp->get());
+        if (tp->get())
+            return GCPolicy<T>::needsSweep(tp->get());
+        return false;
     }
 };
 

js/src/vm/Scope.cpp

@@ -274,6 +274,23 @@ Scope::create(ExclusiveContext* cx, ScopeKind kind, HandleScope enclosing, Handl
     return scope;
 }
 
+template <typename T, typename D>
+/* static */ Scope*
+Scope::create(ExclusiveContext* cx, ScopeKind kind, HandleScope enclosing,
+              HandleShape envShape, mozilla::UniquePtr<T, D> data)
+{
+    Scope* scope = create(cx, kind, enclosing, envShape);
+    if (!scope)
+        return nullptr;
+
+    // It is an invariant that all Scopes that have data (currently, all
+    // ScopeKinds except With) must have non-null data.
+    MOZ_ASSERT(data);
+    scope->initData(Move(data));
+
+    return scope;
+}
+
 uint32_t
 Scope::chainLength() const
 {
@@ -318,10 +335,6 @@ Scope::clone(JSContext* cx, HandleScope scope, HandleScope enclosing)
             return nullptr;
     }
 
-    Scope* scopeClone = create(cx, scope->kind_, enclosing, envShape);
-    if (!scopeClone)
-        return nullptr;
-
     switch (scope->kind_) {
       case ScopeKind::Function:
         MOZ_CRASH("Use FunctionScope::clone.");
@@ -333,8 +346,7 @@ Scope::clone(JSContext* cx, HandleScope scope, HandleScope enclosing)
         UniquePtr<VarScope::Data> dataClone = CopyScopeData<VarScope>(cx, original);
         if (!dataClone)
             return nullptr;
-        scopeClone->initData(Move(dataClone));
-        break;
+        return create(cx, scope->kind_, enclosing, envShape, Move(dataClone));
       }
 
       case ScopeKind::Lexical:
@@ -345,12 +357,11 @@ Scope::clone(JSContext* cx, HandleScope scope, HandleScope enclosing)
         UniquePtr<LexicalScope::Data> dataClone = CopyScopeData<LexicalScope>(cx, original);
         if (!dataClone)
             return nullptr;
-        scopeClone->initData(Move(dataClone));
-        break;
+        return create(cx, scope->kind_, enclosing, envShape, Move(dataClone));
       }
 
       case ScopeKind::With:
-        break;
+        return create(cx, scope->kind_, enclosing, envShape);
 
       case ScopeKind::Eval:
       case ScopeKind::StrictEval: {
@@ -358,8 +369,7 @@ Scope::clone(JSContext* cx, HandleScope scope, HandleScope enclosing)
         UniquePtr<EvalScope::Data> dataClone = CopyScopeData<EvalScope>(cx, original);
         if (!dataClone)
             return nullptr;
-        scopeClone->initData(Move(dataClone));
-        break;
+        return create(cx, scope->kind_, enclosing, envShape, Move(dataClone));
       }
 
       case ScopeKind::Global:
@@ -372,7 +382,7 @@ Scope::clone(JSContext* cx, HandleScope scope, HandleScope enclosing)
         break;
     }
 
-    return scopeClone;
+    return nullptr;
 }
 
 void
@@ -477,15 +487,11 @@ LexicalScope::create(ExclusiveContext* cx, ScopeKind kind, Handle<Data*> data,
     if (!copy)
         return nullptr;
 
-    Scope* scope = Scope::create(cx, kind, enclosing, envShape);
+    Scope* scope = Scope::create(cx, kind, enclosing, envShape, Move(copy.get()));
     if (!scope)
         return nullptr;
-
     MOZ_ASSERT(scope->as<LexicalScope>().firstFrameSlot() == firstFrameSlot);
-
-    LexicalScope* lexicalScope = &scope->as<LexicalScope>();
-    lexicalScope->initData(Move(copy.get()));
-    return lexicalScope;
+    return &scope->as<LexicalScope>();
 }
 
 /* static */ Shape*
@@ -774,13 +780,10 @@ VarScope::create(ExclusiveContext* cx, ScopeKind kind, Handle<Data*> data,
             return nullptr;
     }
 
-    Scope* scope = Scope::create(cx, kind, enclosing, envShape);
+    Scope* scope = Scope::create(cx, kind, enclosing, envShape, Move(copy.get()));
     if (!scope)
         return nullptr;
-
-    VarScope* varScope = &scope->as<VarScope>();
-    varScope->initData(Move(copy.get()));
-    return varScope;
+    return &scope->as<VarScope>();
 }
 
 /* static */ Shape*
@@ -880,13 +883,10 @@ GlobalScope::create(ExclusiveContext* cx, ScopeKind kind, Handle<Data*> data)
     if (!copy)
         return nullptr;
 
-    Scope* scope = Scope::create(cx, kind, nullptr, nullptr);
+    Scope* scope = Scope::create(cx, kind, nullptr, nullptr, Move(copy.get()));
     if (!scope)
         return nullptr;
-
-    GlobalScope* globalScope = &scope->as<GlobalScope>();
-    globalScope->initData(Move(copy.get()));
-    return globalScope;
+    return &scope->as<GlobalScope>();
 }
 
 /* static */ GlobalScope*
@@ -897,13 +897,10 @@ GlobalScope::clone(JSContext* cx, Handle<GlobalScope*> scope, ScopeKind kind)
     if (!dataClone)
         return nullptr;
 
-    Scope* scopeClone = Scope::create(cx, kind, nullptr, nullptr);
+    Scope* scopeClone = Scope::create(cx, kind, nullptr, nullptr, Move(dataClone.get()));
     if (!scopeClone)
         return nullptr;
-
-    GlobalScope* globalScopeClone = &scopeClone->as<GlobalScope>();
-    globalScopeClone->initData(Move(dataClone.get()));
-    return globalScopeClone;
+    return &scopeClone->as<GlobalScope>();
 }
 
 template <XDRMode mode>
@@ -998,13 +995,10 @@ EvalScope::create(ExclusiveContext* cx, ScopeKind scopeKind, Handle<Data*> data,
             return nullptr;
     }
 
-    Scope* scope = Scope::create(cx, scopeKind, enclosing, envShape);
+    Scope* scope = Scope::create(cx, scopeKind, enclosing, envShape, Move(copy.get()));
     if (!scope)
         return nullptr;
-
-    EvalScope* evalScope = &scope->as<EvalScope>();
-    evalScope->initData(Move(copy.get()));
-    return evalScope;
+    return &scope->as<EvalScope>();
 }
 
 /* static */ Scope*

js/src/vm/Scope.h

@@ -205,6 +205,10 @@ class Scope : public js::gc::TenuredCell
     static Scope* create(ExclusiveContext* cx, ScopeKind kind, HandleScope enclosing,
                          HandleShape envShape);
 
+    template <typename T, typename D>
+    static Scope* create(ExclusiveContext* 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);