[inspector] Fix and refactor String16

This CL defines move semantics for String16, and fixes issues with the
hash code not being set correctly on swap or copy.
It also extends the interface by a few handy templates.
All this functionality will be used for the wasm translations, where
String16s are often concatenated and used as keys in hash tables.

BUG=chromium:659715
R=yangguo@chromium.org, kozyatinskiy@chromium.org

Review-Url: https://codereview.chromium.org/2493723003
Cr-Commit-Position: refs/heads/master@{#41007}

src/inspector/string-16.cc

@@ -447,6 +447,26 @@ void String16Builder::append(const char* characters, size_t length) {
   m_buffer.insert(m_buffer.end(), characters, characters + length);
 }
 
+void String16Builder::appendNumber(int number) {
+  const int kBufferSize = 11;
+  char buffer[kBufferSize];
+  int chars = v8::base::OS::SNPrintF(buffer, kBufferSize, "%d", number);
+  DCHECK_GT(kBufferSize, chars);
+  m_buffer.insert(m_buffer.end(), buffer, buffer + chars);
+}
+
+void String16Builder::appendNumber(size_t number) {
+  const int kBufferSize = 20;
+  char buffer[kBufferSize];
+#if !defined(_WIN32) && !defined(_WIN64)
+  int chars = v8::base::OS::SNPrintF(buffer, kBufferSize, "%zu", number);
+#else
+  int chars = v8::base::OS::SNPrintF(buffer, kBufferSize, "%Iu", number);
+#endif
+  DCHECK_GT(kBufferSize, chars);
+  m_buffer.insert(m_buffer.end(), buffer, buffer + chars);
+}
+
 String16 String16Builder::toString() {
   return String16(m_buffer.data(), m_buffer.size());
 }

src/inspector/string-16.h

@@ -21,7 +21,10 @@ class String16 {
   static const size_t kNotFound = static_cast<size_t>(-1);
 
   String16() {}
-  String16(const String16& other) : m_impl(other.m_impl) {}
+  String16(const String16& other)
+      : m_impl(other.m_impl), hash_code(other.hash_code) {}
+  String16(const String16&& other)
+      : m_impl(std::move(other.m_impl)), hash_code(other.hash_code) {}
   String16(const UChar* characters, size_t size) : m_impl(characters, size) {}
   String16(const UChar* characters)  // NOLINT(runtime/explicit)
       : m_impl(characters) {}
@@ -31,6 +34,18 @@ class String16 {
     m_impl.resize(size);
     for (size_t i = 0; i < size; ++i) m_impl[i] = characters[i];
   }
+  explicit String16(const std::basic_string<UChar>& impl) : m_impl(impl) {}
+
+  String16& operator=(const String16& other) {
+    m_impl = other.m_impl;
+    hash_code = other.hash_code;
+    return *this;
+  }
+  String16& operator=(String16&& other) {
+    m_impl = std::move(other.m_impl);
+    hash_code = other.hash_code;
+    return *this;
+  }
 
   static String16 fromInteger(int);
   static String16 fromInteger(size_t);
@@ -52,51 +67,53 @@ class String16 {
   size_t reverseFind(const String16& str, size_t start = UINT_MAX) const {
     return m_impl.rfind(str.m_impl, start);
   }
-  void swap(String16& other) { m_impl.swap(other.m_impl); }
+  size_t find(UChar c, size_t start = 0) const { return m_impl.find(c, start); }
+  size_t reverseFind(UChar c, size_t start = UINT_MAX) const {
+    return m_impl.rfind(c, start);
+  }
+  void swap(String16& other) {
+    m_impl.swap(other.m_impl);
+    std::swap(hash_code, other.hash_code);
+  }
 
   // Convenience methods.
   std::string utf8() const;
   static String16 fromUTF8(const char* stringStart, size_t length);
 
-  const std::basic_string<UChar>& impl() const { return m_impl; }
-  explicit String16(const std::basic_string<UChar>& impl) : m_impl(impl) {}
-
   std::size_t hash() const {
-    if (!has_hash) {
-      size_t hash = 0;
-      for (size_t i = 0; i < length(); ++i) hash = 31 * hash + m_impl[i];
-      hash_code = hash;
-      has_hash = true;
+    if (!hash_code) {
+      for (char c : m_impl) hash_code = 31 * hash_code + c;
+      // Map hash code 0 to 1. This double the number of hash collisions for 1,
+      // but avoids recomputing the hash code.
+      if (!hash_code) ++hash_code;
     }
     return hash_code;
   }
 
+  inline bool operator==(const String16& other) const {
+    return m_impl == other.m_impl;
+  }
+  inline bool operator<(const String16& other) const {
+    return m_impl < other.m_impl;
+  }
+  inline bool operator!=(const String16& other) const {
+    return m_impl != other.m_impl;
+  }
+  inline String16 operator+(const String16& other) const {
+    return String16(m_impl + other.m_impl);
+  }
+
+  // Defined later, since it uses the String16Builder.
+  template <typename... T>
+  static String16 concat(T... args);
+
  private:
   std::basic_string<UChar> m_impl;
-  mutable bool has_hash = false;
   mutable std::size_t hash_code = 0;
 };
 
-inline bool operator==(const String16& a, const String16& b) {
-  return a.impl() == b.impl();
-}
-inline bool operator<(const String16& a, const String16& b) {
-  return a.impl() < b.impl();
-}
-inline bool operator!=(const String16& a, const String16& b) {
-  return a.impl() != b.impl();
-}
-inline bool operator==(const String16& a, const char* b) {
-  return a.impl() == String16(b).impl();
-}
-inline String16 operator+(const String16& a, const char* b) {
-  return String16(a.impl() + String16(b).impl());
-}
 inline String16 operator+(const char* a, const String16& b) {
-  return String16(String16(a).impl() + b.impl());
-}
-inline String16 operator+(const String16& a, const String16& b) {
-  return String16(a.impl() + b.impl());
+  return String16(a) + b;
 }
 
 class String16Builder {
@@ -107,13 +124,29 @@ class String16Builder {
   void append(char);
   void append(const UChar*, size_t);
   void append(const char*, size_t);
+  void appendNumber(int);
+  void appendNumber(size_t);
   String16 toString();
   void reserveCapacity(size_t);
 
+  template <typename T, typename... R>
+  void appendAll(T first, R... rest) {
+    append(first);
+    appendAll(rest...);
+  }
+  void appendAll() {}
+
  private:
   std::vector<UChar> m_buffer;
 };
 
+template <typename... T>
+String16 String16::concat(T... args) {
+  String16Builder builder;
+  builder.appendAll(args...);
+  return builder.toString();
+}
+
 }  // namespace v8_inspector
 
 #if !defined(__APPLE__) || defined(_LIBCPP_VERSION)

src/inspector/v8-debugger-agent-impl.cc

@@ -76,8 +76,14 @@ static String16 breakpointIdSuffix(
 static String16 generateBreakpointId(
     const String16& scriptId, int lineNumber, int columnNumber,
     V8DebuggerAgentImpl::BreakpointSource source) {
-  return scriptId + ":" + String16::fromInteger(lineNumber) + ":" +
-         String16::fromInteger(columnNumber) + breakpointIdSuffix(source);
+  String16Builder builder;
+  builder.append(scriptId);
+  builder.append(':');
+  builder.appendNumber(lineNumber);
+  builder.append(':');
+  builder.appendNumber(columnNumber);
+  builder.append(breakpointIdSuffix(source));
+  return builder.toString();
 }
 
 static bool positionComparator(const std::pair<int, int>& a,