Bug 1478564 - part 1: Optimize TextEditRules::HandleNewLines() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 24 Jul 2018 17:46:12 +0900
changeset 428708 16001f32b7f9a2f6a6bb919fad1dbfeed223eab2
parent 428707 16b8136f47f172303132f7b86aa164f987a41ef7
child 428709 dd5e331122a911f9bb88e50205de4e2e841073cd
push id34340
push userdvarga@mozilla.com
push dateFri, 27 Jul 2018 17:37:02 +0000
treeherdermozilla-central@d353b80fd66c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1478564
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1478564 - part 1: Optimize TextEditRules::HandleNewLines() r=m_kato TextEditRules::HandleNewLines() is expensive since it may scan all of given string twice and more. On the other hand, in most cases, given string does not contain \n, \r nor \r\n. First, for avoid using nsTString::FindCharInSet(), HandleNewLine() should receive string which never contains \r nor \r\n. Then, it always can use nsTSubstring::FindChar() instead. Next, HandleNewLines() should do nothing if given string does not contain \n. Finally, because of unused, this removes unnecessary HandleNewLines() argument which can specify the way to handle new lines. MozReview-Commit-ID: 8WSfxfkuFgN
editor/libeditor/TextEditRules.cpp
editor/libeditor/TextEditRules.h
editor/libeditor/TextEditor.cpp
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -591,83 +591,88 @@ TextEditRules::GetTextNodeAroundSelectio
     mTextEditor->GetTextLength(&txtLen);                               \
     NS_ASSERTION(mPasswordText.Length() == uint32_t(txtLen),           \
                  "password length not equal to number of asterisks");  \
   }
 #else
 #define ASSERT_PASSWORD_LENGTHS_EQUAL()
 #endif
 
-// static
 void
-TextEditRules::HandleNewLines(nsString& aString,
-                              int32_t aNewlineHandling)
+TextEditRules::HandleNewLines(nsString& aString)
 {
-  if (aNewlineHandling < 0) {
-    int32_t caretStyle;
-    TextEditor::GetDefaultEditorPrefs(aNewlineHandling, caretStyle);
+  static const char16_t kLF = static_cast<char16_t>('\n');
+  MOZ_ASSERT(IsEditorDataAvailable());
+  MOZ_ASSERT(aString.FindChar(static_cast<uint16_t>('\r')) == kNotFound);
+
+  // First of all, check if aString contains '\n' since if the string
+  // does not include it, we don't need to do nothing here.
+  int32_t firstLF = aString.FindChar(kLF, 0);
+  if (firstLF == kNotFound) {
+    return;
   }
 
-  switch(aNewlineHandling) {
+  switch(TextEditorRef().mNewlineHandling) {
     case nsIPlaintextEditor::eNewlinesReplaceWithSpaces:
+      // Default of Firefox:
       // Strip trailing newlines first so we don't wind up with trailing spaces
-      aString.Trim(CRLF, false, true);
-      aString.ReplaceChar(CRLF, ' ');
+      aString.Trim(LFSTR, false, true);
+      aString.ReplaceChar(kLF, ' ');
       break;
     case nsIPlaintextEditor::eNewlinesStrip:
-      aString.StripCRLF();
+      aString.StripChar(kLF);
       break;
     case nsIPlaintextEditor::eNewlinesPasteToFirst:
     default: {
-      int32_t firstCRLF = aString.FindCharInSet(CRLF);
-
       // we get first *non-empty* line.
       int32_t offset = 0;
-      while (firstCRLF == offset) {
+      while (firstLF == offset) {
         offset++;
-        firstCRLF = aString.FindCharInSet(CRLF, offset);
+        firstLF = aString.FindChar(kLF, offset);
       }
-      if (firstCRLF > 0) {
-        aString.Truncate(firstCRLF);
+      if (firstLF > 0) {
+        aString.Truncate(firstLF);
       }
       if (offset > 0) {
         aString.Cut(0, offset);
       }
       break;
     }
     case nsIPlaintextEditor::eNewlinesReplaceWithCommas:
-      aString.Trim(CRLF, true, true);
-      aString.ReplaceChar(CRLF, ',');
+      // Default of Thunderbird:
+      aString.Trim(LFSTR, true, true);
+      aString.ReplaceChar(kLF, ',');
       break;
     case nsIPlaintextEditor::eNewlinesStripSurroundingWhitespace: {
       nsAutoString result;
       uint32_t offset = 0;
       while (offset < aString.Length()) {
-        int32_t nextCRLF = aString.FindCharInSet(CRLF, offset);
-        if (nextCRLF < 0) {
+        int32_t nextLF =
+          !offset ? firstLF : aString.FindChar(kLF, offset);
+        if (nextLF < 0) {
           result.Append(nsDependentSubstring(aString, offset));
           break;
         }
-        uint32_t wsBegin = nextCRLF;
+        uint32_t wsBegin = nextLF;
         // look backwards for the first non-whitespace char
         while (wsBegin > offset && NS_IS_SPACE(aString[wsBegin - 1])) {
           --wsBegin;
         }
         result.Append(nsDependentSubstring(aString, offset, wsBegin - offset));
-        offset = nextCRLF + 1;
+        offset = nextLF + 1;
         while (offset < aString.Length() && NS_IS_SPACE(aString[offset])) {
           ++offset;
         }
       }
       aString = result;
       break;
     }
     case nsIPlaintextEditor::eNewlinesPasteIntact:
       // even if we're pasting newlines, don't paste leading/trailing ones
-      aString.Trim(CRLF, true, true);
+      aString.Trim(LFSTR, true, true);
       break;
   }
 }
 
 nsresult
 TextEditRules::WillInsertText(EditSubAction aEditSubAction,
                               bool* aCancel,
                               bool* aHandled,
@@ -755,17 +760,25 @@ TextEditRules::WillInsertText(EditSubAct
   // 1. paste up to the first newline (default)
   // 2. replace newlines with spaces
   // 3. strip newlines
   // 4. replace with commas
   // 5. strip newlines and surrounding whitespace
   // So find out what we're expected to do:
   if (IsSingleLineEditor()) {
     nsAutoString tString(*outString);
-    HandleNewLines(tString, TextEditorRef().mNewlineHandling);
+    // XXX Some callers of TextEditor::InsertTextAsAction()  already make the
+    //     string use only \n as a linebreaker.  However, they are not hot
+    //     path and nsContentUtils::PlatformToDOMLineBreaks() does nothing
+    //     if the string doesn't include \r.  So, let's convert linebreakers
+    //     here.  Note that there are too many callers of
+    //     TextEditor::InsertTextAsAction().  So, it's difficult to keep
+    //     maintaining all of them won't reach here without \r nor \r\n.
+    nsContentUtils::PlatformToDOMLineBreaks(tString);
+    HandleNewLines(tString);
     outString->Assign(tString);
   }
 
   if (IsPasswordEditor()) {
     // manage the password buffer
     mPasswordText.Insert(*outString, start);
 
     if (LookAndFeel::GetEchoPassword() && !DontEchoPassword()) {
@@ -880,16 +893,17 @@ TextEditRules::WillSetText(bool* aCancel
                            bool* aHandled,
                            const nsAString* aString,
                            int32_t aMaxLength)
 {
   MOZ_ASSERT(IsEditorDataAvailable());
   MOZ_ASSERT(aCancel);
   MOZ_ASSERT(aHandled);
   MOZ_ASSERT(aString);
+  MOZ_ASSERT(aString->FindChar(static_cast<char16_t>('\r')) == kNotFound);
 
   CANCEL_OPERATION_IF_READONLY_OR_DISABLED
 
   *aHandled = false;
   *aCancel = false;
 
   if (!IsPlaintextEditor() || TextEditorRef().IsIMEComposing() ||
       aMaxLength != -1) {
@@ -918,17 +932,17 @@ TextEditRules::WillSetText(bool* aCancel
   }
 
   nsAutoString tString(*aString);
 
   if (IsPasswordEditor()) {
     mPasswordText.Assign(tString);
     FillBufWithPWChars(&tString, tString.Length());
   } else if (IsSingleLineEditor()) {
-    HandleNewLines(tString, TextEditorRef().mNewlineHandling);
+    HandleNewLines(tString);
   }
 
   if (!count) {
     if (tString.IsEmpty()) {
       *aHandled = true;
       return NS_OK;
     }
     RefPtr<nsIDocument> doc = TextEditorRef().GetDocument();
--- a/editor/libeditor/TextEditRules.h
+++ b/editor/libeditor/TextEditRules.h
@@ -99,38 +99,36 @@ public:
 
 protected:
   virtual ~TextEditRules();
 
 public:
   void ResetIMETextPWBuf();
 
   /**
-   * Handles the newline characters either according to aNewLineHandling
-   * or to the default system prefs if aNewLineHandling is negative.
+   * Handles the newline characters according to the default system prefs
+   * (editor.singleLine.pasteNewlines).
+   * Each value means:
+   *   nsIPlaintextEditor::eNewlinesReplaceWithSpaces (2, Firefox default):
+   *     replace newlines with spaces.
+   *   nsIPlaintextEditor::eNewlinesStrip (3):
+   *     remove newlines from the string.
+   *   nsIPlaintextEditor::eNewlinesReplaceWithCommas (4, Thunderbird default):
+   *     replace newlines with commas.
+   *   nsIPlaintextEditor::eNewlinesStripSurroundingWhitespace (5):
+   *     collapse newlines and surrounding whitespace characters and
+   *     remove them from the string.
+   *   nsIPlaintextEditor::eNewlinesPasteIntact (0):
+   *     only remove the leading and trailing newlines.
+   *   nsIPlaintextEditor::eNewlinesPasteToFirst (1) or any other value:
+   *     remove the first newline and all characters following it.
    *
    * @param aString the string to be modified in place.
-   * @param aNewLineHandling determine the desired type of newline handling:
-   *        * negative values:
-   *          handle newlines according to platform defaults.
-   *        * nsIPlaintextEditor::eNewlinesReplaceWithSpaces:
-   *          replace newlines with spaces.
-   *        * nsIPlaintextEditor::eNewlinesStrip:
-   *          remove newlines from the string.
-   *        * nsIPlaintextEditor::eNewlinesReplaceWithCommas:
-   *          replace newlines with commas.
-   *        * nsIPlaintextEditor::eNewlinesStripSurroundingWhitespace:
-   *          collapse newlines and surrounding whitespace characters and
-   *          remove them from the string.
-   *        * nsIPlaintextEditor::eNewlinesPasteIntact:
-   *          only remove the leading and trailing newlines.
-   *        * nsIPlaintextEditor::eNewlinesPasteToFirst or any other value:
-   *          remove the first newline and all characters following it.
    */
-  static void HandleNewLines(nsString& aString, int32_t aNewLineHandling);
+  void HandleNewLines(nsString& aString);
 
   /**
    * Prepare a string buffer for being displayed as the contents of a password
    * field.  This function uses the platform-specific character for representing
    * characters entered into password fields.
    *
    * @param aOutString the output string.  When this function returns,
    *        aOutString will contain aLength password characters.
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -1109,16 +1109,18 @@ TextEditor::InsertParagraphSeparatorAsAc
     rv = rules->DidDoAction(selection, subActionInfo, rv);
   }
   return rv;
 }
 
 nsresult
 TextEditor::SetText(const nsAString& aString)
 {
+  MOZ_ASSERT(aString.FindChar(static_cast<char16_t>('\r')) == kNotFound);
+
   if (NS_WARN_IF(!mRules)) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   // Protect the edit rules object from dying
   RefPtr<TextEditRules> rules(mRules);
 
   // delete placeholder txns merge.