Cleanup StringCharacterStream and add initial test cases.

src/objects-inl.h

Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 9ff35efbcf90aac117d2f1001fed87d705a8741e..638e62b289df6d829aaefe62521cdc96ba2acd92 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -2517,14 +2517,12 @@ void String::Visit(
     String* string,
     unsigned offset,
     Visitor& visitor,
-    ConsOp& consOp,
+    ConsOp& cons_op,
     int32_t type,
     unsigned length) {
-
   ASSERT(length == static_cast<unsigned>(string->length()));
   ASSERT(offset <= length);
-
-  unsigned sliceOffset = offset;
+  unsigned slice_offset = offset;
   while (true) {
     ASSERT(type == string->map()->instance_type());
 
@@ -2532,35 +2530,36 @@ void String::Visit(
       case kSeqStringTag | kOneByteStringTag:
         visitor.VisitOneByteString(
             reinterpret_cast<const uint8_t*>(
-                SeqOneByteString::cast(string)->GetChars()) + sliceOffset,
+                SeqOneByteString::cast(string)->GetChars()) + slice_offset,
                 length - offset);
         return;
 
       case kSeqStringTag | kTwoByteStringTag:
         visitor.VisitTwoByteString(
             reinterpret_cast<const uint16_t*>(
-                SeqTwoByteString::cast(string)->GetChars()) + sliceOffset,
+                SeqTwoByteString::cast(string)->GetChars()) + slice_offset,
                 length - offset);
         return;
 
       case kExternalStringTag | kOneByteStringTag:
         visitor.VisitOneByteString(
             reinterpret_cast<const uint8_t*>(
-                ExternalAsciiString::cast(string)->GetChars()) + sliceOffset,
+                ExternalAsciiString::cast(string)->GetChars()) + slice_offset,
                 length - offset);
         return;
 
       case kExternalStringTag | kTwoByteStringTag:
         visitor.VisitTwoByteString(
             reinterpret_cast<const uint16_t*>(
-                ExternalTwoByteString::cast(string)->GetChars()) + sliceOffset,
+                ExternalTwoByteString::cast(string)->GetChars())
+                    + slice_offset,
                 length - offset);
         return;
 
       case kSlicedStringTag | kOneByteStringTag:
       case kSlicedStringTag | kTwoByteStringTag: {
         SlicedString* slicedString = SlicedString::cast(string);
-        sliceOffset += slicedString->offset();
+        slice_offset += slicedString->offset();
         string = slicedString->parent();
         type = string->map()->instance_type();
         continue;
@@ -2568,10 +2567,10 @@ void String::Visit(
 
       case kConsStringTag | kOneByteStringTag:
       case kConsStringTag | kTwoByteStringTag:
-        string = consOp.Operate(ConsString::cast(string), &offset, &type,
+        string = cons_op.Operate(ConsString::cast(string), &offset, &type,
             &length);
         if (string == NULL) return;
-        sliceOffset = offset;
+        slice_offset = offset;
         ASSERT(length == static_cast<unsigned>(string->length()));
         continue;
 
@@ -2766,34 +2765,14 @@ unsigned ConsStringIteratorOp::OffsetForDepth(unsigned depth) {
 }
 
 
-uint32_t ConsStringIteratorOp::MaskForDepth(unsigned depth) {
-  return 1 << OffsetForDepth(depth);
-}
-
-
-void ConsStringIteratorOp::SetRightDescent() {
-  trace_ |= MaskForDepth(depth_ - 1);
-}
-
-
-void ConsStringIteratorOp::ClearRightDescent() {
-  trace_ &= ~MaskForDepth(depth_ - 1);
-}
-
-
 void ConsStringIteratorOp::PushLeft(ConsString* string) {
   frames_[depth_++ & kDepthMask] = string;
 }
 
 
-void ConsStringIteratorOp::PushRight(ConsString* string, int32_t type) {
-  // Inplace update
+void ConsStringIteratorOp::PushRight(ConsString* string) {
+  // Inplace update.
   frames_[(depth_-1) & kDepthMask] = string;
-  if (depth_ != 1) return;
-  // Optimization: can replace root in this case.
-  root_ = string;
-  root_type_ = type;
-  root_length_ = string->length();
 }
 
 
@@ -2810,8 +2789,8 @@ void ConsStringIteratorOp::Pop() {
 
 
 void ConsStringIteratorOp::Reset() {
-  consumed_ = 0;
-  ResetStack();
+  depth_ = 0;
+  maximum_depth_ = 0;
 }
 
 
@@ -2820,19 +2799,13 @@ bool ConsStringIteratorOp::HasMore() {
 }
 
 
-void ConsStringIteratorOp::ResetStack() {
-  depth_ = 0;
-  maximum_depth_ = 0;
-}
-
-
 bool ConsStringIteratorOp::ContinueOperation(ContinueResponse* response) {
-  bool blewStack;
+  bool blew_stack;
   int32_t type;
-  String* string = NextLeaf(&blewStack, &type);
+  unsigned length;
+  String* string = NextLeaf(&blew_stack, &type, &length);
   // String found.
   if (string != NULL) {
-    unsigned length = string->length();
     consumed_ += length;
     response->string_ = string;
     response->offset_ = 0;
@@ -2841,9 +2814,11 @@ bool ConsStringIteratorOp::ContinueOperation(ContinueResponse* response) {
     return true;
   }
   // Traversal complete.
-  if (!blewStack) return false;
+  if (!blew_stack) return false;
   // Restart search.
-  ResetStack();
+  Reset();
+  // TODO(dcarney) This is unnecessary.
+  // After a reset, we don't need a String::Visit
   response->string_ = root_;
   response->offset_ = consumed_;
   response->length_ = root_length_;
@@ -2853,14 +2828,14 @@ bool ConsStringIteratorOp::ContinueOperation(ContinueResponse* response) {
 
 
 uint16_t StringCharacterStream::GetNext() {
-  ASSERT(buffer8_ != NULL);
+  ASSERT((buffer8_ == NULL && end_ == NULL) || buffer8_ < end_);
   return is_one_byte_ ? *buffer8_++ : *buffer16_++;
 }
 
 
 StringCharacterStream::StringCharacterStream(
     String* string, unsigned offset, ConsStringIteratorOp* op)
-  : is_one_byte_(true),
+  : is_one_byte_(false),
     buffer8_(NULL),
     end_(NULL),
     op_(op) {
@@ -2874,11 +2849,7 @@ bool StringCharacterStream::HasMore() {
   if (buffer8_ != end_) return true;
   if (!op_->HasMore()) return false;
   ConsStringIteratorOp::ContinueResponse response;
-  // This has been checked above
-  if (!op_->ContinueOperation(&response)) {
-    UNREACHABLE();
-    return false;
-  }
+  if (!op_->ContinueOperation(&response)) return false;
   String::Visit(response.string_,
       response.offset_, *this, *op_, response.type_, response.length_);
   return true;