[runtime] Implement encodeURI as single runtime function.

src/runtime/runtime-strings.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "src/runtime/runtime-utils.h"
-
 #include "src/arguments.h"
 #include "src/regexp/jsregexp-inl.h"
+#include "src/runtime/runtime-utils.h"
 #include "src/string-builder.h"
+
 #include "src/string-search.h"
 
 namespace v8 {
 namespace internal {
 
 
 // Perform string match of pattern on subject, starting at start index.
 // Caller must ensure that 0 <= start_index <= sub->length(),
 // and should check that pat->length() + start_index <= sub->length().
-int StringMatch(Isolate* isolate, Handle<String> sub, Handle<String> pat,
+int StringMatch(Isolate *isolate, Handle<String> sub, Handle<String> pat,
                 int start_index) {
   DCHECK(0 <= start_index);
   DCHECK(start_index <= sub->length());
 
   int pattern_length = pat->length();
   if (pattern_length == 0) return start_index;
 
   int subject_length = sub->length();
   if (start_index + pattern_length > subject_length) return -1;
 
@@ skipping 20 matching lines @@
     return SearchString(isolate, seq_sub.ToOneByteVector(), pat_vector,
                         start_index);
   }
   return SearchString(isolate, seq_sub.ToUC16Vector(), pat_vector, start_index);
 }
 
 
 // This may return an empty MaybeHandle if an exception is thrown or
 // we abort due to reaching the recursion limit.
 MaybeHandle<String> StringReplaceOneCharWithString(
-    Isolate* isolate, Handle<String> subject, Handle<String> search,
-    Handle<String> replace, bool* found, int recursion_limit) {
+    Isolate *isolate, Handle<String> subject, Handle<String> search,

[Yang, 2016/05/12 07:39:21]

V8's convention is to have the asterisk at the type, not the variable name. "git
cl format" probably will fix this, among other things.

[Franzi, 2016/05/13 09:42:02]

Done.

+    Handle<String> replace, bool *found, int recursion_limit) {
   StackLimitCheck stackLimitCheck(isolate);
   if (stackLimitCheck.HasOverflowed() || (recursion_limit == 0)) {
     return MaybeHandle<String>();
   }
   recursion_limit--;
   if (subject->IsConsString()) {
-    ConsString* cons = ConsString::cast(*subject);
+    ConsString *cons = ConsString::cast(*subject);
     Handle<String> first = Handle<String>(cons->first());
     Handle<String> second = Handle<String>(cons->second());
     Handle<String> new_first;
     if (!StringReplaceOneCharWithString(isolate, first, search, replace, found,
                                         recursion_limit).ToHandle(&new_first)) {
       return MaybeHandle<String>();
     }
     if (*found) return isolate->factory()->NewConsString(new_first, second);
 
     Handle<String> new_second;
@@ skipping 59 matching lines @@
   CONVERT_ARG_HANDLE_CHECKED(Object, index, 2);
 
   uint32_t start_index = 0;
   if (!index->ToArrayIndex(&start_index)) return Smi::FromInt(-1);
 
   RUNTIME_ASSERT(start_index <= static_cast<uint32_t>(sub->length()));
   int position = StringMatch(isolate, sub, pat, start_index);
   return Smi::FromInt(position);
 }
 
-
 template <typename schar, typename pchar>
 static int StringMatchBackwards(Vector<const schar> subject,
                                 Vector<const pchar> pattern, int idx) {
   int pattern_length = pattern.length();
   DCHECK(pattern_length >= 1);
   DCHECK(idx + pattern_length <= subject.length());
 
   if (sizeof(schar) == 1 && sizeof(pchar) > 1) {
     for (int i = 0; i < pattern_length; i++) {
       uc16 c = pattern[i];
@@ skipping 181 matching lines @@
 
   RegExpImpl::GlobalCache global_cache(regexp, subject, isolate);
   if (global_cache.HasException()) return isolate->heap()->exception();
 
   int capture_count = regexp->CaptureCount();
 
   ZoneScope zone_scope(isolate->runtime_zone());
   ZoneList<int> offsets(8, zone_scope.zone());
 
   while (true) {
-    int32_t* match = global_cache.FetchNext();
+    int32_t *match = global_cache.FetchNext();
     if (match == NULL) break;
     offsets.Add(match[0], zone_scope.zone());  // start
     offsets.Add(match[1], zone_scope.zone());  // end
   }
 
   if (global_cache.HasException()) return isolate->heap()->exception();
 
   if (offsets.length() == 0) {
     // Not a single match.
     return isolate->heap()->null_value();
@@ skipping 86 matching lines @@
   int special_length = special->length();
   if (!array->HasFastObjectElements()) {
     return isolate->Throw(isolate->heap()->illegal_argument_string());
   }
 
   int length;
   bool one_byte = special->HasOnlyOneByteChars();
 
   {
     DisallowHeapAllocation no_gc;
-    FixedArray* fixed_array = FixedArray::cast(array->elements());
+    FixedArray *fixed_array = FixedArray::cast(array->elements());
     if (fixed_array->length() < array_length) {
       array_length = fixed_array->length();
     }
 
     if (array_length == 0) {
       return isolate->heap()->empty_string();
     } else if (array_length == 1) {
-      Object* first = fixed_array->get(0);
+      Object *first = fixed_array->get(0);
       if (first->IsString()) return first;
     }
     length = StringBuilderConcatLength(special_length, fixed_array,
                                        array_length, &one_byte);
   }
 
   if (length == -1) {
     return isolate->Throw(isolate->heap()->illegal_argument_string());
   }
 
@@ skipping 30 matching lines @@
   RUNTIME_ASSERT(array_length >= 0);
 
   Handle<FixedArray> fixed_array(FixedArray::cast(array->elements()));
   if (fixed_array->length() < array_length) {
     array_length = fixed_array->length();
   }
 
   if (array_length == 0) {
     return isolate->heap()->empty_string();
   } else if (array_length == 1) {
-    Object* first = fixed_array->get(0);
+    Object *first = fixed_array->get(0);
     RUNTIME_ASSERT(first->IsString());
     return first;
   }
 
   int separator_length = separator->length();
   RUNTIME_ASSERT(separator_length > 0);
   int max_nof_separators =
       (String::kMaxLength + separator_length - 1) / separator_length;
   if (max_nof_separators < (array_length - 1)) {
     THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewInvalidStringLengthError());
   }
   int length = (array_length - 1) * separator_length;
   for (int i = 0; i < array_length; i++) {
-    Object* element_obj = fixed_array->get(i);
+    Object *element_obj = fixed_array->get(i);
     RUNTIME_ASSERT(element_obj->IsString());
-    String* element = String::cast(element_obj);
+    String *element = String::cast(element_obj);
     int increment = element->length();
     if (increment > String::kMaxLength - length) {
       STATIC_ASSERT(String::kMaxLength < kMaxInt);
       length = kMaxInt;  // Provoke exception;
       break;
     }
     length += increment;
   }
 
   Handle<SeqTwoByteString> answer;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, answer, isolate->factory()->NewRawTwoByteString(length));
 
   DisallowHeapAllocation no_gc;
 
-  uc16* sink = answer->GetChars();
+  uc16 *sink = answer->GetChars();
 #ifdef DEBUG
   uc16* end = sink + length;
 #endif
 
   RUNTIME_ASSERT(fixed_array->get(0)->IsString());
-  String* first = String::cast(fixed_array->get(0));
-  String* separator_raw = *separator;
+  String *first = String::cast(fixed_array->get(0));
+  String *separator_raw = *separator;
 
   int first_length = first->length();
   String::WriteToFlat(first, sink, 0, first_length);
   sink += first_length;
 
   for (int i = 1; i < array_length; i++) {
     DCHECK(sink + separator_length <= end);
     String::WriteToFlat(separator_raw, sink, 0, separator_length);
     sink += separator_length;
 
     RUNTIME_ASSERT(fixed_array->get(i)->IsString());
-    String* element = String::cast(fixed_array->get(i));
+    String *element = String::cast(fixed_array->get(i));
     int element_length = element->length();
     DCHECK(sink + element_length <= end);
     String::WriteToFlat(element, sink, 0, element_length);
     sink += element_length;
   }
   DCHECK(sink == end);
 
   // Use %_FastOneByteArrayJoin instead.
   DCHECK(!answer->IsOneByteRepresentation());
   return *answer;
 }
 
 template <typename sinkchar>
-static void WriteRepeatToFlat(String* src, Vector<sinkchar> buffer, int cursor,
+static void WriteRepeatToFlat(String *src, Vector<sinkchar> buffer, int cursor,
                               int repeat, int length) {
   if (repeat == 0) return;
 
-  sinkchar* start = &buffer[cursor];
+  sinkchar *start = &buffer[cursor];
   String::WriteToFlat<sinkchar>(src, start, 0, length);
 
   int done = 1;
-  sinkchar* next = start + length;
+  sinkchar *next = start + length;
 
   while (done < repeat) {
     int block = Min(done, repeat - done);
     int block_chars = block * length;
     CopyChars(next, start, block_chars);
     next += block_chars;
     done += block;
   }
 }
 
 template <typename Char>
-static void JoinSparseArrayWithSeparator(FixedArray* elements,
+static void JoinSparseArrayWithSeparator(FixedArray *elements,
                                          int elements_length,
                                          uint32_t array_length,
-                                         String* separator,
+                                         String *separator,
                                          Vector<Char> buffer) {
   DisallowHeapAllocation no_gc;
   int previous_separator_position = 0;
   int separator_length = separator->length();
   DCHECK_LT(0, separator_length);
   int cursor = 0;
   for (int i = 0; i < elements_length; i += 2) {
     int position = NumberToInt32(elements->get(i));
-    String* string = String::cast(elements->get(i + 1));
+    String *string = String::cast(elements->get(i + 1));
     int string_length = string->length();
     if (string->length() > 0) {
       int repeat = position - previous_separator_position;
       WriteRepeatToFlat<Char>(separator, buffer, cursor, repeat,
                               separator_length);
       cursor += repeat * separator_length;
       previous_separator_position = position;
       String::WriteToFlat<Char>(string, &buffer[cursor], 0, string_length);
       cursor += string->length();
     }
@@ skipping 23 matching lines @@
   // separator is string to put between elements. Assumed to be non-empty.
   RUNTIME_ASSERT(array_length > 0);
 
   // Find total length of join result.
   int string_length = 0;
   bool is_one_byte = separator->IsOneByteRepresentation();
   bool overflow = false;
   CONVERT_NUMBER_CHECKED(int, elements_length, Int32, elements_array->length());
   RUNTIME_ASSERT(elements_length <= elements_array->elements()->length());
   RUNTIME_ASSERT((elements_length & 1) == 0);  // Even length.
-  FixedArray* elements = FixedArray::cast(elements_array->elements());
+  FixedArray *elements = FixedArray::cast(elements_array->elements());
   {
     DisallowHeapAllocation no_gc;
     for (int i = 0; i < elements_length; i += 2) {
-      String* string = String::cast(elements->get(i + 1));
+      String *string = String::cast(elements->get(i + 1));
       int length = string->length();
       if (is_one_byte && !string->IsOneByteRepresentation()) {
         is_one_byte = false;
       }
       if (length > String::kMaxLength ||
           String::kMaxLength - length < string_length) {
         overflow = true;
         break;
       }
       string_length += length;
@@ skipping 44 matching lines @@
         Vector<uc16>(result->GetChars(), string_length));
     return *result;
   }
 }
 
 
 // Copies Latin1 characters to the given fixed array looking up
 // one-char strings in the cache. Gives up on the first char that is
 // not in the cache and fills the remainder with smi zeros. Returns
 // the length of the successfully copied prefix.
-static int CopyCachedOneByteCharsToArray(Heap* heap, const uint8_t* chars,
-                                         FixedArray* elements, int length) {
+static int CopyCachedOneByteCharsToArray(Heap *heap, const uint8_t *chars,
+                                         FixedArray *elements, int length) {
   DisallowHeapAllocation no_gc;
-  FixedArray* one_byte_cache = heap->single_character_string_cache();
-  Object* undefined = heap->undefined_value();
+  FixedArray *one_byte_cache = heap->single_character_string_cache();
+  Object *undefined = heap->undefined_value();
   int i;
   WriteBarrierMode mode = elements->GetWriteBarrierMode(no_gc);
   for (i = 0; i < length; ++i) {
-    Object* value = one_byte_cache->get(chars[i]);
+    Object *value = one_byte_cache->get(chars[i]);
     if (value == undefined) break;
     elements->set(i, value, mode);
   }
   if (i < length) {
     DCHECK(Smi::FromInt(0) == 0);
     memset(elements->data_start() + i, 0, kPointerSize * (length - i));
   }
 #ifdef DEBUG
   for (int j = 0; j < length; ++j) {
     Object* element = elements->get(j);
@@ skipping 54 matching lines @@
 
 
 static inline bool ToUpperOverflows(uc32 character) {
   // y with umlauts and the micro sign are the only characters that stop
   // fitting into one-byte when converting to uppercase.
   static const uc32 yuml_code = 0xff;
   static const uc32 micro_code = 0xb5;
   return (character == yuml_code || character == micro_code);
 }
 
-
 template <class Converter>
-MUST_USE_RESULT static Object* ConvertCaseHelper(
-    Isolate* isolate, String* string, SeqString* result, int result_length,
-    unibrow::Mapping<Converter, 128>* mapping) {
+MUST_USE_RESULT static Object *ConvertCaseHelper(
+    Isolate *isolate, String *string, SeqString *result, int result_length,
+    unibrow::Mapping<Converter, 128> *mapping) {
   DisallowHeapAllocation no_gc;
   // We try this twice, once with the assumption that the result is no longer
   // than the input and, if that assumption breaks, again with the exact
   // length.  This may not be pretty, but it is nicer than what was here before
   // and I hereby claim my vaffel-is.
   //
   // NOTE: This assumes that the upper/lower case of an ASCII
   // character is also ASCII.  This is currently the case, but it
   // might break in the future if we implement more context and locale
   // dependent upper/lower conversions.
@@ skipping 115 matching lines @@
       DCHECK(dst[i] == src[i] + ('a' - 'A'));
     } else {
       DCHECK('a' <= src[i] && src[i] <= 'z');
       DCHECK(dst[i] == src[i] - ('a' - 'A'));
     }
   }
   return (expected_changed == changed);
 }
 #endif
 
-
 template <class Converter>
-static bool FastAsciiConvert(char* dst, const char* src, int length,
-                             bool* changed_out) {
+static bool FastAsciiConvert(char *dst, const char *src, int length,
+                             bool *changed_out) {
 #ifdef DEBUG
   char* saved_dst = dst;
   const char* saved_src = src;
 #endif
   DisallowHeapAllocation no_gc;
   // We rely on the distance between upper and lower case letters
   // being a known power of 2.
   DCHECK('a' - 'A' == (1 << 5));
   // Boundaries for the range of input characters than require conversion.
   static const char lo = Converter::kIsToLower ? 'A' - 1 : 'a' - 1;
   static const char hi = Converter::kIsToLower ? 'Z' + 1 : 'z' + 1;
   bool changed = false;
   uintptr_t or_acc = 0;
-  const char* const limit = src + length;
+  const char *const limit = src + length;
 
   // dst is newly allocated and always aligned.
   DCHECK(IsAligned(reinterpret_cast<intptr_t>(dst), sizeof(uintptr_t)));
   // Only attempt processing one word at a time if src is also aligned.
   if (IsAligned(reinterpret_cast<intptr_t>(src), sizeof(uintptr_t))) {
     // Process the prefix of the input that requires no conversion one aligned
     // (machine) word at a time.
     while (src <= limit - sizeof(uintptr_t)) {
-      const uintptr_t w = *reinterpret_cast<const uintptr_t*>(src);
+      const uintptr_t w = *reinterpret_cast<const uintptr_t *>(src);
       or_acc |= w;
       if (AsciiRangeMask(w, lo, hi) != 0) {
         changed = true;
         break;
       }
-      *reinterpret_cast<uintptr_t*>(dst) = w;
+      *reinterpret_cast<uintptr_t *>(dst) = w;
       src += sizeof(uintptr_t);
       dst += sizeof(uintptr_t);
     }
     // Process the remainder of the input performing conversion when
     // required one word at a time.
     while (src <= limit - sizeof(uintptr_t)) {
-      const uintptr_t w = *reinterpret_cast<const uintptr_t*>(src);
+      const uintptr_t w = *reinterpret_cast<const uintptr_t *>(src);
       or_acc |= w;
       uintptr_t m = AsciiRangeMask(w, lo, hi);
       // The mask has high (7th) bit set in every byte that needs
       // conversion and we know that the distance between cases is
       // 1 << 5.
-      *reinterpret_cast<uintptr_t*>(dst) = w ^ (m >> 2);
+      *reinterpret_cast<uintptr_t *>(dst) = w ^ (m >> 2);
       src += sizeof(uintptr_t);
       dst += sizeof(uintptr_t);
     }
   }
   // Process the last few bytes of the input (or the whole input if
   // unaligned access is not supported).
   while (src < limit) {
     char c = *src;
     or_acc |= c;
     if (lo < c && c < hi) {
       c ^= (1 << 5);
       changed = true;
     }
     *dst = c;
     ++src;
     ++dst;
   }
 
   if ((or_acc & kAsciiMask) != 0) return false;
 
   DCHECK(CheckFastAsciiConvert(saved_dst, saved_src, length, changed,
                                Converter::kIsToLower));
 
   *changed_out = changed;
   return true;
 }
 
-
 template <class Converter>
-MUST_USE_RESULT static Object* ConvertCase(
-    Handle<String> s, Isolate* isolate,
-    unibrow::Mapping<Converter, 128>* mapping) {
+MUST_USE_RESULT static Object *ConvertCase(

[Yang, 2016/05/12 07:39:21]

You probably did some auto format on your IDE. Please don't do that, at least
not on code unrelated to your change. And also, the formatting rules should fit
with the rest of V8.

[Franzi, 2016/05/13 09:42:03]

Acknowledged.

+    Handle<String> s, Isolate *isolate,
+    unibrow::Mapping<Converter, 128> *mapping) {
   s = String::Flatten(s);
   int length = s->length();
   // Assume that the string is not empty; we need this assumption later
   if (length == 0) return *s;
 
   // Simpler handling of ASCII strings.
   //
   // NOTE: This assumes that the upper/lower case of an ASCII
   // character is also ASCII.  This is currently the case, but it
   // might break in the future if we implement more context and locale
   // dependent upper/lower conversions.
   if (s->IsOneByteRepresentationUnderneath()) {
     // Same length as input.
     Handle<SeqOneByteString> result =
         isolate->factory()->NewRawOneByteString(length).ToHandleChecked();
     DisallowHeapAllocation no_gc;
     String::FlatContent flat_content = s->GetFlatContent();
     DCHECK(flat_content.IsFlat());
     bool has_changed_character = false;
     bool is_ascii = FastAsciiConvert<Converter>(
-        reinterpret_cast<char*>(result->GetChars()),
-        reinterpret_cast<const char*>(flat_content.ToOneByteVector().start()),
+        reinterpret_cast<char *>(result->GetChars()),
+        reinterpret_cast<const char *>(flat_content.ToOneByteVector().start()),
         length, &has_changed_character);
     // If not ASCII, we discard the result and take the 2 byte path.
     if (is_ascii) return has_changed_character ? *result : *s;
   }
 
   Handle<SeqString> result;  // Same length as input.
   if (s->IsOneByteRepresentation()) {
     result = isolate->factory()->NewRawOneByteString(length).ToHandleChecked();
   } else {
     result = isolate->factory()->NewRawTwoByteString(length).ToHandleChecked();
   }
 
-  Object* answer = ConvertCaseHelper(isolate, *s, *result, length, mapping);
+  Object *answer = ConvertCaseHelper(isolate, *s, *result, length, mapping);
   if (answer->IsException() || answer->IsString()) return answer;
 
   DCHECK(answer->IsSmi());
   length = Smi::cast(answer)->value();
   if (s->IsOneByteRepresentation() && length > 0) {
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
         isolate, result, isolate->factory()->NewRawOneByteString(length));
   } else {
     if (length < 0) length = -length;
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
@@ skipping 24 matching lines @@
   DCHECK(args.length() == 3);
 
   CONVERT_ARG_HANDLE_CHECKED(String, string, 0);
   CONVERT_BOOLEAN_ARG_CHECKED(trimLeft, 1);
   CONVERT_BOOLEAN_ARG_CHECKED(trimRight, 2);
 
   string = String::Flatten(string);
   int length = string->length();
 
   int left = 0;
-  UnicodeCache* unicode_cache = isolate->unicode_cache();
+  UnicodeCache *unicode_cache = isolate->unicode_cache();
   if (trimLeft) {
     while (left < length &&
            unicode_cache->IsWhiteSpaceOrLineTerminator(string->Get(left))) {
       left++;
     }
   }
 
   int right = length;
   if (trimRight) {
     while (
@@ skipping 27 matching lines @@
   if (is_one_byte) {
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
         isolate, result, isolate->factory()->NewRawOneByteString(length));
   } else {
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
         isolate, result, isolate->factory()->NewRawTwoByteString(length));
   }
   return *result;
 }
 
+bool unescapePredicateInComponent(uint16_t c) {

[Yang, 2016/05/12 07:39:21]

CamelCase with capital first letter please.

Also put these local functions into an anonymous namespace please.

[Franzi, 2016/05/13 09:42:03]

Done.

+  // do not escape alphanumeric or !'()*-._~
+  if (c < 256 && isalnum(c)) {

[Yang, 2016/05/12 07:39:21]

Let's use IsAlphaNumeric from char-predicates.h. There is also no need to
compare to 256.

[Franzi, 2016/05/13 09:42:03]

Done.

+    return true;
+  }
+  if (c == '!' || c == '\'' || c == '(' || c == ')' || c == '*' || c == '-' ||
+      c == '.' || c == '_' || c == '~') {

[Yang, 2016/05/12 07:39:21]

A switch would be a lot nicer to read.

[Franzi, 2016/05/13 09:42:02]

Done.

+    return true;
+  }
+
+  return false;
+}
+
+bool uriSeparator(uint16_t c) {
+  // separators are #:;/?$&+,@=
+  if (c == '#' || c == ':' || c == ';' || c == '/' || c == '?' || c == '$' ||

[Yang, 2016/05/12 07:39:21]

Same here, a switch would be nice.

[Franzi, 2016/05/13 09:42:02]

Done.

+      c == '&' || c == '+' || c == ',' || c == '@' || c == '=') {
+    return true;
+  }
+  return false;
+}
+
+bool unescape(uint16_t c, bool is_url) {

[Yang, 2016/05/12 07:39:21]

This is only used once. Let's inline it at the call site. I.e.

if (unescapePredicateInComponent(c) || (is_url && uriSeparator(c)))

We also seem to be inconsistent about "is_url" and "is_uri" here.

[Franzi, 2016/05/13 09:42:03]

Done.

+  if (unescapePredicateInComponent(c)) {
+    return true;
+  }
+  return is_url && uriSeparator(c);
+}
+
+void uriAddEncodedOctetToBuffer(uint16_t octet, List<uint16_t> *result) {
+  // Do I need lazy initialization?

[Yang, 2016/05/12 07:39:21]

No. Just declare them as static const. And using a char array (instead of int)
would suffice.

[Franzi, 2016/05/13 09:42:02]

Done.

+  int hexCharCodeArray[16] = {48, 49, 50, 51, 52, 53, 54, 55, 56, 57,  // 0..9

[Yang, 2016/05/12 07:39:21]

We have a HexCharOfValue in bignum.cc. You could move that to utils.h next to
HexValue and use that.

I'm not sure whether that one is faster than a simple table lookup though. If
this one is faster, let's replace the other implementation.

+                              65, 66, 67, 68, 69, 70};                 // A..F
+
+  result->Add(37);  // Char code of '%'.

[Yang, 2016/05/12 07:39:21]

How about result->Add('%')

[Franzi, 2016/05/13 09:42:02]

Done.

+  uint16_t firstHex = octet >> 4;
+  uint16_t secondHex = octet & 0x0F;
+
+  DCHECK(firstHex < 16);
+  DCHECK(secondHex < 16);
+
+  result->Add(hexCharCodeArray[firstHex]);
+  result->Add(hexCharCodeArray[secondHex]);
+}
+
+void uriEncodeOctets(uint16_t *octets, List<uint16_t> *result) {
+  uriAddEncodedOctetToBuffer(octets[0], result);
+  if (octets[1]) uriAddEncodedOctetToBuffer(octets[1], result);
+  if (octets[2]) uriAddEncodedOctetToBuffer(octets[2], result);
+  if (octets[3]) uriAddEncodedOctetToBuffer(octets[3], result);
+}
+
+void uriEncodeSingle(uint16_t cc, List<uint16_t> *result) {
+  // 16 bits total, cut it in 4,6, and 6 bits

[Yang, 2016/05/12 07:39:21]

whitespace after comma.

[Franzi, 2016/05/13 09:42:02]

Done.

+  uint16_t x = (cc >> 12) & 0xF;

[Yang, 2016/05/12 07:39:21]

uint8_t should suffice.

[Franzi, 2016/05/13 09:42:02]

Done.

+  uint16_t y = (cc >> 6) & 63;
+  uint16_t z = cc & 63;               // get last 6 bits
+  uint16_t octets[4] = {0, 0, 0, 0};  // TODO(franzih) 3 is enough here
+  if (cc <= 0x007F) {                 // smaller than 8 bits, i.e., 128
+    octets[0] = cc;                   // ascii same as UTF-8

[Yang, 2016/05/12 07:39:21]

Instead of collecting octets, let's just call uriAddEncodedOctetToBuffer
directly from here.

[Franzi, 2016/05/13 09:42:03]

Done.

+  } else if (cc <= 0x07FF) {
+    octets[0] = y + 192;  // Leading byte: 110xxxxx
+    octets[1] = z + 128;  // Continuation byte: 10xxxxxx
+  } else {
+    octets[0] = x + 224;  // Leading byte: 1110xxxx
+    octets[1] = y + 128;  // Continuation byte: 10xxxxxx
+    octets[2] = z + 128;  // Continuation byte: 10xxxxxx
+  }
+
+  uriEncodeOctets(octets, result);
+}
+
+void uriEncodePair(uint16_t cc1, uint16_t cc2, List<uint16_t> *result) {
+  uint16_t u = ((cc1 >> 6) & 0xF) + 1;
+  uint16_t w = (cc1 >> 2) & 0xF;
+  uint16_t x = cc1 & 3;
+  uint16_t y = (cc2 >> 6) & 0xF;
+  uint16_t z = cc2 & 63;
+  uint16_t octets[4] = {0, 0, 0, 0};
+  octets[0] = (u >> 2) + 240;
+  octets[1] = (((u & 3) << 4) | w) + 128;
+  octets[2] = ((x << 4) | y) + 128;
+  octets[3] = z + 128;
+  uriEncodeOctets(octets, result);
+}
+
+RUNTIME_FUNCTION(Runtime_Encode) {
+  HandleScope scope(isolate);
+  DCHECK(args.length() == 2);
+  CONVERT_ARG_HANDLE_CHECKED(String, uri, 0);
+  CONVERT_BOOLEAN_ARG_CHECKED(is_uri, 1);
+
+  size_t uriLength = uri->length();

[Yang, 2016/05/12 07:39:20]

Let's use int here.

[Franzi, 2016/05/13 09:42:03]

Done.

+  List<uint16_t> buffer;

[Yang, 2016/05/12 07:39:21]

The result can only be ASCII characters, so please use uint8_t here. As a small
optimization, we could pre-allocate the input string length, since the result is
at least that long.

[Franzi, 2016/05/13 09:42:03]

Done.

+
+  for (size_t k = 0; k < uriLength; k++) {

[Yang, 2016/05/12 07:39:21]

And int here.

[Franzi, 2016/05/13 09:42:02]

Done.

+    uint16_t cc1 = uri->Get(static_cast<int>(k));

[Yang, 2016/05/12 07:39:21]

Calling Get is fairly inefficient.  We should flatten the string first and then
use a FlatStringReader. Even better would be using String::GetFlatContent and
call into a templatized helper function depending on the encoding.

[Franzi, 2016/05/13 09:42:02]

Done. Using Get() on FlatContent. 

+
+    if (unescape(cc1, is_uri)) {
+      DCHECK(cc1 <= String::kMaxOneByteCharCode);
+      buffer.Add(cc1);
+    } else {
+      if (cc1 >= 0xDC00 && cc1 <= 0xDFFF) {

[Yang, 2016/05/12 07:39:21]

please do not use magic numbers here. Use unibrow::Utf16::IsTrailSurrogate

[Franzi, 2016/05/13 09:42:02]

Done.

+        THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewMakeURIError());
+      }
+      if (cc1 < 0xD800 || cc1 > 0xDBFF) {

[Yang, 2016/05/12 07:39:21]

!unibrow::Utf16::IsLeadSurrogate

[Franzi, 2016/05/13 09:42:02]

Done.

+        uriEncodeSingle(cc1, &buffer);
+      } else {
+        k++;
+        if (k == uriLength) {
+          THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewMakeURIError());
+        }
+        uint16_t cc2 = uri->Get(static_cast<int>(k));
+        if (cc2 < 0xDC00 || cc2 > 0xDFFF) {

[Yang, 2016/05/12 07:39:21]

Here as well.

[Franzi, 2016/05/13 09:42:03]

Done.

+          THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewMakeURIError());
+        }
+        uriEncodePair(cc1, cc2, &buffer);
+      }
+    }
+  }
+
+  Handle<String> result;
+  int totalLength = buffer.length();
+
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result, isolate->factory()->NewRawOneByteString(totalLength));

[Yang, 2016/05/12 07:39:21]

NewStringFromAscii(buffer.ToConstVector()) would work even better.

[Franzi, 2016/05/13 09:42:03]

Done. I'm using NewStringFromOneByte() because buffer is a List<uint8_t>, so
less casting.

+
+  for (int i = 0; i < totalLength; i++) {
+    uint16_t value = buffer.at(i);
+    result->Set(i, value);
+  }
+  return *result;
+}
+
 RUNTIME_FUNCTION(Runtime_StringLessThan) {
   HandleScope handle_scope(isolate);
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(String, x, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, y, 1);
   switch (String::Compare(x, y)) {
     case ComparisonResult::kLessThan:
       return isolate->heap()->true_value();
     case ComparisonResult::kEqual:
     case ComparisonResult::kGreaterThan:
@@ skipping 94 matching lines @@
   return isolate->heap()->empty_string();
 }
 
 
 RUNTIME_FUNCTION(Runtime_StringCharAt) {
   SealHandleScope shs(isolate);
   DCHECK(args.length() == 2);
   if (!args[0]->IsString()) return Smi::FromInt(0);
   if (!args[1]->IsNumber()) return Smi::FromInt(0);
   if (std::isinf(args.number_at(1))) return isolate->heap()->empty_string();
-  Object* code = __RT_impl_Runtime_StringCharCodeAtRT(args, isolate);
+  Object *code = __RT_impl_Runtime_StringCharCodeAtRT(args, isolate);
   if (code->IsNaN()) return isolate->heap()->empty_string();
   return __RT_impl_Runtime_StringCharFromCode(Arguments(1, &code), isolate);
 }
 
 RUNTIME_FUNCTION(Runtime_ExternalStringGetChar) {
   SealHandleScope shs(isolate);
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_CHECKED(ExternalString, string, 0);
   CONVERT_INT32_ARG_CHECKED(index, 1);
   return Smi::FromInt(string->Get(index));
@@ skipping 43 matching lines @@
   SealHandleScope shs(isolate);
   DCHECK(args.length() == 2);
   if (!args[0]->IsString()) return isolate->heap()->undefined_value();
   if (!args[1]->IsNumber()) return isolate->heap()->undefined_value();
   if (std::isinf(args.number_at(1))) return isolate->heap()->nan_value();
   return __RT_impl_Runtime_StringCharCodeAtRT(args, isolate);
 }
 
 }  // namespace internal
 }  // namespace v8