Bug 1428075 - Remove non-rooted saved frame iterators r=fitzgen
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 05 Jan 2018 10:42:16 +0000
changeset 449749 134e8f9868a1019310e43f6092aef8dbce090c6e
parent 449748 21060660b9386c7f9cab116cec114297139d2015
child 449750 86275396a96b61ca5983d2a0d5e3dcda1a3c33d6
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1428075
milestone59.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 1428075 - Remove non-rooted saved frame iterators r=fitzgen
js/src/jsapi-tests/testSavedStacks.cpp
js/src/jsapi-tests/testStructuredClone.cpp
js/src/vm/SavedFrame.h
--- a/js/src/jsapi-tests/testSavedStacks.cpp
+++ b/js/src/jsapi-tests/testSavedStacks.cpp
@@ -81,30 +81,16 @@ BEGIN_TEST(testSavedStacks_RangeBasedFor
                    &val));
 
     CHECK(val.isObject());
     JS::RootedObject obj(cx, &val.toObject());
 
     CHECK(obj->is<js::SavedFrame>());
     JS::Rooted<js::SavedFrame*> savedFrame(cx, &obj->as<js::SavedFrame>());
 
-    js::SavedFrame* f = savedFrame.get();
-    for (auto& frame : *savedFrame.get()) {
-        CHECK(&frame == f);
-        f = f->getParent();
-    }
-    CHECK(f == nullptr);
-
-    const js::SavedFrame* cf = savedFrame.get();
-    for (const auto& frame : *savedFrame.get()) {
-        CHECK(&frame == cf);
-        cf = cf->getParent();
-    }
-    CHECK(cf == nullptr);
-
     JS::Rooted<js::SavedFrame*> rf(cx, savedFrame);
     for (JS::Handle<js::SavedFrame*> frame : js::SavedFrame::RootedRange(cx, rf)) {
         JS_GC(cx);
         CHECK(frame == rf);
         rf = rf->getParent();
     }
     CHECK(rf == nullptr);
 
--- a/js/src/jsapi-tests/testStructuredClone.cpp
+++ b/js/src/jsapi-tests/testStructuredClone.cpp
@@ -185,45 +185,45 @@ BEGIN_TEST(testStructuredClone_SavedFram
 
         JS::RootedValue destVal(cx);
         CHECK(JS_StructuredClone(cx, srcVal, &destVal, nullptr, nullptr));
 
         CHECK(destVal.isObject());
         JS::RootedObject destObj(cx, &destVal.toObject());
 
         CHECK(destObj->is<js::SavedFrame>());
-        auto destFrame = &destObj->as<js::SavedFrame>();
+        JS::Handle<js::SavedFrame*> destFrame = destObj.as<js::SavedFrame>();
 
         size_t framesCopied = 0;
-        for (auto& f : *destFrame) {
+        for (JS::Handle<js::SavedFrame*> f : js::SavedFrame::RootedRange(cx, destFrame)) {
             framesCopied++;
 
-            CHECK(&f != srcFrame);
+            CHECK(f != srcFrame);
 
             if (pp->principals == testPrincipals) {
                 // We shouldn't get a pointer to the same
                 // StructuredCloneTestPrincipals instance since we should have
                 // serialized and then deserialized it into a new instance.
-                CHECK(f.getPrincipals() != pp->principals);
+                CHECK(f->getPrincipals() != pp->principals);
 
                 // But it should certainly have the same rank.
-                CHECK(StructuredCloneTestPrincipals::getRank(f.getPrincipals()) ==
+                CHECK(StructuredCloneTestPrincipals::getRank(f->getPrincipals()) ==
                       StructuredCloneTestPrincipals::getRank(pp->principals));
             } else {
                 // For our singleton principals, we should always get the same
                 // pointer back.
                 CHECK(js::ReconstructedSavedFramePrincipals::is(pp->principals) ||
                       pp->principals == nullptr);
-                CHECK(f.getPrincipals() == pp->principals);
+                CHECK(f->getPrincipals() == pp->principals);
             }
 
-            CHECK(EqualStrings(f.getSource(), srcFrame->getSource()));
-            CHECK(f.getLine() == srcFrame->getLine());
-            CHECK(f.getColumn() == srcFrame->getColumn());
-            CHECK(EqualStrings(f.getFunctionDisplayName(), srcFrame->getFunctionDisplayName()));
+            CHECK(EqualStrings(f->getSource(), srcFrame->getSource()));
+            CHECK(f->getLine() == srcFrame->getLine());
+            CHECK(f->getColumn() == srcFrame->getColumn());
+            CHECK(EqualStrings(f->getFunctionDisplayName(), srcFrame->getFunctionDisplayName()));
 
             srcFrame = srcFrame->getParent();
         }
 
         // Four function frames + one global frame.
         CHECK(framesCopied == 4);
     }
 
--- a/js/src/vm/SavedFrame.h
+++ b/js/src/vm/SavedFrame.h
@@ -46,57 +46,26 @@ class SavedFrame : public NativeObject {
     uint32_t      getLine();
     uint32_t      getColumn();
     JSAtom*       getFunctionDisplayName();
     JSAtom*       getAsyncCause();
     SavedFrame*   getParent() const;
     JSPrincipals* getPrincipals();
     bool          isSelfHosted(JSContext* cx);
 
-    // Iterators for use with C++11 range based for loops, eg:
-    //
-    //     SavedFrame* stack = getSomeSavedFrameStack();
-    //     for (const SavedFrame* frame : *stack) {
-    //         ...
-    //     }
-    //
-    // If you need to keep each frame rooted during iteration, you can use
-    // `SavedFrame::RootedRange`. Each frame yielded by
-    // `SavedFrame::RootedRange` is only a valid handle to a rooted `SavedFrame`
-    // within the loop's block for a single loop iteration. When the next
-    // iteration begins, the value is invalidated.
+    // Iterator for use with C++11 range based for loops, eg:
     //
     //     RootedSavedFrame stack(cx, getSomeSavedFrameStack());
     //     for (HandleSavedFrame frame : SavedFrame::RootedRange(cx, stack)) {
     //         ...
     //     }
-
-    class Iterator {
-        SavedFrame* frame_;
-      public:
-        explicit Iterator(SavedFrame* frame) : frame_(frame) { }
-        SavedFrame& operator*() const { MOZ_ASSERT(frame_); return *frame_; }
-        bool operator!=(const Iterator& rhs) const { return rhs.frame_ != frame_; }
-        inline void operator++();
-    };
-
-    Iterator begin() { return Iterator(this); }
-    Iterator end() { return Iterator(nullptr); }
-
-    class ConstIterator {
-        const SavedFrame* frame_;
-      public:
-        explicit ConstIterator(const SavedFrame* frame) : frame_(frame) { }
-        const SavedFrame& operator*() const { MOZ_ASSERT(frame_); return *frame_; }
-        bool operator!=(const ConstIterator& rhs) const { return rhs.frame_ != frame_; }
-        inline void operator++();
-    };
-
-    ConstIterator begin() const { return ConstIterator(this); }
-    ConstIterator end() const { return ConstIterator(nullptr); }
+    //
+    // Each frame yielded by `SavedFrame::RootedRange` is only a valid handle to
+    // a rooted `SavedFrame` within the loop's block for a single loop
+    // iteration. When the next iteration begins, the value is invalidated.
 
     class RootedRange;
 
     class MOZ_STACK_CLASS RootedIterator {
         friend class RootedRange;
         RootedRange* range_;
         // For use by RootedRange::end() only.
         explicit RootedIterator() : range_(nullptr) { }
@@ -245,28 +214,16 @@ struct ReconstructedSavedFramePrincipals
     // given JS::ubi::StackFrame that is being reconstructed as a SavedFrame
     // stack.
     static JSPrincipals* getSingleton(JS::ubi::StackFrame& f) {
         return f.isSystem() ? &IsSystem : &IsNotSystem;
     }
 };
 
 inline void
-SavedFrame::Iterator::operator++()
-{
-    frame_ = frame_->getParent();
-}
-
-inline void
-SavedFrame::ConstIterator::operator++()
-{
-    frame_ = frame_->getParent();
-}
-
-inline void
 SavedFrame::RootedIterator::operator++()
 {
     MOZ_ASSERT(range_);
     range_->frame_ = range_->frame_->getParent();
 }
 
 } // namespace js