[string] Don't MakeThin on shared strings.

This is a temporary solution so prototyping of shared structs and shared
strings can be worked on in parallel.

Bug: v8:12007
Change-Id: Ic849ec66da1d3824d50d695f16e4b77380afa015
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3379222Reviewed-by: Patrick Thier <pthier@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78570}

src/objects/string-inl.h

@@ -707,7 +707,10 @@ String::FlatContent String::GetFlatContent(
   {
     Isolate* isolate;
     // We don't have to check read only strings as those won't move.
-    DCHECK_IMPLIES(GetIsolateFromHeapObject(*this, &isolate),
+    //
+    // TODO(v8:12007): Currently character data is never overwritten for
+    // shared strings.
+    DCHECK_IMPLIES(GetIsolateFromHeapObject(*this, &isolate) && !IsShared(),
                    ThreadId::Current() == isolate->thread_id());
   }
 #endif

src/objects/string.cc

@@ -308,11 +308,12 @@ void String::MakeThin(IsolateT* isolate, String internalized) {
   Map initial_map = this->map(kAcquireLoad);
   StringShape initial_shape(initial_map);
 
-  // Another thread may have already migrated the string.
-  if (initial_shape.IsThin()) {
-    DCHECK(initial_shape.IsShared());
-    return;
-  }
+  // TODO(v8:12007): Support shared ThinStrings.
+  //
+  // Currently in-place migrations to ThinStrings are disabled for shared
+  // strings to unblock prototyping.
+  if (initial_shape.IsShared()) return;
+  DCHECK(!initial_shape.IsThin());
 
   bool has_pointers = initial_shape.IsIndirect();
   int old_size = this->SizeFromMap(initial_map);
@@ -336,7 +337,9 @@ void String::MakeThin(IsolateT* isolate, String internalized) {
       break;
     case StringMigrationResult::kAnotherThreadMigrated:
       // Nothing to do.
-      return;
+      //
+      // TODO(v8:12007): Support shared ThinStrings.
+      UNREACHABLE();
   }
 
   ThinString thin = ThinString::cast(*this);

test/cctest/test-shared-strings.cc

@@ -264,21 +264,22 @@ class ConcurrentInternalizationThread final : public v8::base::Thread {
 
     HandleScope scope(i_isolate);
 
-    Handle<String> manual_thin_actual =
-        factory->InternalizeString(factory->NewStringFromAsciiChecked("TODO"));
-
     for (int i = 0; i < shared_strings_->length(); i++) {
       Handle<String> input_string(String::cast(shared_strings_->get(i)),
                                   i_isolate);
       CHECK(input_string->IsShared());
+      Handle<String> interned = factory->InternalizeString(input_string);
+      CHECK(interned->IsShared());
       if (hit_or_miss_ == kTestMiss) {
-        Handle<String> interned = factory->InternalizeString(input_string);
         CHECK_EQ(*input_string, *interned);
       } else {
-        // TODO(v8:12007): Make this branch also test InternalizeString. But
-        // LookupString needs to be made threadsafe first and restart-aware.
-        input_string->MakeThin(i_isolate, *manual_thin_actual);
-        CHECK(input_string->IsThinString());
+        // TODO(v8:12007): In-place internalization currently do not migrate
+        // shared strings to ThinStrings. This is triviailly threadsafe for
+        // character access but bad for performance, as run-time
+        // internalizations do not speed up comparisons for shared strings.
+        CHECK(!input_string->IsThinString());
+        CHECK_NE(*input_string, *interned);
+        CHECK(String::Equals(i_isolate, input_string, interned));
       }
     }