Bug 1597679 - part 3: Create methods to compare given string with values of `TextControlState`, `nsTextControlFrame`, `HTMLInputElement` and `HTMLTextAreaElement` r=smaug

For avoiding unnecessary copy of string buffer only for comparing setting
value and current value, especially with `nsAutoString`, this patch
creates `*Equals()` methods for every class.

And also this avoids to call `nsContentUtils::PlatformToDOMLineBreaks()` in
most paths.

Differential Revision: https://phabricator.services.mozilla.com/D54331

--HG--
extra : moz-landing-system : lando

dom/html/HTMLInputElement.cpp

@@ -6756,6 +6756,11 @@ void HTMLInputElement::GetTextEditorValue(nsAString& aValue,
   }
 }
 
+bool HTMLInputElement::TextEditorValueEquals(const nsAString& aValue) const {
+  TextControlState* state = GetEditorState();
+  return state ? state->ValueEquals(aValue) : aValue.IsEmpty();
+}
+
 void HTMLInputElement::InitializeKeyboardEventListeners() {
   TextControlState* state = GetEditorState();
   if (state) {

dom/html/HTMLInputElement.h

@@ -250,6 +250,13 @@ class HTMLInputElement final : public TextControlElement,
       const nsAString& aValue) override;
   virtual bool HasCachedSelection() override;
 
+  /**
+   * TextEditorValueEquals() is designed for internal use so that aValue
+   * shouldn't include \r character.  It should be handled before calling this
+   * with nsContentUtils::PlatformToDOMLineBreaks().
+   */
+  bool TextEditorValueEquals(const nsAString& aValue) const;
+
   // Methods for nsFormFillController so it can do selection operations on input
   // types the HTML spec doesn't support them on, like "email".
   uint32_t GetSelectionStartIgnoringType(ErrorResult& aRv);

dom/html/HTMLTextAreaElement.cpp

@@ -201,13 +201,8 @@ void HTMLTextAreaElement::GetType(nsAString& aType) {
 }
 
 void HTMLTextAreaElement::GetValue(nsAString& aValue) {
-  nsAutoString value;
-  GetValueInternal(value, true);
-
-  // Normalize CRLF and CR to LF
-  nsContentUtils::PlatformToDOMLineBreaks(value);
-
-  aValue = value;
+  GetValueInternal(aValue, true);
+  MOZ_ASSERT(aValue.FindChar(static_cast<char16_t>('\r')) == -1);
 }
 
 void HTMLTextAreaElement::GetValueInternal(nsAString& aValue,
@@ -216,6 +211,11 @@ void HTMLTextAreaElement::GetValueInternal(nsAString& aValue,
   mState->GetValue(aValue, aIgnoreWrap);
 }
 
+bool HTMLTextAreaElement::ValueEquals(const nsAString& aValue) const {
+  MOZ_ASSERT(mState);
+  return mState->ValueEquals(aValue);
+}
+
 TextEditor* HTMLTextAreaElement::GetTextEditor() {
   MOZ_ASSERT(mState);
   return mState->GetTextEditor();

dom/html/HTMLTextAreaElement.h

@@ -237,6 +237,12 @@ class HTMLTextAreaElement final : public TextControlElement,
   void GetDefaultValue(nsAString& aDefaultValue, ErrorResult& aError);
   void SetDefaultValue(const nsAString& aDefaultValue, ErrorResult& aError);
   void GetValue(nsAString& aValue);
+  /**
+   * ValueEquals() is designed for internal use so that aValue shouldn't
+   * include \r character.  It should be handled before calling this with
+   * nsContentUtils::PlatformToDOMLineBreaks().
+   */
+  bool ValueEquals(const nsAString& aValue) const;
   MOZ_CAN_RUN_SCRIPT_BOUNDARY
   void SetValue(const nsAString& aValue, ErrorResult& aError);
 

dom/html/TextControlState.cpp

@@ -2491,6 +2491,7 @@ void TextControlState::GetValue(nsAString& aValue, bool aIgnoreWrap) const {
   if (mHandlingState &&
       mHandlingState->IsHandling(TextControlAction::CommitComposition)) {
     aValue = mHandlingState->GetSettingValue();
+    MOZ_ASSERT(aValue.FindChar(static_cast<char16_t>('\r')) == -1);
     return;
   }
 
@@ -2498,6 +2499,7 @@ void TextControlState::GetValue(nsAString& aValue, bool aIgnoreWrap) const {
       (mEditorInitialized || !IsSingleLineTextControl())) {
     if (aIgnoreWrap && !mBoundFrame->CachedValue().IsVoid()) {
       aValue = mBoundFrame->CachedValue();
+      MOZ_ASSERT(aValue.FindChar(static_cast<char16_t>('\r')) == -1);
       return;
     }
 
@@ -2532,6 +2534,7 @@ void TextControlState::GetValue(nsAString& aValue, bool aIgnoreWrap) const {
       AutoNoJSAPI nojsapi;
 
       DebugOnly<nsresult> rv = mTextEditor->ComputeTextValue(flags, aValue);
+      MOZ_ASSERT(aValue.FindChar(static_cast<char16_t>('\r')) == -1);
       NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to get value");
     }
     // Only when the result doesn't include line breaks caused by hard-wrap,
@@ -2543,13 +2546,27 @@ void TextControlState::GetValue(nsAString& aValue, bool aIgnoreWrap) const {
     }
   } else {
     if (!mTextCtrlElement->ValueChanged() || !mValue) {
-      mTextCtrlElement->GetDefaultValueFromContent(aValue);
+      // Use nsString to avoid copying string buffer at setting aValue.
+      nsString value;
+      mTextCtrlElement->GetDefaultValueFromContent(value);
+      // TODO: We should make default value not include \r.
+      nsContentUtils::PlatformToDOMLineBreaks(value);
+      aValue = value;
     } else {
       aValue = *mValue;
+      MOZ_ASSERT(aValue.FindChar(static_cast<char16_t>('\r')) == -1);
     }
   }
 }
 
+bool TextControlState::ValueEquals(const nsAString& aValue) const {
+  // We can avoid copying string buffer in many cases.  Therefore, we should
+  // use nsString rather than nsAutoString here.
+  nsString value;
+  GetValue(value, true);
+  return aValue.Equals(value);
+}
+
 #ifdef DEBUG
 // @param aFlags TextControlState::SetValueFlags
 bool AreFlagsNotDemandingContradictingMovements(uint32_t aFlags) {
@@ -2596,17 +2613,12 @@ bool TextControlState::SetValue(const nsAString& aValue,
       } else {
         // If setting value won't change current value, we shouldn't commit
         // composition for compatibility with the other browsers.
-        nsAutoString currentValue;
-        if (aOldValue) {
-#ifdef DEBUG
-          mBoundFrame->GetText(currentValue);
-          MOZ_ASSERT(currentValue.Equals(*aOldValue));
-#endif
-          currentValue.Assign(*aOldValue);
-        } else {
-          mBoundFrame->GetText(currentValue);
-        }
-        if (handlingSetValue.GetSettingValue() == currentValue) {
+        MOZ_ASSERT(!aOldValue || mBoundFrame->TextEquals(*aOldValue));
+        bool isSameAsCurrentValue =
+            aOldValue
+                ? aOldValue->Equals(handlingSetValue.GetSettingValue())
+                : mBoundFrame->TextEquals(handlingSetValue.GetSettingValue());
+        if (isSameAsCurrentValue) {
           // Note that in this case, we shouldn't fire any events with setting
           // value because event handlers may try to set value recursively but
           // we cannot commit composition at that time due to unsafe to run
@@ -2691,19 +2703,16 @@ bool TextControlState::SetValueWithTextEditor(
   }
 #endif
 
-  nsAutoString currentValue;
-  if (aHandlingSetValue.GetOldValue()) {
-#ifdef DEBUG
-    mBoundFrame->GetText(currentValue);
-    MOZ_ASSERT(currentValue.Equals(*aHandlingSetValue.GetOldValue()));
-#endif
-    currentValue.Assign(*aHandlingSetValue.GetOldValue());
-  } else {
-    mBoundFrame->GetText(currentValue);
-  }
+  MOZ_ASSERT(!aHandlingSetValue.GetOldValue() ||
+             mBoundFrame->TextEquals(*aHandlingSetValue.GetOldValue()));
+  bool isSameAsCurrentValue =
+      aHandlingSetValue.GetOldValue()
+          ? aHandlingSetValue.GetOldValue()->Equals(
+                aHandlingSetValue.GetSettingValue())
+          : mBoundFrame->TextEquals(aHandlingSetValue.GetSettingValue());
 
   // this is necessary to avoid infinite recursion
-  if (currentValue == aHandlingSetValue.GetSettingValue()) {
+  if (isSameAsCurrentValue) {
     return true;
   }
 
@@ -2763,6 +2772,13 @@ bool TextControlState::SetValueWithTextEditor(
     //       }
     //     However, this path won't be used in web content anymore.
     nsCOMPtr<nsISelectionController> kungFuDeathGrip = mSelCon.get();
+    // Use nsString to avoid copying string buffer in most cases.
+    nsString currentValue;
+    if (aHandlingSetValue.GetOldValue()) {
+      currentValue.Assign(*aHandlingSetValue.GetOldValue());
+    } else {
+      mBoundFrame->GetText(currentValue);
+    }
     uint32_t currentLength = currentValue.Length();
     uint32_t newlength = aHandlingSetValue.GetSettingValue().Length();
     if (!currentLength ||

dom/html/TextControlState.h

@@ -208,6 +208,15 @@ class TextControlState final : public SupportsWeakPtr<TextControlState> {
     // it.
     eSetValue_MoveCursorToBeginSetSelectionDirectionForward = 1 << 6,
   };
+  /**
+   * SetValue() sets the value to aValue with replacing \r\n and \r with \n.
+   *
+   * @param aValue      The new value.  Can contain \r.
+   * @param aOldValue   Optional.  If you have already know current value,
+   *                    set this to it.  However, this must not contain \r
+   *                    for the performance.
+   * @param aFlags      See SetValueFlags.
+   */
   MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE bool SetValue(const nsAString& aValue,
                                                 const nsAString* aOldValue,
                                                 uint32_t aFlags);
@@ -215,7 +224,17 @@ class TextControlState final : public SupportsWeakPtr<TextControlState> {
                                                 uint32_t aFlags) {
     return SetValue(aValue, nullptr, aFlags);
   }
+  /**
+   * GetValue() returns current value either with or without TextEditor.
+   * The result never includes \r.
+   */
   void GetValue(nsAString& aValue, bool aIgnoreWrap) const;
+  /**
+   * ValueEquals() is designed for internal use so that aValue shouldn't
+   * include \r character.  It should be handled before calling this with
+   * nsContentUtils::PlatformToDOMLineBreaks().
+   */
+  bool ValueEquals(const nsAString& aValue) const;
   bool HasNonEmptyValue();
   // The following methods are for textarea element to use whether default
   // value or not.

layout/forms/nsTextControlFrame.cpp

@@ -1124,18 +1124,43 @@ nsresult nsTextControlFrame::AttributeChanged(int32_t aNameSpaceID,
 }
 
 void nsTextControlFrame::GetText(nsString& aText) {
-  TextControlElement* textControlElement =
-      TextControlElement::FromNode(GetContent());
-  MOZ_ASSERT(textControlElement);
-  if (IsSingleLineTextControl()) {
-    // There will be no line breaks so we can ignore the wrap property.
-    textControlElement->GetTextEditorValue(aText, true);
-  } else {
-    HTMLTextAreaElement* textArea = HTMLTextAreaElement::FromNode(mContent);
-    if (textArea) {
-      textArea->GetValue(aText);
+  if (HTMLInputElement* inputElement = HTMLInputElement::FromNode(mContent)) {
+    if (IsSingleLineTextControl()) {
+      // There will be no line breaks so we can ignore the wrap property.
+      inputElement->GetTextEditorValue(aText, true);
+      return;
     }
+    aText.Truncate();
+    return;
   }
+
+  MOZ_ASSERT(!IsSingleLineTextControl());
+  if (HTMLTextAreaElement* textAreaElement =
+          HTMLTextAreaElement::FromNode(mContent)) {
+    textAreaElement->GetValue(aText);
+    return;
+  }
+
+  MOZ_ASSERT(aText.IsEmpty());
+  aText.Truncate();
+}
+
+bool nsTextControlFrame::TextEquals(const nsAString& aText) const {
+  if (HTMLInputElement* inputElement = HTMLInputElement::FromNode(mContent)) {
+    if (IsSingleLineTextControl()) {
+      // There will be no line breaks so we can ignore the wrap property.
+      return inputElement->TextEditorValueEquals(aText);
+    }
+    return aText.IsEmpty();
+  }
+
+  MOZ_ASSERT(!IsSingleLineTextControl());
+  if (HTMLTextAreaElement* textAreaElement =
+          HTMLTextAreaElement::FromNode(mContent)) {
+    return textAreaElement->ValueEquals(aText);
+  }
+
+  return aText.IsEmpty();
 }
 
 /// END NSIFRAME OVERLOADS

layout/forms/nsTextControlFrame.h

@@ -163,6 +163,13 @@ class nsTextControlFrame final : public nsContainerFrame,
 
   void GetText(nsString& aText);
 
+  /**
+   * TextEquals() is designed for internal use so that aValue shouldn't
+   * include \r character.  It should be handled before calling this with
+   * nsContentUtils::PlatformToDOMLineBreaks().
+   */
+  bool TextEquals(const nsAString& aText) const;
+
   virtual nsresult PeekOffset(nsPeekOffsetStruct* aPos) override;
 
   NS_DECL_QUERYFRAME