[heap] Fix ThinString promotion counters.

From the promotion code point of view, a ThinString being forwarded to
an ExternalString is exactly the same as an actual ExternalString being
forwarded to its promoted copy. These changes provide a way to disambiguate
both scenarios since they are different for external memory accounting.

Bug: chromium:867902
Change-Id: I6fd56ee5e0f8900318466108273ab26e936eb439
Reviewed-on: https://chromium-review.googlesource.com/1152975
Commit-Queue: Rodrigo Bruno <rfbpb@google.com>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54790}

src/heap/heap.cc

@@ -2286,7 +2286,6 @@ bool Heap::ExternalStringTable::Contains(HeapObject* obj) {
   return false;
 }
 
-
 void Heap::ProcessMovedExternalString(Page* old_page, Page* new_page,
                                       ExternalString* string) {
   size_t size = string->ExternalPayloadSize();
@@ -2314,7 +2313,12 @@ String* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(Heap* heap,
 
   // String is still reachable.
   String* new_string = String::cast(first_word.ToForwardingAddress());
-  if (new_string->IsThinString()) {
+  String* original_string = reinterpret_cast<String*>(*p);
+  // The length of the original string is used to disambiguate the scenario
+  // of a ThingString being forwarded to an ExternalString (which already exists
+  // in the OLD space), and an ExternalString being forwarded to its promoted
+  // copy. See Scavenger::EvacuateThinString.
+  if (new_string->IsThinString() || original_string->length() == 0) {
     // Filtering Thin strings out of the external string table.
     return nullptr;
   } else if (new_string->IsExternalString()) {

src/heap/scavenger-inl.h

@@ -148,7 +148,8 @@ void Scavenger::EvacuateThinString(Map* map, HeapObject** slot,
                                    ThinString* object, int object_size) {
   if (!is_incremental_marking_) {
     // Loading actual is fine in a parallel setting is there is no write.
-    HeapObject* actual = object->actual();
+    String* actual = object->actual();
+    object->set_length(0);
     *slot = actual;
     // ThinStrings always refer to internalized strings, which are
     // always in old space.

test/cctest/heap/test-external-string-tracker.cc

@@ -170,6 +170,55 @@ TEST(ExternalString_ExternalBackingStoreSizeIncreasesAfterExternalization) {
                   old_backing_store_before);
 }
 
+TEST(ExternalString_PromotedThinString) {
+  ManualGCScope manual_gc_scope;
+  CcTest::InitializeVM();
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  i::Isolate* i_isolate = CcTest::i_isolate();
+  i::Factory* factory = i_isolate->factory();
+  Heap* heap = i_isolate->heap();
+
+  {
+    v8::HandleScope handle_scope(isolate);
+
+    // New external string in the old space.
+    v8::internal::Handle<v8::internal::String> string1 =
+        factory
+            ->NewExternalStringFromOneByte(
+                new TestOneByteResource(i::StrDup(TEST_STR)))
+            .ToHandleChecked();
+
+    // Internalize external string.
+    i::Handle<i::String> isymbol1 = factory->InternalizeString(string1);
+    CHECK(isymbol1->IsInternalizedString());
+    CHECK(string1->IsExternalString());
+    CHECK(!heap->InNewSpace(*isymbol1));
+
+    // New external string in the young space. This string has the same content
+    // as the previous one (that was already internalized).
+    v8::Local<v8::String> string2 =
+        v8::String::NewFromUtf8(isolate, TEST_STR, v8::NewStringType::kNormal)
+            .ToLocalChecked();
+    bool success =
+        string2->MakeExternal(new TestOneByteResource(i::StrDup(TEST_STR)));
+    CHECK(success);
+
+    // Internalize (it will create a thin string in the new space).
+    i::Handle<i::String> istring = v8::Utils::OpenHandle(*string2);
+    i::Handle<i::String> isymbol2 = factory->InternalizeString(istring);
+    CHECK(isymbol2->IsInternalizedString());
+    CHECK(istring->IsThinString());
+    CHECK(heap->InNewSpace(*istring));
+
+    // Collect thin string. References to the thin string will be updated to
+    // point to the actual external string in the old space.
+    heap::GcAndSweep(heap, NEW_SPACE);
+
+    USE(isymbol1);
+    USE(isymbol2);
+  }
+}
 }  // namespace heap
 }  // namespace internal
 }  // namespace v8