Avoiding re-externalization of strings.

Bug: chromium:845409
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I75bddcf0e8879d2161486f24d1cd4e46d8fe008d
Reviewed-on: https://chromium-review.googlesource.com/1139056
Commit-Queue: Rodrigo Bruno <rfbpb@google.com>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54599}

include/v8.h

@@ -3048,6 +3048,12 @@ class V8_EXPORT String : public Name {
   void VerifyExternalStringResourceBase(ExternalStringResourceBase* v,
                                         Encoding encoding) const;
   void VerifyExternalStringResource(ExternalStringResource* val) const;
+  ExternalStringResource* GetExternalStringResourceSlow() const;
+  ExternalStringResourceBase* GetExternalStringResourceBaseSlow(
+      String::Encoding* encoding_out) const;
+  const ExternalOneByteStringResource* GetExternalOneByteStringResourceSlow()
+      const;
+
   static void CheckCast(v8::Value* obj);
 };
 
@@ -10182,12 +10188,13 @@ String::ExternalStringResource* String::GetExternalStringResource() const {
   typedef internal::Object O;
   typedef internal::Internals I;
   O* obj = *reinterpret_cast<O* const*>(this);
-  String::ExternalStringResource* result;
+
+  ExternalStringResource* result;
   if (I::IsExternalTwoByteString(I::GetInstanceType(obj))) {
     void* value = I::ReadField<void*>(obj, I::kStringResourceOffset);
     result = reinterpret_cast<String::ExternalStringResource*>(value);
   } else {
-    result = NULL;
+    result = GetExternalStringResourceSlow();
   }
 #ifdef V8_ENABLE_CHECKS
   VerifyExternalStringResource(result);
@@ -10203,11 +10210,13 @@ String::ExternalStringResourceBase* String::GetExternalStringResourceBase(
   O* obj = *reinterpret_cast<O* const*>(this);
   int type = I::GetInstanceType(obj) & I::kFullStringRepresentationMask;
   *encoding_out = static_cast<Encoding>(type & I::kStringEncodingMask);
-  ExternalStringResourceBase* resource = NULL;
+  ExternalStringResourceBase* resource;
   if (type == I::kExternalOneByteRepresentationTag ||
       type == I::kExternalTwoByteRepresentationTag) {
     void* value = I::ReadField<void*>(obj, I::kStringResourceOffset);
     resource = static_cast<ExternalStringResourceBase*>(value);
+  } else {
+    resource = GetExternalStringResourceBaseSlow(encoding_out);
   }
 #ifdef V8_ENABLE_CHECKS
   VerifyExternalStringResourceBase(resource, *encoding_out);

src/api.cc

@@ -5961,15 +5961,74 @@ void v8::String::VerifyExternalStringResourceBase(
   CHECK_EQ(expectedEncoding, encoding);
 }
 
-const v8::String::ExternalOneByteStringResource*
-v8::String::GetExternalOneByteStringResource() const {
+String::ExternalStringResource* String::GetExternalStringResourceSlow() const {
+  typedef internal::Internals I;
+  i::Handle<i::String> str = Utils::OpenHandle(this);
+  ExternalStringResource* result = nullptr;
+  internal::Object* obj;
+
+  if (str->IsThinString()) {
+    obj = i::Handle<i::ThinString>::cast(str)->actual();
+  } else {
+    obj = *str;
+  }
+
+  if (I::IsExternalTwoByteString(I::GetInstanceType(obj))) {
+    void* value = I::ReadField<void*>(obj, I::kStringResourceOffset);
+    result = reinterpret_cast<String::ExternalStringResource*>(value);
+  }
+  return result;
+}
+
+String::ExternalStringResourceBase* String::GetExternalStringResourceBaseSlow(
+    String::Encoding* encoding_out) const {
+  typedef internal::Internals I;
   i::Handle<i::String> str = Utils::OpenHandle(this);
+  ExternalStringResourceBase* resource = nullptr;
+  internal::Object* obj;
+
+  if (str->IsThinString()) {
+    obj = i::Handle<i::ThinString>::cast(str)->actual();
+  } else {
+    obj = *str;
+  }
+
+  int type = I::GetInstanceType(obj) & I::kFullStringRepresentationMask;
+  *encoding_out = static_cast<Encoding>(type & I::kStringEncodingMask);
+
+  if (type == I::kExternalOneByteRepresentationTag ||
+      type == I::kExternalTwoByteRepresentationTag) {
+    void* value = I::ReadField<void*>(obj, I::kStringResourceOffset);
+    resource = static_cast<ExternalStringResourceBase*>(value);
+  }
+  return resource;
+}
+
+const String::ExternalOneByteStringResource*
+String::GetExternalOneByteStringResourceSlow() const {
+  i::Handle<i::String> str = Utils::OpenHandle(this);
+  if (str->IsThinString()) {
+    i::MemoryChunk* chunk = i::MemoryChunk::FromHeapObject(*str);
+    str = i::Handle<i::String>(i::Handle<i::ThinString>::cast(str)->actual(),
+                               chunk->heap()->isolate());
+  }
   if (i::StringShape(*str).IsExternalOneByte()) {
     const void* resource =
         i::Handle<i::ExternalOneByteString>::cast(str)->resource();
     return reinterpret_cast<const ExternalOneByteStringResource*>(resource);
-  } else {
+  }
   return nullptr;
+}
+
+const v8::String::ExternalOneByteStringResource*
+v8::String::GetExternalOneByteStringResource() const {
+  i::Handle<i::String> str = Utils::OpenHandle(this);
+  if (i::StringShape(*str).IsExternalOneByte()) {
+    const void* value =
+        i::Handle<i::ExternalOneByteString>::cast(str)->resource();
+    return reinterpret_cast<const ExternalOneByteStringResource*>(value);
+  } else {
+    return GetExternalOneByteStringResourceSlow();
   }
 }
 
@@ -6864,6 +6923,10 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) {
     return false;
   }
 
+  if (obj->IsThinString()) {
+    obj = i::Handle<i::String>(i::ThinString::cast(*obj)->actual(), isolate);
+  }
+
   if (i::StringShape(*obj).IsExternal()) {
     return false;  // Already an external string.
   }
@@ -6875,7 +6938,7 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) {
 
   bool result = obj->MakeExternal(resource);
   // Assert that if CanMakeExternal(), then externalizing actually succeeds.
-  DCHECK(!CanMakeExternal() || result);
+  DCHECK(!Utils::ToLocal(obj)->CanMakeExternal() || result);
   if (result) {
     DCHECK(obj->IsExternalString());
   }
@@ -6893,6 +6956,10 @@ bool v8::String::MakeExternal(
     return false;
   }
 
+  if (obj->IsThinString()) {
+    obj = i::Handle<i::String>(i::ThinString::cast(*obj)->actual(), isolate);
+  }
+
   if (i::StringShape(*obj).IsExternal()) {
     return false;  // Already an external string.
   }
@@ -6904,7 +6971,7 @@ bool v8::String::MakeExternal(
 
   bool result = obj->MakeExternal(resource);
   // Assert that if CanMakeExternal(), then externalizing actually succeeds.
-  DCHECK(!CanMakeExternal() || result);
+  DCHECK(!Utils::ToLocal(obj)->CanMakeExternal() || result);
   if (result) {
     DCHECK(obj->IsExternalString());
   }

src/heap/heap-inl.h

@@ -530,6 +530,8 @@ Isolate* Heap::isolate() {
 
 void Heap::ExternalStringTable::AddString(String* string) {
   DCHECK(string->IsExternalString());
+  DCHECK(!Contains(string));
+
   if (InNewSpace(string)) {
     new_space_strings_.push_back(string);
   } else {

src/heap/heap.cc

@@ -2296,6 +2296,16 @@ void Heap::ProtectUnprotectedMemoryChunks() {
   unprotected_memory_chunks_.clear();
 }
 
+bool Heap::ExternalStringTable::Contains(HeapObject* obj) {
+  for (size_t i = 0; i < new_space_strings_.size(); ++i) {
+    if (new_space_strings_[i] == obj) return true;
+  }
+  for (size_t i = 0; i < old_space_strings_.size(); ++i) {
+    if (old_space_strings_[i] == obj) return true;
+  }
+  return false;
+}
+
 String* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(Heap* heap,
                                                                 Object** p) {
   MapWord first_word = HeapObject::cast(*p)->map_word();

src/heap/heap.h

@@ -1481,6 +1481,7 @@ class Heap {
 
     // Registers an external string.
     inline void AddString(String* string);
+    bool Contains(HeapObject* obj);
 
     void IterateAll(RootVisitor* v);
     void IterateNewSpaceStrings(RootVisitor* v);