Bug 1391538 - nsTextFragment for text nodes in <input> or <textarea> shouldn't store text as single byte characters even if all characters are less than U+0100 r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 18 Aug 2017 16:05:16 +0900
changeset 649331 59aa719a2f22d3bd05d0032adf532888073ca289
parent 648952 da7bd11fed7a1dd03310f065d9f2721d7c06ea5f
child 727062 aea6df1dbdd448fbae542805e7acc9cf3a0ab7f5
push id75014
push usermasayuki@d-toybox.com
push dateSat, 19 Aug 2017 00:47:22 +0000
reviewerssmaug
bugs1391538
milestone57.0a1
Bug 1391538 - nsTextFragment for text nodes in <input> or <textarea> shouldn't store text as single byte characters even if all characters are less than U+0100 r?smaug nsTextFrame stores text as single byte character array if all characters are less than U+0100. Although, this saves footprint, but retrieving and modifying text needs converting cost. Therefore, if it's created for a text node in <input> or <textarea>, it should store text as char16_t array. MozReview-Commit-ID: 9Z82rketT7g
dom/base/nsDocument.cpp
dom/base/nsGenericDOMDataNode.cpp
dom/base/nsGenericDOMDataNode.h
dom/base/nsIDocument.h
dom/base/nsTextFragment.cpp
dom/base/nsTextFragment.h
dom/html/nsTextEditorState.cpp
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/TextEditRules.cpp
layout/forms/nsTextControlFrame.cpp
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -5901,16 +5901,23 @@ nsDocument::CreateElementNS(const nsAStr
 NS_IMETHODIMP
 nsDocument::CreateTextNode(const nsAString& aData, nsIDOMText** aReturn)
 {
   *aReturn = nsIDocument::CreateTextNode(aData).take();
   return NS_OK;
 }
 
 already_AddRefed<nsTextNode>
+nsIDocument::CreateEmptyTextNode() const
+{
+  RefPtr<nsTextNode> text = new nsTextNode(mNodeInfoManager);
+  return text.forget();
+}
+
+already_AddRefed<nsTextNode>
 nsIDocument::CreateTextNode(const nsAString& aData) const
 {
   RefPtr<nsTextNode> text = new nsTextNode(mNodeInfoManager);
   // Don't notify; this node is still being created.
   text->SetText(aData, false);
   return text.forget();
 }
 
--- a/dom/base/nsGenericDOMDataNode.cpp
+++ b/dom/base/nsGenericDOMDataNode.cpp
@@ -314,22 +314,28 @@ nsGenericDOMDataNode::SetTextInternal(ui
 
   Directionality oldDir = eDir_NotSet;
   bool dirAffectsAncestor = (NodeType() == nsIDOMNode::TEXT_NODE &&
                              TextNodeWillChangeDirection(this, &oldDir, aOffset));
 
   if (aOffset == 0 && endOffset == textLength) {
     // Replacing whole text or old text was empty.  Don't bother to check for
     // bidi in this string if the document already has bidi enabled.
-    bool ok = mText.SetTo(aBuffer, aLength, !document || !document->GetBidiEnabled());
+    // If this is marked as "maybe modified frequently", the text should be
+    // stored as char16_t since converting char* to char16_t* is expensive.
+    bool ok =
+      mText.SetTo(aBuffer, aLength, !document || !document->GetBidiEnabled(),
+                  HasFlag(NS_MAYBE_MODIFIED_FREQUENTLY));
     NS_ENSURE_TRUE(ok, NS_ERROR_OUT_OF_MEMORY);
   }
   else if (aOffset == textLength) {
     // Appending to existing
-    bool ok = mText.Append(aBuffer, aLength, !document || !document->GetBidiEnabled());
+    bool ok =
+      mText.Append(aBuffer, aLength, !document || !document->GetBidiEnabled(),
+                   HasFlag(NS_MAYBE_MODIFIED_FREQUENTLY));
     NS_ENSURE_TRUE(ok, NS_ERROR_OUT_OF_MEMORY);
   }
   else {
     // Merging old and new
 
     // Allocate new buffer
     int32_t newLength = textLength - aCount + aLength;
     char16_t* to = new char16_t[newLength];
@@ -340,17 +346,21 @@ nsGenericDOMDataNode::SetTextInternal(ui
     }
     if (aLength) {
       memcpy(to + aOffset, aBuffer, aLength * sizeof(char16_t));
     }
     if (endOffset != textLength) {
       mText.CopyTo(to + aOffset + aLength, endOffset, textLength - endOffset);
     }
 
-    bool ok = mText.SetTo(to, newLength, !document || !document->GetBidiEnabled());
+    // If this is marked as "maybe modified frequently", the text should be
+    // stored as char16_t since converting char* to char16_t* is expensive.
+    bool ok =
+      mText.SetTo(to, newLength, !document || !document->GetBidiEnabled(),
+                  HasFlag(NS_MAYBE_MODIFIED_FREQUENTLY));
 
     delete [] to;
 
     NS_ENSURE_TRUE(ok, NS_ERROR_OUT_OF_MEMORY);
   }
 
   UnsetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE);
 
--- a/dom/base/nsGenericDOMDataNode.h
+++ b/dom/base/nsGenericDOMDataNode.h
@@ -50,33 +50,42 @@ enum {
 
   // This bit is set if there is a NewlineProperty attached to the node
   // (used by nsTextFrame).
   NS_HAS_NEWLINE_PROPERTY =               DATA_NODE_FLAG_BIT(4),
 
   // This bit is set if there is a FlowLengthProperty attached to the node
   // (used by nsTextFrame).
   NS_HAS_FLOWLENGTH_PROPERTY =            DATA_NODE_FLAG_BIT(5),
+
+  // This bit is set if the node may be modified frequently.  This is typically
+  // specified if the instance is in <input> or <textarea>.
+  NS_MAYBE_MODIFIED_FREQUENTLY =          DATA_NODE_FLAG_BIT(6),
 };
 
 // Make sure we have enough space for those bits
-ASSERT_NODE_FLAGS_SPACE(NODE_TYPE_SPECIFIC_BITS_OFFSET + 6);
+ASSERT_NODE_FLAGS_SPACE(NODE_TYPE_SPECIFIC_BITS_OFFSET + 7);
 
 #undef DATA_NODE_FLAG_BIT
 
 class nsGenericDOMDataNode : public nsIContent
 {
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
 
   NS_DECL_ADDSIZEOFEXCLUDINGTHIS
 
   explicit nsGenericDOMDataNode(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo);
   explicit nsGenericDOMDataNode(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
 
+  void MarkAsMaybeModifiedFrequently()
+  {
+    SetFlags(NS_MAYBE_MODIFIED_FREQUENTLY);
+  }
+
   virtual void GetNodeValueInternal(nsAString& aNodeValue) override;
   virtual void SetNodeValueInternal(const nsAString& aNodeValue,
                                     mozilla::ErrorResult& aError) override;
 
   // Implementation for nsIDOMCharacterData
   nsresult GetData(nsAString& aData) const;
   nsresult SetData(const nsAString& aData);
   nsresult GetLength(uint32_t* aLength);
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -2735,16 +2735,17 @@ public:
   virtual already_AddRefed<Element>
     CreateElementNS(const nsAString& aNamespaceURI,
                     const nsAString& aQualifiedName,
                     const mozilla::dom::ElementCreationOptionsOrString& aOptions,
                     mozilla::ErrorResult& rv) = 0;
   already_AddRefed<mozilla::dom::DocumentFragment>
     CreateDocumentFragment() const;
   already_AddRefed<nsTextNode> CreateTextNode(const nsAString& aData) const;
+  already_AddRefed<nsTextNode> CreateEmptyTextNode() const;
   already_AddRefed<mozilla::dom::Comment>
     CreateComment(const nsAString& aData) const;
   already_AddRefed<mozilla::dom::ProcessingInstruction>
     CreateProcessingInstruction(const nsAString& target, const nsAString& data,
                                 mozilla::ErrorResult& rv) const;
   already_AddRefed<nsINode>
     ImportNode(nsINode& aNode, bool aDeep, mozilla::ErrorResult& rv) const;
   nsINode* AdoptNode(nsINode& aNode, mozilla::ErrorResult& rv);
--- a/dom/base/nsTextFragment.cpp
+++ b/dom/base/nsTextFragment.cpp
@@ -191,39 +191,41 @@ FirstNon8Bit(const char16_t *str, const 
     return mozilla::SSE2::FirstNon8Bit(str, end);
   }
 #endif
 
   return FirstNon8BitUnvectorized(str, end);
 }
 
 bool
-nsTextFragment::SetTo(const char16_t* aBuffer, int32_t aLength, bool aUpdateBidi)
+nsTextFragment::SetTo(const char16_t* aBuffer, int32_t aLength,
+                      bool aUpdateBidi, bool aForce2b)
 {
   ReleaseText();
 
   if (aLength == 0) {
     return true;
   }
 
   char16_t firstChar = *aBuffer;
-  if (aLength == 1 && firstChar < 256) {
+  if (!aForce2b && aLength == 1 && firstChar < 256) {
     m1b = sSingleCharSharedString + firstChar;
     mState.mInHeap = false;
     mState.mIs2b = false;
     mState.mLength = 1;
 
     return true;
   }
 
   const char16_t *ucp = aBuffer;
   const char16_t *uend = aBuffer + aLength;
 
   // Check if we can use a shared string
-  if (aLength <= 1 + TEXTFRAG_WHITE_AFTER_NEWLINE + TEXTFRAG_MAX_NEWLINES &&
+  if (!aForce2b &&
+      aLength <= 1 + TEXTFRAG_WHITE_AFTER_NEWLINE + TEXTFRAG_MAX_NEWLINES &&
      (firstChar == ' ' || firstChar == '\n' || firstChar == '\t')) {
     if (firstChar == ' ') {
       ++ucp;
     }
 
     const char16_t* start = ucp;
     while (ucp < uend && *ucp == '\n') {
       ++ucp;
@@ -250,17 +252,17 @@ nsTextFragment::SetTo(const char16_t* aB
       mState.mIs2b = false;
       mState.mLength = aLength;
 
       return true;
     }
   }
 
   // See if we need to store the data in ucs2 or not
-  int32_t first16bit = FirstNon8Bit(ucp, uend);
+  int32_t first16bit = aForce2b ? 0 : FirstNon8Bit(ucp, uend);
 
   if (first16bit != -1) { // aBuffer contains no non-8bit character
     // Use ucs2 storage because we have to
     CheckedUint32 m2bSize = aLength;
     m2bSize *= sizeof(char16_t);
     if (!m2bSize.isValid()) {
       return false;
     }
@@ -319,22 +321,23 @@ nsTextFragment::CopyTo(char16_t *aDest, 
       const char *end = cp + aCount;
       LossyConvertEncoding8to16 converter(aDest);
       copy_string(cp, end, converter);
     }
   }
 }
 
 bool
-nsTextFragment::Append(const char16_t* aBuffer, uint32_t aLength, bool aUpdateBidi)
+nsTextFragment::Append(const char16_t* aBuffer, uint32_t aLength,
+                       bool aUpdateBidi, bool aForce2b)
 {
   // This is a common case because some callsites create a textnode
   // with a value by creating the node and then calling AppendData.
   if (mState.mLength == 0) {
-    return SetTo(aBuffer, aLength, aUpdateBidi);
+    return SetTo(aBuffer, aLength, aUpdateBidi, aForce2b);
   }
 
   // Should we optimize for aData.Length() == 0?
 
   CheckedUint32 length = mState.mLength;
   length += aLength;
 
   if (!length.isValid()) {
@@ -360,17 +363,17 @@ nsTextFragment::Append(const char16_t* a
     if (aUpdateBidi) {
       UpdateBidiFlag(aBuffer, aLength);
     }
 
     return true;
   }
 
   // Current string is a 1-byte string, check if the new data fits in one byte too.
-  int32_t first16bit = FirstNon8Bit(aBuffer, aBuffer + aLength);
+  int32_t first16bit = aForce2b ? 0 : FirstNon8Bit(aBuffer, aBuffer + aLength);
 
   if (first16bit != -1) { // aBuffer contains no non-8bit character
     length *= sizeof(char16_t);
     if (!length.isValid()) {
       return false;
     }
 
     // The old data was 1-byte, but the new is not so we have to expand it
--- a/dom/base/nsTextFragment.h
+++ b/dom/base/nsTextFragment.h
@@ -107,25 +107,33 @@ public:
   {
     return n < (1 << 29) && mState.mLength + n < (1 << 29);
   }
 
   /**
    * Change the contents of this fragment to be a copy of the given
    * buffer. If aUpdateBidi is true, contents of the fragment will be scanned,
    * and mState.mIsBidi will be turned on if it includes any Bidi characters.
+   * If aForce2b is true, aBuffer will be stored as char16_t as is.  Then,
+   * you can access the value faster but may waste memory if all characters
+   * are less than U+0100.
    */
-  bool SetTo(const char16_t* aBuffer, int32_t aLength, bool aUpdateBidi);
+  bool SetTo(const char16_t* aBuffer, int32_t aLength, bool aUpdateBidi,
+             bool aForce2b);
 
   /**
    * Append aData to the end of this fragment. If aUpdateBidi is true, contents
    * of the fragment will be scanned, and mState.mIsBidi will be turned on if
    * it includes any Bidi characters.
+   * If aForce2b is true, the string will be stored as char16_t as is.  Then,
+   * you can access the value faster but may waste memory if all characters
+   * are less than U+0100.
    */
-  bool Append(const char16_t* aBuffer, uint32_t aLength, bool aUpdateBidi);
+  bool Append(const char16_t* aBuffer, uint32_t aLength, bool aUpdateBidi,
+              bool aForce2b);
 
   /**
    * Append the contents of this string fragment to aString
    */
   void AppendTo(nsAString& aString) const {
     if (!AppendTo(aString, mozilla::fallible)) {
       aString.AllocFailed(aString.Length() + GetLength());
     }
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -2354,16 +2354,17 @@ nsTextEditorState::CreateEmptyDivNode()
   nodeInfo = pNodeInfoManager->GetNodeInfo(nsGkAtoms::div, nullptr,
                                            kNameSpaceID_XHTML,
                                            nsIDOMNode::ELEMENT_NODE);
 
   element = NS_NewHTMLDivElement(nodeInfo.forget());
 
   // Create the text node for DIV
   RefPtr<nsTextNode> textNode = new nsTextNode(pNodeInfoManager);
+  textNode->MarkAsMaybeModifiedFrequently();
 
   element->AppendChildTo(textNode, false);
 
   return element;
 }
 
 nsresult
 nsTextEditorState::CreatePlaceholderNode()
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2509,17 +2509,18 @@ EditorBase::InsertTextImpl(const nsAStri
       offset = 0;
     }
   }
 
   if (ShouldHandleIMEComposition()) {
     CheckedInt<int32_t> newOffset;
     if (!node->IsNodeOfType(nsINode::eTEXT)) {
       // create a text node
-      RefPtr<nsTextNode> newNode = aDoc->CreateTextNode(EmptyString());
+      RefPtr<nsTextNode> newNode =
+        EditorBase::CreateTextNode(*aDoc, EmptyString());
       // then we insert it into the dom tree
       nsresult rv = InsertNode(*newNode, *node, offset);
       NS_ENSURE_SUCCESS(rv, rv);
       node = newNode;
       offset = 0;
       newOffset = lengthToInsert;
     } else {
       newOffset = lengthToInsert + offset;
@@ -2536,17 +2537,18 @@ EditorBase::InsertTextImpl(const nsAStri
       // we are inserting text into an existing text node.
       nsresult rv =
         InsertTextIntoTextNodeImpl(aStringToInsert, *node->GetAsText(), offset);
       NS_ENSURE_SUCCESS(rv, rv);
       offset = newOffset.value();
     } else {
       // we are inserting text into a non-text node.  first we have to create a
       // textnode (this also populates it with the text)
-      RefPtr<nsTextNode> newNode = aDoc->CreateTextNode(aStringToInsert);
+      RefPtr<nsTextNode> newNode =
+        EditorBase::CreateTextNode(*aDoc, aStringToInsert);
       // then we insert it into the dom tree
       nsresult rv = InsertNode(*newNode, *node, offset);
       NS_ENSURE_SUCCESS(rv, rv);
       node = newNode;
       offset = lengthToInsert.value();
     }
   }
 
@@ -4725,16 +4727,28 @@ EditorBase::CreateHTMLContent(nsIAtom* a
              "check caller.");
     return nullptr;
   }
 
   return doc->CreateElem(nsDependentAtomString(aTag), nullptr,
                          kNameSpaceID_XHTML);
 }
 
+// static
+already_AddRefed<nsTextNode>
+EditorBase::CreateTextNode(nsIDocument& aDocument,
+                           const nsAString& aData)
+{
+  RefPtr<nsTextNode> text = aDocument.CreateEmptyTextNode();
+  text->MarkAsMaybeModifiedFrequently();
+  // Don't notify; this node is still being created.
+  text->SetText(aData, false);
+  return text.forget();
+}
+
 NS_IMETHODIMP
 EditorBase::SetAttributeOrEquivalent(nsIDOMElement* aElement,
                                      const nsAString& aAttribute,
                                      const nsAString& aValue,
                                      bool aSuppressTransaction)
 {
   nsCOMPtr<Element> element = do_QueryInterface(aElement);
   if (NS_WARN_IF(!element)) {
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -344,16 +344,22 @@ public:
   /**
    * Method to replace certain CreateElementNS() calls.
    *
    * @param aTag        Tag you want.
    */
   already_AddRefed<Element> CreateHTMLContent(nsIAtom* aTag);
 
   /**
+   * Creates text node which is marked as "maybe modified frequently".
+   */
+  static already_AddRefed<nsTextNode> CreateTextNode(nsIDocument& aDocument,
+                                                     const nsAString& aData);
+
+  /**
    * IME event handlers.
    */
   virtual nsresult BeginIMEComposition(WidgetCompositionEvent* aEvent);
   virtual nsresult UpdateIMEComposition(
                      WidgetCompositionEvent* aCompositionChangeEvet) = 0;
   void EndIMEComposition();
 
   /**
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -4608,17 +4608,18 @@ HTMLEditRules::CreateStyleForInsertText(
       NS_ENSURE_STATE(mHTMLEditor);
       offset = mHTMLEditor->SplitNodeDeep(*text, *text, offset);
       NS_ENSURE_STATE(offset != -1);
       node = node->GetParentNode();
     }
     if (!mHTMLEditor->IsContainer(node)) {
       return NS_OK;
     }
-    OwningNonNull<Text> newNode = aDoc.CreateTextNode(EmptyString());
+    OwningNonNull<Text> newNode =
+      EditorBase::CreateTextNode(aDoc, EmptyString());
     NS_ENSURE_STATE(mHTMLEditor);
     nsresult rv = mHTMLEditor->InsertNode(*newNode, *node, offset);
     NS_ENSURE_SUCCESS(rv, rv);
     node = newNode;
     offset = 0;
     weDidSomething = true;
 
     if (relFontSize) {
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -874,17 +874,17 @@ TextEditRules::WillSetText(Selection& aS
     if (tString.IsEmpty()) {
       *aHandled = true;
       return NS_OK;
     }
     RefPtr<nsIDocument> doc = textEditor->GetDocument();
     if (NS_WARN_IF(!doc)) {
       return NS_OK;
     }
-    RefPtr<nsTextNode> newNode = doc->CreateTextNode(tString);
+    RefPtr<nsTextNode> newNode = EditorBase::CreateTextNode(*doc, tString);
     if (NS_WARN_IF(!newNode)) {
       return NS_OK;
     }
     nsresult rv = textEditor->InsertNode(*newNode, *rootElement, 0);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     *aHandled = true;
--- a/layout/forms/nsTextControlFrame.cpp
+++ b/layout/forms/nsTextControlFrame.cpp
@@ -1166,16 +1166,17 @@ nsTextControlFrame::UpdateValueDisplay(b
   NS_ASSERTION(!mUsePlaceholder || txtCtrl->GetPlaceholderNode(),
                "A placeholder div must exist");
 
   nsIContent *textContent = rootNode->GetChildAt(0);
   if (!textContent) {
     // Set up a textnode with our value
     RefPtr<nsTextNode> textNode =
       new nsTextNode(mContent->NodeInfo()->NodeInfoManager());
+    textNode->MarkAsMaybeModifiedFrequently();
 
     NS_ASSERTION(textNode, "Must have textcontent!\n");
 
     rootNode->AppendChildTo(textNode, aNotify);
     textContent = textNode;
   }
 
   NS_ENSURE_TRUE(textContent, NS_ERROR_UNEXPECTED);