From 0d78b57012fd85cf0dc4498d7e9a813ae9682dfc Mon Sep 17 00:00:00 2001 From: Terrence Cole Date: Fri, 24 Jul 2015 15:09:28 -0700 Subject: [PATCH] Bug 1188197 - Allow PersistentRooted to store DynamicTraceable; r=sfink --- js/public/RootingAPI.h | 7 ++- js/src/gc/RootMarking.cpp | 10 ++++- js/src/jsapi-tests/testGCExactRooting.cpp | 53 +++++++++++++++++++++++ js/src/jsgc.cpp | 1 + 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h index cdaeb56430f..1dd807fa336 100644 --- a/js/public/RootingAPI.h +++ b/js/public/RootingAPI.h @@ -997,6 +997,10 @@ template class PersistentRooted : public js::PersistentRootedBase, private mozilla::LinkedListElement> { + static_assert(!mozilla::IsConvertible::value && + !mozilla::IsConvertible::value, + "Rooted takes pointer or Traceable types but not Traceable* type"); + typedef mozilla::LinkedListElement> ListBase; friend class mozilla::LinkedList; @@ -1016,7 +1020,8 @@ class PersistentRooted : public js::PersistentRootedBase, kind == js::THING_ROOT_SCRIPT || kind == js::THING_ROOT_STRING || kind == js::THING_ROOT_ID || - kind == js::THING_ROOT_VALUE); + kind == js::THING_ROOT_VALUE || + kind == js::THING_ROOT_DYNAMIC_TRACEABLE); } public: diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index c2afc8e7f0d..b725722d13a 100644 --- a/js/src/gc/RootMarking.cpp +++ b/js/src/gc/RootMarking.cpp @@ -304,13 +304,14 @@ struct PersistentRootedMarker { typedef PersistentRooted Element; typedef mozilla::LinkedList List; - typedef void (*MarkFunc)(JSTracer* trc, T* ref, const char* name); + using TraceFunc = void (*)(JSTracer* trc, T* ref, const char* name); + template TraceFn = TraceNullableRoot> static void markChain(JSTracer* trc, List& list, const char* name) { for (Element* r = list.getFirst(); r; r = r->getNext()) - TraceNullableRoot(trc, r->address(), name); + TraceFn(trc, r->address(), name); } }; @@ -331,6 +332,11 @@ js::gc::MarkPersistentRootedChainsInLists(RootLists& roots, JSTracer* trc) "PersistentRooted"); PersistentRootedMarker::markChain(trc, roots.getPersistentRootedList(), "PersistentRooted"); + + PersistentRootedMarker::markChain(trc, + reinterpret_cast>&>( + roots.heapRoots_[THING_ROOT_DYNAMIC_TRACEABLE]), + "PersistentRooted"); } void diff --git a/js/src/jsapi-tests/testGCExactRooting.cpp b/js/src/jsapi-tests/testGCExactRooting.cpp index 3ac3b1b2b4d..92ec1c0c78f 100644 --- a/js/src/jsapi-tests/testGCExactRooting.cpp +++ b/js/src/jsapi-tests/testGCExactRooting.cpp @@ -123,6 +123,15 @@ struct RootedBase { RelocatablePtrObject& obj() { return static_cast*>(this)->get().obj; } RelocatablePtrString& str() { return static_cast*>(this)->get().str; } }; +template <> +struct PersistentRootedBase { + RelocatablePtrObject& obj() { + return static_cast*>(this)->get().obj; + } + RelocatablePtrString& str() { + return static_cast*>(this)->get().str; + } +}; } // namespace js BEGIN_TEST(testGCRootedDynamicStructInternalStackStorage) @@ -153,6 +162,40 @@ BEGIN_TEST(testGCRootedDynamicStructInternalStackStorageAugmented) JS::RootedObject obj(cx, container.obj()); JS::RootedValue val(cx, StringValue(container.str())); CHECK(JS_SetProperty(cx, obj, "foo", val)); + obj = nullptr; + val = UndefinedValue(); + + { + JS::RootedString actual(cx); + bool same; + + // Automatic move from stack to heap. + JS::PersistentRooted heap(cx, container); + + // clear prior rooting. + container.obj() = nullptr; + container.str() = nullptr; + + obj = heap.obj(); + CHECK(JS_GetProperty(cx, obj, "foo", &val)); + actual = val.toString(); + CHECK(JS_StringEqualsAscii(cx, actual, "Hello", &same)); + CHECK(same); + obj = nullptr; + actual = nullptr; + + JS_GC(cx->runtime()); + JS_GC(cx->runtime()); + + obj = heap.obj(); + CHECK(JS_GetProperty(cx, obj, "foo", &val)); + actual = val.toString(); + CHECK(JS_StringEqualsAscii(cx, actual, "Hello", &same)); + CHECK(same); + obj = nullptr; + actual = nullptr; + } + return true; } END_TEST(testGCRootedDynamicStructInternalStackStorageAugmented) @@ -232,6 +275,16 @@ BEGIN_TEST(testGCHandleHashMap) CHECK(CheckMyHashMap(cx, map)); + // Unfortunately, the type of get() needs to be non-const ref, so + // we need to explicitly Move the storage. + JS::PersistentRooted heapMap(cx, mozilla::Move(map.get())); + CHECK(CheckMyHashMap(cx, heapMap)); + + JS_GC(rt); + JS_GC(rt); + + CHECK(CheckMyHashMap(cx, heapMap)); + return true; } END_TEST(testGCHandleHashMap) diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index d5d4f5654db..e45f1f288c6 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -1371,6 +1371,7 @@ js::gc::FinishPersistentRootedChains(RootLists& roots) FinishPersistentRootedChain(roots.getPersistentRootedList()); FinishPersistentRootedChain(roots.getPersistentRootedList()); FinishPersistentRootedChain(roots.getPersistentRootedList()); + FinishPersistentRootedChain(roots.heapRoots_[THING_ROOT_DYNAMIC_TRACEABLE]); } void