Remove refcount thrash in trusted types.

This is required for removing the refcount thrashes in
Element::setAttribute() and ultimately avoids nearly 13% of regression
in blink_perf.bindings set-attribute.html story when StringImpl is made
threadsafe.

Bug: 1083392
Change-Id: I904396d7913ad18ba0f097afd087aa796dd530ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2351095
Auto-Submit: Albert J. Wong <ajwong@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797239}

third_party/blink/renderer/core/dom/element-hot.cc

@@ -138,13 +138,13 @@ void Element::setAttribute(const QualifiedName& name,
                          ? GetElementData()->Attributes().FindIndex(name)
                          : kNotFound;
 
-  String trusted_value =
+  AtomicString trusted_value(
       TrustedTypesCheckFor(ExpectedTrustedTypeForAttribute(name), value,
-                           GetExecutionContext(), exception_state);
+                           GetExecutionContext(), exception_state));
   if (exception_state.HadException())
     return;
 
-  SetAttributeInternal(index, name, AtomicString(trusted_value),
+  SetAttributeInternal(index, name, trusted_value,
                        kNotInSynchronizationOfLazyAttribute);
 }
 
@@ -172,13 +172,13 @@ void Element::SetAttributeHinted(const AtomicString& local_name,
   QualifiedName q_name = QualifiedName::Null();
   std::tie(index, q_name) = LookupAttributeQNameHinted(local_name, hint);
 
-  String trusted_value =
-      TrustedTypesCheckFor(ExpectedTrustedTypeForAttribute(q_name), value,
-                           GetExecutionContext(), exception_state);
+  AtomicString trusted_value(TrustedTypesCheckFor(
+      ExpectedTrustedTypeForAttribute(q_name), std::move(value),
+      GetExecutionContext(), exception_state));
   if (exception_state.HadException())
     return;
 
-  SetAttributeInternal(index, q_name, AtomicString(trusted_value),
+  SetAttributeInternal(index, q_name, trusted_value,
                        kNotInSynchronizationOfLazyAttribute);
 }
 
@@ -199,12 +199,12 @@ void Element::SetAttributeHinted(
   wtf_size_t index;
   QualifiedName q_name = QualifiedName::Null();
   std::tie(index, q_name) = LookupAttributeQNameHinted(local_name, hint);
-  String value = TrustedTypesCheckFor(ExpectedTrustedTypeForAttribute(q_name),
-                                      string_or_trusted, GetExecutionContext(),
-                                      exception_state);
+  AtomicString value(TrustedTypesCheckFor(
+      ExpectedTrustedTypeForAttribute(q_name), string_or_trusted,
+      GetExecutionContext(), exception_state));
   if (exception_state.HadException())
     return;
-  SetAttributeInternal(index, q_name, AtomicString(value),
+  SetAttributeInternal(index, q_name, value,
                        kNotInSynchronizationOfLazyAttribute);
 }
 
@@ -270,9 +270,9 @@ Attr* Element::setAttributeNode(Attr* attr_node,
   SynchronizeAllAttributes();
   const UniqueElementData& element_data = EnsureUniqueElementData();
 
-  String value = TrustedTypesCheckFor(
+  AtomicString value(TrustedTypesCheckFor(
       ExpectedTrustedTypeForAttribute(attr_node->GetQualifiedName()),
-      attr_node->value(), GetExecutionContext(), exception_state);
+      attr_node->value(), GetExecutionContext(), exception_state));
   if (exception_state.HadException())
     return nullptr;
 
@@ -301,8 +301,7 @@ Attr* Element::setAttributeNode(Attr* attr_node,
     }
   }
 
-  SetAttributeInternal(index, attr_node->GetQualifiedName(),
-                       AtomicString(value),
+  SetAttributeInternal(index, attr_node->GetQualifiedName(), value,
                        kNotInSynchronizationOfLazyAttribute);
 
   attr_node->AttachToElement(this, local_name);

third_party/blink/renderer/core/dom/element.cc

@@ -3788,13 +3788,13 @@ void Element::setAttributeNS(
                           exception_state))
     return;
 
-  String value = TrustedTypesCheckFor(
+  AtomicString value(TrustedTypesCheckFor(
       ExpectedTrustedTypeForAttribute(parsed_name), string_or_trusted,
-      GetExecutionContext(), exception_state);
+      GetExecutionContext(), exception_state));
   if (exception_state.HadException())
     return;
 
-  setAttribute(parsed_name, AtomicString(value));
+  setAttribute(parsed_name, value);
 }
 
 void Element::RemoveAttributeInternal(

third_party/blink/renderer/core/trustedtypes/trusted_html.cc

@@ -6,9 +6,9 @@
 
 namespace blink {
 
-TrustedHTML::TrustedHTML(const String& html) : html_(html) {}
+TrustedHTML::TrustedHTML(String html) : html_(std::move(html)) {}
 
-String TrustedHTML::toString() const {
+const String& TrustedHTML::toString() const {
   return html_;
 }
 

third_party/blink/renderer/core/trustedtypes/trusted_html.h

@@ -15,10 +15,10 @@ class CORE_EXPORT TrustedHTML final : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  explicit TrustedHTML(const String& html);
+  explicit TrustedHTML(String html);
 
   // TrustedHTML.idl
-  String toString() const;
+  const String& toString() const;
 
  private:
   const String html_;

third_party/blink/renderer/core/trustedtypes/trusted_script.cc

@@ -6,9 +6,9 @@
 
 namespace blink {
 
-TrustedScript::TrustedScript(const String& script) : script_(script) {}
+TrustedScript::TrustedScript(String script) : script_(std::move(script)) {}
 
-String TrustedScript::toString() const {
+const String& TrustedScript::toString() const {
   return script_;
 }
 

third_party/blink/renderer/core/trustedtypes/trusted_script.h

@@ -15,10 +15,10 @@ class CORE_EXPORT TrustedScript final : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  explicit TrustedScript(const String& script);
+  explicit TrustedScript(String script);
 
   // TrustedScript.idl
-  String toString() const;
+  const String& toString() const;
 
  private:
   const String script_;

third_party/blink/renderer/core/trustedtypes/trusted_script_url.cc

@@ -6,9 +6,9 @@
 
 namespace blink {
 
-TrustedScriptURL::TrustedScriptURL(const String& url) : url_(url) {}
+TrustedScriptURL::TrustedScriptURL(String url) : url_(std::move(url)) {}
 
-String TrustedScriptURL::toString() const {
+const String& TrustedScriptURL::toString() const {
   return url_;
 }
 

third_party/blink/renderer/core/trustedtypes/trusted_script_url.h

@@ -15,10 +15,10 @@ class CORE_EXPORT TrustedScriptURL final : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  explicit TrustedScriptURL(const String& url);
+  explicit TrustedScriptURL(String url);
 
   // TrustedScriptURL.idl
-  String toString() const;
+  const String& toString() const;
 
  private:
   const String url_;

third_party/blink/renderer/core/trustedtypes/trusted_types_util.cc

@@ -206,7 +206,7 @@ TrustedTypePolicy* GetDefaultPolicy(const ExecutionContext* execution_context) {
 // and has a number of additional parameters to enable proper error reporting
 // for each case.
 String GetStringFromScriptHelper(
-    const String& script,
+    String script,
     ExecutionContext* context,
 
     // Parameters to customize error messages:
@@ -277,7 +277,7 @@ bool RequireTrustedTypesCheck(const ExecutionContext* execution_context) {
          !ContentSecurityPolicy::ShouldBypassMainWorld(execution_context);
 }
 
-String TrustedTypesCheckForHTML(const String& html,
+String TrustedTypesCheckForHTML(String html,
                                 const ExecutionContext* execution_context,
                                 ExceptionState& exception_state) {
   bool require_trusted_type = RequireTrustedTypesCheck(execution_context);
@@ -302,6 +302,9 @@ String TrustedTypesCheckForHTML(const String& html,
       return html;
     }
   }
+  // TODO(ajwong): This can be optimized to avoid a AddRef in the
+  // StringCache::CreateStringAndInsertIntoCache() also, but it's a hard mess.
+  // Punt for now.
   TrustedHTML* result = default_policy->CreateHTML(
       execution_context->GetIsolate(), html,
       GetDefaultCallbackArgs(execution_context->GetIsolate(), "TrustedHTML",
@@ -323,7 +326,7 @@ String TrustedTypesCheckForHTML(const String& html,
   return result->toString();
 }
 
-String TrustedTypesCheckForScript(const String& script,
+String TrustedTypesCheckForScript(String script,
                                   const ExecutionContext* execution_context,
                                   ExceptionState& exception_state) {
   bool require_trusted_type = RequireTrustedTypesCheck(execution_context);
@@ -348,6 +351,9 @@ String TrustedTypesCheckForScript(const String& script,
       return script;
     }
   }
+  // TODO(ajwong): This can be optimized to avoid a AddRef in the
+  // StringCache::CreateStringAndInsertIntoCache() also, but it's a hard mess.
+  // Punt for now.
   TrustedScript* result = default_policy->CreateScript(
       execution_context->GetIsolate(), script,
       GetDefaultCallbackArgs(execution_context->GetIsolate(), "TrustedScript",
@@ -370,7 +376,7 @@ String TrustedTypesCheckForScript(const String& script,
   return result->toString();
 }
 
-String TrustedTypesCheckForScriptURL(const String& script_url,
+String TrustedTypesCheckForScriptURL(String script_url,
                                      const ExecutionContext* execution_context,
                                      ExceptionState& exception_state) {
   bool require_trusted_type =
@@ -397,6 +403,9 @@ String TrustedTypesCheckForScriptURL(const String& script_url,
       return script_url;
     }
   }
+  // TODO(ajwong): This can be optimized to avoid a AddRef in the
+  // StringCache::CreateStringAndInsertIntoCache() also, but it's a hard mess.
+  // Punt for now.
   TrustedScriptURL* result = default_policy->CreateScriptURL(
       execution_context->GetIsolate(), script_url,
       GetDefaultCallbackArgs(execution_context->GetIsolate(),
@@ -446,7 +455,8 @@ String TrustedTypesCheckFor(
   }
 
   // In all other cases: run the full check against the string value.
-  return TrustedTypesCheckFor(type, value, execution_context, exception_state);
+  return TrustedTypesCheckFor(type, std::move(value), execution_context,
+                              exception_state);
 }
 
 String TrustedTypesCheckForScript(StringOrTrustedScript trusted,
@@ -468,41 +478,41 @@ String TrustedTypesCheckForScript(StringOrTrustedScript trusted,
 }
 
 String TrustedTypesCheckFor(SpecificTrustedType type,
-                            const String& trusted,
+                            String trusted,
                             const ExecutionContext* execution_context,
                             ExceptionState& exception_state) {
   switch (type) {
     case SpecificTrustedType::kHTML:
-      return TrustedTypesCheckForHTML(trusted, execution_context,
+      return TrustedTypesCheckForHTML(std::move(trusted), execution_context,
                                       exception_state);
     case SpecificTrustedType::kScript:
-      return TrustedTypesCheckForScript(trusted, execution_context,
+      return TrustedTypesCheckForScript(std::move(trusted), execution_context,
                                         exception_state);
     case SpecificTrustedType::kScriptURL:
-      return TrustedTypesCheckForScriptURL(trusted, execution_context,
-                                           exception_state);
+      return TrustedTypesCheckForScriptURL(std::move(trusted),
+                                           execution_context, exception_state);
     case SpecificTrustedType::kNone:
       return trusted;
   }
   NOTREACHED();
-  return "";
+  return g_empty_string;
 }
 
 String CORE_EXPORT
-GetStringForScriptExecution(const String& script,
+GetStringForScriptExecution(String script,
                             const ScriptElementBase::Type type,
                             ExecutionContext* context) {
-  return GetStringFromScriptHelper(script, context, GetElementName(type),
-                                   "text", kScriptExecution,
-                                   kScriptExecutionAndDefaultPolicyFailed);
+  return GetStringFromScriptHelper(
+      std::move(script), context, GetElementName(type), "text",
+      kScriptExecution, kScriptExecutionAndDefaultPolicyFailed);
 }
 
 String TrustedTypesCheckForJavascriptURLinNavigation(
-    const String& javascript_url,
+    String javascript_url,
     ExecutionContext* context) {
   return GetStringFromScriptHelper(
-      javascript_url, context, "Location", "href", kNavigateToJavascriptURL,
-      kNavigateToJavascriptURLAndDefaultPolicyFailed);
+      std::move(javascript_url), context, "Location", "href",
+      kNavigateToJavascriptURL, kNavigateToJavascriptURLAndDefaultPolicyFailed);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/trustedtypes/trusted_types_util.h

@@ -40,17 +40,17 @@ CORE_EXPORT String TrustedTypesCheckForScript(StringOrTrustedScript,
 // Returns the effective value (which may have been modified by the "default"
 // policy. We use WARN_UNUSED_RESULT to prevent erroneous usage.
 String TrustedTypesCheckFor(SpecificTrustedType,
-                            const String&,
+                            String,
                             const ExecutionContext*,
                             ExceptionState&) WARN_UNUSED_RESULT;
-CORE_EXPORT String TrustedTypesCheckForHTML(const String&,
+CORE_EXPORT String TrustedTypesCheckForHTML(String,
                                             const ExecutionContext*,
                                             ExceptionState&) WARN_UNUSED_RESULT;
-CORE_EXPORT String TrustedTypesCheckForScript(const String&,
+CORE_EXPORT String TrustedTypesCheckForScript(String,
                                               const ExecutionContext*,
                                               ExceptionState&)
     WARN_UNUSED_RESULT;
-CORE_EXPORT String TrustedTypesCheckForScriptURL(const String&,
+CORE_EXPORT String TrustedTypesCheckForScriptURL(String,
                                                  const ExecutionContext*,
                                                  ExceptionState&)
     WARN_UNUSED_RESULT;
@@ -58,9 +58,8 @@ CORE_EXPORT String TrustedTypesCheckForScriptURL(const String&,
 // Functionally equivalent to TrustedTypesCheckForScript(const String&, ...),
 // but with setup & error handling suitable for the asynchronous execution
 // cases.
-String TrustedTypesCheckForJavascriptURLinNavigation(const String&,
-                                                     ExecutionContext*);
-CORE_EXPORT String GetStringForScriptExecution(const String&,
+String TrustedTypesCheckForJavascriptURLinNavigation(String, ExecutionContext*);
+CORE_EXPORT String GetStringForScriptExecution(String,
                                                ScriptElementBase::Type,
                                                ExecutionContext*);