Revert "[objects] Cache the ExternalString's data in its resource"

This reverts commit ed225df7.

Reason for revert: Blocks the roll, causing compile failures in Chromium:
https://ci.chromium.org/p/chromium/builders/try/win_chromium_compile_dbg_ng/800868?

Original change's description:
> [objects] Cache the ExternalString's data in its resource
>
> For external uncached strings (also called "Small External Strings")
> with cacheable resources, we can cache its resource's data at the
> string's creation time. This allows us to safely read the data from the
> background as we wouldn't trigger a data() callback.
>
> For more information regarding the investigation and possible proposals
> see
> https://docs.google.com/document/d/101eAQqFpBPWFGNJicxtdlwYShJkTOUsEuxkVVeu5Hrk/edit?usp=sharing
>
> Bug: v8:7790, v8:11463
> Change-Id: I6164092b01a6ccb525a9516f476e066b35fb1f96
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2685177
> Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#72862}

Bug: v8:7790
Bug: v8:11463
Change-Id: I1d14c2f9872d156d43d5d95c8a032a37ba9379cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2708824
Auto-Submit: Bill Budge <bbudge@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72880}

include/v8.h

@@ -3323,8 +3323,7 @@ class V8_EXPORT String : public Name {
     ~ExternalStringResource() override = default;
 
     /**
-     * The string data from the underlying buffer. If the resource is cacheable
-     * then data() must return the same value for all invocations.
+     * The string data from the underlying buffer.
      */
     virtual const uint16_t* data() const = 0;
 
@@ -3333,31 +3332,8 @@ class V8_EXPORT String : public Name {
      */
     virtual size_t length() const = 0;
 
-    /**
-     * Returns the cached data from the underlying buffer. This method can be
-     * called only for cacheable resources (i.e. IsCacheable() == true) and only
-     * after UpdateDataCache() was called.
-     */
-    const uint16_t* cached_data() const {
-#if DEBUG
-      CheckCachedDataInvariants();
-#endif
-      return cached_data_;
-    }
-
-    /**
-     * Update {cached_data_} with the data from the underlying buffer. This can
-     * be called only for cacheable resources.
-     */
-    void UpdateDataCache();
-
    protected:
     ExternalStringResource() = default;
-
-   private:
-    void CheckCachedDataInvariants() const;
-
-    const uint16_t* cached_data_ = nullptr;
   };
 
   /**
@@ -3378,41 +3354,12 @@ class V8_EXPORT String : public Name {
      * buffer.
      */
     ~ExternalOneByteStringResource() override = default;
-
-    /**
-     * The string data from the underlying buffer. If the resource is cacheable
-     * then data() must return the same value for all invocations.
-     */
+    /** The string data from the underlying buffer.*/
     virtual const char* data() const = 0;
-
     /** The number of Latin-1 characters in the string.*/
     virtual size_t length() const = 0;
-
-    /**
-     * Returns the cached data from the underlying buffer. If the resource is
-     * uncacheable or if UpdateDataCache() was not called before, it has
-     * undefined behaviour.
-     */
-    const char* cached_data() const {
-#if DEBUG
-      CheckCachedDataInvariants();
-#endif
-      return cached_data_;
-    }
-
-    /**
-     * Update {cached_data_} with the data from the underlying buffer. This can
-     * be called only for cacheable resources.
-     */
-    void UpdateDataCache();
-
    protected:
     ExternalOneByteStringResource() = default;
-
-   private:
-    void CheckCachedDataInvariants() const;
-
-    const char* cached_data_ = nullptr;
   };
 
   /**

src/api/api.cc

@@ -5453,24 +5453,6 @@ String::ExternalStringResource* String::GetExternalStringResourceSlow() const {
   return nullptr;
 }
 
-void String::ExternalStringResource::UpdateDataCache() {
-  DCHECK(IsCacheable());
-  cached_data_ = data();
-}
-
-void String::ExternalStringResource::CheckCachedDataInvariants() const {
-  DCHECK(IsCacheable() && cached_data_ != nullptr);
-}
-
-void String::ExternalOneByteStringResource::UpdateDataCache() {
-  DCHECK(IsCacheable());
-  cached_data_ = data();
-}
-
-void String::ExternalOneByteStringResource::CheckCachedDataInvariants() const {
-  DCHECK(IsCacheable() && cached_data_ != nullptr);
-}
-
 String::ExternalStringResourceBase* String::GetExternalStringResourceBaseSlow(
     String::Encoding* encoding_out) const {
   i::DisallowGarbageCollection no_gc;

src/objects/string-inl.h

@@ -864,23 +864,15 @@ void ExternalString::DisposeResource(Isolate* isolate) {
 
 DEF_GETTER(ExternalOneByteString, resource,
            const ExternalOneByteString::Resource*) {
-  return mutable_resource();
-}
-
-DEF_GETTER(ExternalOneByteString, mutable_resource,
-           ExternalOneByteString::Resource*) {
   return reinterpret_cast<Resource*>(resource_as_address(isolate));
 }
 
 void ExternalOneByteString::update_data_cache(Isolate* isolate) {
+  if (is_uncached()) return;
   DisallowGarbageCollection no_gc;
-  if (is_uncached()) {
-    if (resource()->IsCacheable()) mutable_resource()->UpdateDataCache();
-  } else {
-    WriteExternalPointerField(kResourceDataOffset, isolate,
-                              reinterpret_cast<Address>(resource()->data()),
-                              kExternalStringResourceDataTag);
-  }
+  WriteExternalPointerField(kResourceDataOffset, isolate,
+                            reinterpret_cast<Address>(resource()->data()),
+                            kExternalStringResourceDataTag);
 }
 
 void ExternalOneByteString::SetResource(
@@ -902,23 +894,6 @@ void ExternalOneByteString::set_resource(
 
 const uint8_t* ExternalOneByteString::GetChars() {
   DisallowGarbageCollection no_gc;
-  if (is_uncached()) {
-    if (resource()->IsCacheable()) {
-      // TODO(solanes): Teach TurboFan/CSA to not bailout to the runtime to
-      // avoid this call.
-      return reinterpret_cast<const uint8_t*>(resource()->cached_data());
-    }
-#if DEBUG
-    // Check that this method is called only from the main thread if we have an
-    // uncached string with an uncacheable resource.
-    {
-      Isolate* isolate;
-      DCHECK_IMPLIES(GetIsolateFromHeapObject(*this, &isolate),
-                     ThreadId::Current() == isolate->thread_id());
-    }
-#endif
-  }
-
   return reinterpret_cast<const uint8_t*>(resource()->data());
 }
 
@@ -929,23 +904,15 @@ uint8_t ExternalOneByteString::Get(int index) {
 
 DEF_GETTER(ExternalTwoByteString, resource,
            const ExternalTwoByteString::Resource*) {
-  return mutable_resource();
-}
-
-DEF_GETTER(ExternalTwoByteString, mutable_resource,
-           ExternalTwoByteString::Resource*) {
   return reinterpret_cast<Resource*>(resource_as_address(isolate));
 }
 
 void ExternalTwoByteString::update_data_cache(Isolate* isolate) {
+  if (is_uncached()) return;
   DisallowGarbageCollection no_gc;
-  if (is_uncached()) {
-    if (resource()->IsCacheable()) mutable_resource()->UpdateDataCache();
-  } else {
-    WriteExternalPointerField(kResourceDataOffset, isolate,
-                              reinterpret_cast<Address>(resource()->data()),
-                              kExternalStringResourceDataTag);
-  }
+  WriteExternalPointerField(kResourceDataOffset, isolate,
+                            reinterpret_cast<Address>(resource()->data()),
+                            kExternalStringResourceDataTag);
 }
 
 void ExternalTwoByteString::SetResource(
@@ -967,23 +934,6 @@ void ExternalTwoByteString::set_resource(
 
 const uint16_t* ExternalTwoByteString::GetChars() {
   DisallowGarbageCollection no_gc;
-  if (is_uncached()) {
-    if (resource()->IsCacheable()) {
-      // TODO(solanes): Teach TurboFan/CSA to not bailout to the runtime to
-      // avoid this call.
-      return resource()->cached_data();
-    }
-#if DEBUG
-    // Check that this method is called only from the main thread if we have an
-    // uncached string with an uncacheable resource.
-    {
-      Isolate* isolate;
-      DCHECK_IMPLIES(GetIsolateFromHeapObject(*this, &isolate),
-                     ThreadId::Current() == isolate->thread_id());
-    }
-#endif
-  }
-
   return resource()->data();
 }
 

src/objects/string.h

@@ -864,10 +864,6 @@ class ExternalOneByteString : public ExternalString {
   STATIC_ASSERT(kSize == kSizeOfAllExternalStrings);
 
   OBJECT_CONSTRUCTORS(ExternalOneByteString, ExternalString);
-
- private:
-  // The underlying resource as a non-const pointer.
-  DECL_GETTER(mutable_resource, Resource*)
 };
 
 // The ExternalTwoByteString class is an external string backed by a UTF-16
@@ -913,10 +909,6 @@ class ExternalTwoByteString : public ExternalString {
   STATIC_ASSERT(kSize == kSizeOfAllExternalStrings);
 
   OBJECT_CONSTRUCTORS(ExternalTwoByteString, ExternalString);
-
- private:
-  // The underlying resource as a non-const pointer.
-  DECL_GETTER(mutable_resource, Resource*)
 };
 
 // A flat string reader provides random access to the contents of a