Simplify stack iterators implementation

In order to fix https://code.google.com/p/chromium/issues/detail?id=252097 I
need to change SafeStackTraceFrameIterator. Stack iterators hierarchy looks
excessively complicated and I'd like to flatten it a bit by removing some
intermediate classes. In particular there are two hierarchies sharing
JavaScriptFrameIteratorTemp<T> template for no good reason.

This change extracts some of JavaScriptFrameIteratorTemp functionality directly
into SafeStackTraceFrameIterator. This made it obvious that a few checks were
performed twice.

The rest of JavaScriptFrameIteratorTemp<T> is merged with
JavaScriptFrameIterator. Now that the class is not a template some of its
implementation is moved from frames-inl.h into frames.cc

So in this change I removed JavaScriptFrameIterator and
SafeJavaScriptFrameIterator. As the next step I'm going to merge
SafeStackFrameIterator into SafeStackTraceFrameIterator.

BUG=None
R=loislo@chromium.org, svenpanne@chromium.org

Review URL: https://codereview.chromium.org/16917004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15275 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/frames-inl.h

@@ -299,24 +299,21 @@ inline ConstructFrame::ConstructFrame(StackFrameIterator* iterator)
 }
 
 
-template<typename Iterator>
-inline JavaScriptFrameIteratorTemp<Iterator>::JavaScriptFrameIteratorTemp(
+inline JavaScriptFrameIterator::JavaScriptFrameIterator(
     Isolate* isolate)
     : iterator_(isolate) {
   if (!done()) Advance();
 }
 
 
-template<typename Iterator>
-inline JavaScriptFrameIteratorTemp<Iterator>::JavaScriptFrameIteratorTemp(
+inline JavaScriptFrameIterator::JavaScriptFrameIterator(
     Isolate* isolate, ThreadLocalTop* top)
     : iterator_(isolate, top) {
   if (!done()) Advance();
 }
 
 
-template<typename Iterator>
-inline JavaScriptFrame* JavaScriptFrameIteratorTemp<Iterator>::frame() const {
+inline JavaScriptFrame* JavaScriptFrameIterator::frame() const {
   // TODO(1233797): The frame hierarchy needs to change. It's
   // problematic that we can't use the safe-cast operator to cast to
   // the JavaScript frame type, because we may encounter arguments
@@ -327,43 +324,14 @@ inline JavaScriptFrame* JavaScriptFrameIteratorTemp<Iterator>::frame() const {
 }
 
 
-template<typename Iterator>
-JavaScriptFrameIteratorTemp<Iterator>::JavaScriptFrameIteratorTemp(
-    Isolate* isolate, StackFrame::Id id)
-    : iterator_(isolate) {
-  AdvanceToId(id);
-}
-
-
-template<typename Iterator>
-void JavaScriptFrameIteratorTemp<Iterator>::Advance() {
-  do {
-    iterator_.Advance();
-  } while (!iterator_.done() && !iterator_.frame()->is_java_script());
-}
-
-
-template<typename Iterator>
-void JavaScriptFrameIteratorTemp<Iterator>::AdvanceToArgumentsFrame() {
-  if (!frame()->has_adapted_arguments()) return;
-  iterator_.Advance();
-  ASSERT(iterator_.frame()->is_arguments_adaptor());
-}
-
-
-template<typename Iterator>
-void JavaScriptFrameIteratorTemp<Iterator>::AdvanceToId(StackFrame::Id id) {
-  while (!done()) {
-    Advance();
-    if (frame()->id() == id) return;
-  }
-}
-
-
-template<typename Iterator>
-void JavaScriptFrameIteratorTemp<Iterator>::Reset() {
-  iterator_.Reset();
-  if (!done()) Advance();
+inline JavaScriptFrame* SafeStackTraceFrameIterator::frame() const {
+  // TODO(1233797): The frame hierarchy needs to change. It's
+  // problematic that we can't use the safe-cast operator to cast to
+  // the JavaScript frame type, because we may encounter arguments
+  // adaptor frames.
+  StackFrame* frame = iterator_.frame();
+  ASSERT(frame->is_java_script());
+  return static_cast<JavaScriptFrame*>(frame);
 }
 
 

src/frames.cc

@@ -202,6 +202,33 @@ StackFrame* StackFrameIterator::SingletonFor(StackFrame::Type type) {
 // -------------------------------------------------------------------------
 
 
+JavaScriptFrameIterator::JavaScriptFrameIterator(
+    Isolate* isolate, StackFrame::Id id)
+    : iterator_(isolate) {
+  while (!done()) {
+    Advance();
+    if (frame()->id() == id) return;
+  }
+}
+
+
+void JavaScriptFrameIterator::Advance() {
+  do {
+    iterator_.Advance();
+  } while (!iterator_.done() && !iterator_.frame()->is_java_script());
+}
+
+
+void JavaScriptFrameIterator::AdvanceToArgumentsFrame() {
+  if (!frame()->has_adapted_arguments()) return;
+  iterator_.Advance();
+  ASSERT(iterator_.frame()->is_arguments_adaptor());
+}
+
+
+// -------------------------------------------------------------------------
+
+
 StackTraceFrameIterator::StackTraceFrameIterator(Isolate* isolate)
     : JavaScriptFrameIterator(isolate) {
   if (!done() && !IsValidFrame()) Advance();
@@ -341,30 +368,22 @@ bool SafeStackFrameIterator::IsValidCaller(StackFrame* frame) {
 }
 
 
-void SafeStackFrameIterator::Reset() {
-  if (is_working_iterator_) {
-    iterator_.Reset();
-    iteration_done_ = false;
-  }
-}
-
-
 // -------------------------------------------------------------------------
 
 
 SafeStackTraceFrameIterator::SafeStackTraceFrameIterator(
     Isolate* isolate,
-    Address fp, Address sp, Address low_bound, Address high_bound) :
-    SafeJavaScriptFrameIterator(isolate, fp, sp, low_bound, high_bound) {
-  if (!done() && !frame()->is_java_script()) Advance();
+    Address fp, Address sp, Address low_bound, Address high_bound)
+    : iterator_(isolate, fp, sp, low_bound, high_bound) {
+  if (!done()) Advance();
 }
 
 
 void SafeStackTraceFrameIterator::Advance() {
   while (true) {
-    SafeJavaScriptFrameIterator::Advance();
-    if (done()) return;
-    if (frame()->is_java_script()) return;
+    iterator_.Advance();
+    if (iterator_.done()) return;
+    if (iterator_.frame()->is_java_script()) return;
   }
 }
 

src/frames.h

@@ -797,10 +797,10 @@ class StackFrameIterator BASE_EMBEDDED {
   bool done() const { return frame_ == NULL; }
   void Advance() { (this->*advance_)(); }
 
+ private:
   // Go back to the first frame.
   void Reset();
 
- private:
   Isolate* isolate_;
 #define DECLARE_SINGLETON(ignore, type) type type##_;
   STACK_FRAME_TYPE_LIST(DECLARE_SINGLETON)
@@ -832,34 +832,12 @@ class StackFrameIterator BASE_EMBEDDED {
 
 
 // Iterator that supports iterating through all JavaScript frames.
-template<typename Iterator>
-class JavaScriptFrameIteratorTemp BASE_EMBEDDED {
+class JavaScriptFrameIterator BASE_EMBEDDED {
  public:
-  inline explicit JavaScriptFrameIteratorTemp(Isolate* isolate);
-
-  inline JavaScriptFrameIteratorTemp(Isolate* isolate, ThreadLocalTop* top);
-
+  inline explicit JavaScriptFrameIterator(Isolate* isolate);
+  inline JavaScriptFrameIterator(Isolate* isolate, ThreadLocalTop* top);
   // Skip frames until the frame with the given id is reached.
-  explicit JavaScriptFrameIteratorTemp(StackFrame::Id id) { AdvanceToId(id); }
-
-  inline JavaScriptFrameIteratorTemp(Isolate* isolate, StackFrame::Id id);
-
-  JavaScriptFrameIteratorTemp(Address fp,
-                              Address sp,
-                              Address low_bound,
-                              Address high_bound) :
-      iterator_(fp, sp, low_bound, high_bound) {
-    if (!done()) Advance();
-  }
-
-  JavaScriptFrameIteratorTemp(Isolate* isolate,
-                              Address fp,
-                              Address sp,
-                              Address low_bound,
-                              Address high_bound) :
-      iterator_(isolate, fp, sp, low_bound, high_bound) {
-    if (!done()) Advance();
-  }
+  JavaScriptFrameIterator(Isolate* isolate, StackFrame::Id id);
 
   inline JavaScriptFrame* frame() const;
 
@@ -871,19 +849,11 @@ class JavaScriptFrameIteratorTemp BASE_EMBEDDED {
   // arguments.
   void AdvanceToArgumentsFrame();
 
-  // Go back to the first frame.
-  void Reset();
-
  private:
-  inline void AdvanceToId(StackFrame::Id id);
-
-  Iterator iterator_;
+  StackFrameIterator iterator_;
 };
 
 
-typedef JavaScriptFrameIteratorTemp<StackFrameIterator> JavaScriptFrameIterator;
-
-
 // NOTE: The stack trace frame iterator is an iterator that only
 // traverse proper JavaScript frames; that is JavaScript frames that
 // have proper JavaScript functions. This excludes the problematic
@@ -913,7 +883,6 @@ class SafeStackFrameIterator BASE_EMBEDDED {
   bool done() const { return iteration_done_ ? true : iterator_.done(); }
 
   void Advance();
-  void Reset();
 
   static bool is_active(Isolate* isolate);
 
@@ -978,16 +947,21 @@ class SafeStackFrameIterator BASE_EMBEDDED {
 };
 
 
-typedef JavaScriptFrameIteratorTemp<SafeStackFrameIterator>
-    SafeJavaScriptFrameIterator;
+class SafeStackTraceFrameIterator BASE_EMBEDDED {
+ public:
+  SafeStackTraceFrameIterator(Isolate* isolate,
+                              Address fp,
+                              Address sp,
+                              Address low_bound,
+                              Address high_bound);
 
+  inline JavaScriptFrame* frame() const;
 
-class SafeStackTraceFrameIterator: public SafeJavaScriptFrameIterator {
- public:
-  explicit SafeStackTraceFrameIterator(Isolate* isolate,
-                                       Address fp, Address sp,
-                                       Address low_bound, Address high_bound);
+  bool done() const { return iterator_.done(); }
   void Advance();
+
+ private:
+  SafeStackFrameIterator iterator_;
 };