Bug 1364814 - Use RAII class to set and restore editor flags. r=masayuki
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Mon, 15 May 2017 13:22:25 +0900
changeset 358473 1a4d93b432d98329335ae4d242d79c85fb23879b
parent 358472 9d7ab545518c46e61ee6317d3bb363f62588038a
child 358474 27e007f58a3687c80fc3a4816fb7adcabfaab07f
push id90339
push userkwierso@gmail.com
push dateMon, 15 May 2017 23:34:45 +0000
treeherdermozilla-inbound@30813e40f36c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1364814
milestone55.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 1364814 - Use RAII class to set and restore editor flags. r=masayuki input.value setter removes editor flags and max-length to set value. To clean up code, I would like to use RAII class to set and restore it. Actually, when the frame is being destroyed by InsertText, it isn't restored. But it might be safe since the flags is set again on nsTextEditorState::PrepareEditor by focus. MozReview-Commit-ID: J0OYYluWD8z
dom/html/nsTextEditorState.cpp
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -128,16 +128,55 @@ public:
     mTextEditorState = nullptr;
   }
 
 private:
   nsTextControlFrame* mFrame;
   nsTextEditorState* mTextEditorState;
 };
 
+class MOZ_RAII AutoRestoreEditorState final
+{
+public:
+  explicit AutoRestoreEditorState(nsIEditor* aEditor,
+                                  nsIPlaintextEditor* aPlaintextEditor
+                                  MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+    : mEditor(aEditor)
+    , mPlaintextEditor(aPlaintextEditor)
+  {
+    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mEditor);
+    MOZ_ASSERT(mPlaintextEditor);
+
+    uint32_t flags;
+    mEditor->GetFlags(&mSavedFlags);
+    flags = mSavedFlags;
+    flags &= ~(nsIPlaintextEditor::eEditorDisabledMask);
+    flags &= ~(nsIPlaintextEditor::eEditorReadonlyMask);
+    flags |= nsIPlaintextEditor::eEditorDontEchoPassword;
+    mEditor->SetFlags(flags);
+
+    mPlaintextEditor->GetMaxTextLength(&mSavedMaxLength);
+    mPlaintextEditor->SetMaxTextLength(-1);
+  }
+
+  ~AutoRestoreEditorState()
+  {
+     mPlaintextEditor->SetMaxTextLength(mSavedMaxLength);
+     mEditor->SetFlags(mSavedFlags);
+  }
+
+private:
+  nsIEditor* mEditor;
+  nsIPlaintextEditor* mPlaintextEditor;
+  uint32_t mSavedFlags;
+  int32_t mSavedMaxLength;
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+};
+
 /*static*/
 bool
 nsITextControlElement::GetWrapPropertyEnum(nsIContent* aContent,
   nsITextControlElement::nsHTMLTextWrap& aWrapProp)
 {
   // soft is the default; "physical" defaults to soft as well because all other
   // browsers treat it that way and there is no real reason to maintain physical
   // and virtual as separate entities if no one else does.  Only hard and off
@@ -2546,18 +2585,19 @@ nsTextEditorState::SetValue(const nsAStr
 
         nsCOMPtr<nsISelection> domSel;
         nsCOMPtr<nsISelectionPrivate> selPriv;
         mSelCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
                               getter_AddRefs(domSel));
         if (domSel)
         {
           selPriv = do_QueryInterface(domSel);
-          if (selPriv)
+          if (selPriv) {
             selPriv->StartBatchChanges();
+          }
         }
 
         nsCOMPtr<nsISelectionController> kungFuDeathGrip = mSelCon.get();
         uint32_t currentLength = currentValue.Length();
         uint32_t newlength = newValue.Length();
         if (!currentLength ||
             !StringBeginsWith(newValue, currentValue)) {
           // Replace the whole text.
@@ -2572,44 +2612,35 @@ nsTextEditorState::SetValue(const nsAStr
         nsCOMPtr<nsIPlaintextEditor> plaintextEditor = do_QueryInterface(mEditor);
         if (!plaintextEditor || !weakFrame.IsAlive()) {
           NS_WARNING("Somehow not a plaintext editor?");
           return true;
         }
 
         valueSetter.Init();
 
-        // get the flags, remove readonly and disabled, set the value,
-        // restore flags
-        uint32_t flags, savedFlags;
-        mEditor->GetFlags(&savedFlags);
-        flags = savedFlags;
-        flags &= ~(nsIPlaintextEditor::eEditorDisabledMask);
-        flags &= ~(nsIPlaintextEditor::eEditorReadonlyMask);
-        flags |= nsIPlaintextEditor::eEditorDontEchoPassword;
-        mEditor->SetFlags(flags);
-
-        mTextListener->SettingValue(true);
-        bool notifyValueChanged = !!(aFlags & eSetValue_Notify);
-        mTextListener->SetValueChanged(notifyValueChanged);
-
-        // Also don't enforce max-length here
-        int32_t savedMaxLength;
-        plaintextEditor->GetMaxTextLength(&savedMaxLength);
-        plaintextEditor->SetMaxTextLength(-1);
-
-        if (insertValue.IsEmpty()) {
-          mEditor->DeleteSelection(nsIEditor::eNone, nsIEditor::eStrip);
-        } else {
-          plaintextEditor->InsertText(insertValue);
+        // get the flags, remove readonly, disabled and max-length,
+        // set the value, restore flags
+        {
+          AutoRestoreEditorState restoreState(mEditor, plaintextEditor);
+
+          mTextListener->SettingValue(true);
+          bool notifyValueChanged = !!(aFlags & eSetValue_Notify);
+          mTextListener->SetValueChanged(notifyValueChanged);
+
+          if (insertValue.IsEmpty()) {
+            mEditor->DeleteSelection(nsIEditor::eNone, nsIEditor::eStrip);
+          } else {
+            plaintextEditor->InsertText(insertValue);
+          }
+
+          mTextListener->SetValueChanged(true);
+          mTextListener->SettingValue(false);
         }
 
-        mTextListener->SetValueChanged(true);
-        mTextListener->SettingValue(false);
-
         if (!weakFrame.IsAlive()) {
           // If the frame was destroyed because of a flush somewhere inside
           // InsertText, mBoundFrame here will be false.  But it's also possible
           // for the frame to go away because of another reason (such as deleting
           // the existing selection -- see bug 574558), in which case we don't
           // need to reset the value here.
           if (!mBoundFrame) {
             return SetValue(newValue, aFlags & eSetValue_Notify);
@@ -2618,20 +2649,19 @@ nsTextEditorState::SetValue(const nsAStr
         }
 
         if (!IsSingleLineTextControl()) {
           if (!mCachedValue.Assign(newValue, fallible)) {
             return false;
           }
         }
 
-        plaintextEditor->SetMaxTextLength(savedMaxLength);
-        mEditor->SetFlags(savedFlags);
-        if (selPriv)
+        if (selPriv) {
           selPriv->EndBatchChanges();
+        }
       }
     }
   } else {
     if (!mValue) {
       mValue.emplace();
     }
 
     // We can't just early-return here if mValue->Equals(newValue), because