From 14afe613f8d1fd4edabbdfa74f0ad221a2c9049f Mon Sep 17 00:00:00 2001
From: Jon Coppeard <jcoppeard@mozilla.com>
Date: Thu, 23 Apr 2015 11:17:15 +0100
Subject: [PATCH] Bug 1156295 - Refactor GC rooting in
 StructType::DefineInternal() r=terrence

---
 js/src/ctypes/CTypes.cpp | 64 +++++++++++++++++++++++++---------------
 js/src/ctypes/CTypes.h   |  3 ++
 2 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/js/src/ctypes/CTypes.cpp b/js/src/ctypes/CTypes.cpp
index b2211f63ce3..cd2d667a2f2 100644
--- a/js/src/ctypes/CTypes.cpp
+++ b/js/src/ctypes/CTypes.cpp
@@ -4028,6 +4028,18 @@ CType::Finalize(JSFreeOp* fop, JSObject* obj)
   }
 }
 
+void
+TraceFieldInfoHash(JSTracer* trc, FieldInfoHash* fields)
+{
+  for (FieldInfoHash::Enum e(*fields); !e.empty(); e.popFront()) {
+    JSString* key = e.front().key();
+    JS_CallUnbarrieredStringTracer(trc, &key, "fieldName");
+    if (key != e.front().key())
+      e.rekeyFront(JS_ASSERT_STRING_IS_FLAT(key));
+    JS_CallObjectTracer(trc, &e.front().value().mType, "fieldType");
+  }
+}
+
 void
 CType::Trace(JSTracer* trc, JSObject* obj)
 {
@@ -4044,14 +4056,7 @@ CType::Trace(JSTracer* trc, JSObject* obj)
       return;
 
     FieldInfoHash* fields = static_cast<FieldInfoHash*>(slot.toPrivate());
-    for (FieldInfoHash::Enum e(*fields); !e.empty(); e.popFront()) {
-      JSString* key = e.front().key();
-      JS_CallUnbarrieredStringTracer(trc, &key, "fieldName");
-      if (key != e.front().key())
-          e.rekeyFront(JS_ASSERT_STRING_IS_FLAT(key));
-      JS_CallObjectTracer(trc, &e.front().value().mType, "fieldType");
-    }
-
+    TraceFieldInfoHash(trc, fields);
     break;
   }
   case TYPE_function: {
@@ -5457,6 +5462,31 @@ PostBarrierCallback(JSTracer* trc, JSString* key, void* data)
     table->rekeyIfMoved(JS_ASSERT_STRING_IS_FLAT(prior), JS_ASSERT_STRING_IS_FLAT(key));
 }
 
+// Holds a pointer to a FieldInfoHash while it is being constructed, tracing it
+// on GC and destroying it when it dies unless release() has been called first.
+class FieldInfoHolder : public JS::CustomAutoRooter
+{
+  public:
+    FieldInfoHolder(JSContext* cx, FieldInfoHash* fields)
+      : CustomAutoRooter(cx), fields_(fields) {}
+    ~FieldInfoHolder() {
+      delete fields_;
+    }
+    virtual void trace(JSTracer* trc) override {
+      if (fields_)
+        TraceFieldInfoHash(trc, fields_);
+    }
+    FieldInfoHash* operator->() { return fields_; }
+    operator FieldInfoHash*() { return fields_; }
+    FieldInfoHash* release() {
+      FieldInfoHash* result = fields_;
+      fields_ = nullptr;
+      return result;
+    }
+  private:
+    FieldInfoHash* fields_;
+};
+
 bool
 StructType::DefineInternal(JSContext* cx, JSObject* typeObj_, JSObject* fieldsObj_)
 {
@@ -5487,16 +5517,11 @@ StructType::DefineInternal(JSContext* cx, JSObject* typeObj_, JSObject* fieldsOb
   // its constituents. (We cannot simply stash the hash in a reserved slot now
   // to get GC safety for free, since if anything in this function fails we
   // do not want to mutate 'typeObj'.)
-  auto fields = cx->make_unique<FieldInfoHash>();
+  FieldInfoHolder fields(cx, cx->new_<FieldInfoHash>());
   if (!fields || !fields->init(len)) {
     JS_ReportOutOfMemory(cx);
     return false;
   }
-  JS::AutoValueVector fieldRoots(cx);
-  if (!fieldRoots.resize(len)) {
-    JS_ReportOutOfMemory(cx);
-    return false;
-  }
 
   // Process the field types.
   size_t structSize, structAlign;
@@ -5513,7 +5538,6 @@ StructType::DefineInternal(JSContext* cx, JSObject* typeObj_, JSObject* fieldsOb
       Rooted<JSFlatString*> name(cx, ExtractStructField(cx, item, &fieldType));
       if (!name)
         return false;
-      fieldRoots[i].setObject(*fieldType);
 
       // Make sure each field name is unique
       FieldInfoHash::AddPtr entryPtr = fields->lookupForAdd(name);
@@ -5563,11 +5587,11 @@ StructType::DefineInternal(JSContext* cx, JSObject* typeObj_, JSObject* fieldsOb
 
       // Add field name to the hash
       FieldInfo info;
-      info.mType = nullptr; // Value of fields are not yet traceable here.
+      info.mType = fieldType;
       info.mIndex = i;
       info.mOffset = fieldOffset;
       ASSERT_OK(fields->add(entryPtr, name, info));
-      JS_StoreStringPostBarrierCallback(cx, PostBarrierCallback, name, fields.get());
+      JS_StoreStringPostBarrierCallback(cx, PostBarrierCallback, name, fields);
 
       structSize = fieldOffset + fieldSize;
 
@@ -5596,12 +5620,6 @@ StructType::DefineInternal(JSContext* cx, JSObject* typeObj_, JSObject* fieldsOb
   if (!SizeTojsval(cx, structSize, &sizeVal))
     return false;
 
-  for (FieldInfoHash::Range r = fields->all(); !r.empty(); r.popFront()) {
-    FieldInfo& field = r.front().value();
-    MOZ_ASSERT(field.mIndex < fieldRoots.length());
-    field.mType = &fieldRoots[field.mIndex].toObject();
-  }
-
   JS_SetReservedSlot(typeObj, SLOT_FIELDINFO, PRIVATE_TO_JSVAL(fields.release()));
 
   JS_SetReservedSlot(typeObj, SLOT_SIZE, sizeVal);
diff --git a/js/src/ctypes/CTypes.h b/js/src/ctypes/CTypes.h
index 7c6c971e076..a36e56be808 100644
--- a/js/src/ctypes/CTypes.h
+++ b/js/src/ctypes/CTypes.h
@@ -281,6 +281,9 @@ struct FieldHashPolicy : DefaultHasher<JSFlatString*>
 
 typedef HashMap<JSFlatString*, FieldInfo, FieldHashPolicy, SystemAllocPolicy> FieldInfoHash;
 
+void
+TraceFieldInfoHash(JSTracer* trc, FieldInfoHash* fields);
+
 // Descriptor of ABI, return type, argument types, and variadicity for a
 // FunctionType.
 struct FunctionInfo