Revision: 15275
Author: [email protected]
Date: Mon Jun 24 01:38:37 2013
Log: 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
[email protected], [email protected]
Review URL: https://codereview.chromium.org/16917004
http://code.google.com/p/v8/source/detail?r=15275
Modified:
/branches/bleeding_edge/src/frames-inl.h
/branches/bleeding_edge/src/frames.cc
/branches/bleeding_edge/src/frames.h
=======================================
--- /branches/bleeding_edge/src/frames-inl.h Wed May 8 01:08:23 2013
+++ /branches/bleeding_edge/src/frames-inl.h Mon Jun 24 01:38:37 2013
@@ -299,24 +299,21 @@
}
-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 @@
}
-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);
}
=======================================
--- /branches/bleeding_edge/src/frames.cc Mon Jun 3 08:32:22 2013
+++ /branches/bleeding_edge/src/frames.cc Mon Jun 24 01:38:37 2013
@@ -197,6 +197,33 @@
#undef FRAME_TYPE_CASE
}
+
+
+//
-------------------------------------------------------------------------
+
+
+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());
+}
//
-------------------------------------------------------------------------
@@ -339,14 +366,6 @@
return IsValidStackAddress(state.sp) && IsValidStackAddress(state.fp) &&
iterator_.SingletonFor(frame->GetCallerState(&state)) != NULL;
}
-
-
-void SafeStackFrameIterator::Reset() {
- if (is_working_iterator_) {
- iterator_.Reset();
- iteration_done_ = false;
- }
-}
//
-------------------------------------------------------------------------
@@ -354,17 +373,17 @@
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;
}
}
=======================================
--- /branches/bleeding_edge/src/frames.h Fri May 17 01:27:56 2013
+++ /branches/bleeding_edge/src/frames.h Mon Jun 24 01:38:37 2013
@@ -797,10 +797,10 @@
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 @@
// 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 @@
// 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 @@
bool done() const { return iteration_done_ ? true : iterator_.done(); }
void Advance();
- void Reset();
static bool is_active(Isolate* isolate);
@@ -978,16 +947,21 @@
};
-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_;
};
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
