Bug 1393348 - part1: nsTextEditorState should use nsTextInputSelectionImpl::GetSelection(SelectionType) instead of nsTextInputSelectionImpl::GetSelection(RawSelectionType, nsISelection**) r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 24 Aug 2017 16:19:28 +0900
changeset 653544 a1ce888b1710d2f18d214d825b7f9ecd0e4a8108
parent 653538 31465a03c03d1eec31cd4dd5d6b803724dcb29cd
child 653545 8b6a73ca698da09867071bdf9ee7a7ef8fab783b
push id76339
push usermasayuki@d-toybox.com
push dateSat, 26 Aug 2017 02:22:48 +0000
reviewersm_kato
bugs1393348
milestone57.0a1
Bug 1393348 - part1: nsTextEditorState should use nsTextInputSelectionImpl::GetSelection(SelectionType) instead of nsTextInputSelectionImpl::GetSelection(RawSelectionType, nsISelection**) r?m_kato nsTextEditorState stores selection controller as RefPtr<nsTextInputSelectionImpl> mSelCon. However, some methods still use nsTextInputSelectionImpl::GetSelection(RawSelectionType, nsISelection**) which is a virtual method overriding nsISelectionController. So, instead, we should make it use nsTextInputSelectionImpl::GetSelection(SelectionType). MozReview-Commit-ID: Cvxa85LegsO
dom/html/nsTextEditorState.cpp
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -499,20 +499,21 @@ nsTextInputSelectionImpl::SetCaretReadOn
 {
   if (!mPresShellWeak) return NS_ERROR_NOT_INITIALIZED;
   nsresult result;
   nsCOMPtr<nsIPresShell> shell = do_QueryReferent(mPresShellWeak, &result);
   if (shell)
   {
     RefPtr<nsCaret> caret = shell->GetCaret();
     if (caret) {
-      nsISelection* domSel =
+      Selection* selection =
         mFrameSelection->GetSelection(SelectionType::eNormal);
-      if (domSel)
+      if (selection) {
         caret->SetCaretReadOnly(aReadOnly);
+      }
       return NS_OK;
     }
   }
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsTextInputSelectionImpl::GetCaretEnabled(bool *_retval)
@@ -542,20 +543,21 @@ nsTextInputSelectionImpl::SetCaretVisibi
 {
   if (!mPresShellWeak) return NS_ERROR_NOT_INITIALIZED;
   nsresult result;
   nsCOMPtr<nsIPresShell> shell = do_QueryReferent(mPresShellWeak, &result);
   if (shell)
   {
     RefPtr<nsCaret> caret = shell->GetCaret();
     if (caret) {
-      nsISelection* domSel =
+      Selection* selection =
         mFrameSelection->GetSelection(SelectionType::eNormal);
-      if (domSel)
+      if (selection) {
         caret->SetVisibilityDuringSelection(aVisibility);
+      }
       return NS_OK;
     }
   }
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsTextInputSelectionImpl::PhysicalMove(int16_t aDirection, int16_t aAmount,
@@ -1333,32 +1335,26 @@ nsTextEditorState::BindToFrame(nsTextCon
   mSelCon = new nsTextInputSelectionImpl(frameSel, shell, rootNode);
   MOZ_ASSERT(!mTextListener, "Should not overwrite the object");
   mTextListener = new nsTextInputListener(mTextCtrlElement);
 
   mTextListener->SetFrame(mBoundFrame);
   mSelCon->SetDisplaySelection(nsISelectionController::SELECTION_ON);
 
   // Get the caret and make it a selection listener.
-  RefPtr<nsISelection> domSelection;
-  if (NS_SUCCEEDED(mSelCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
-                                         getter_AddRefs(domSelection))) &&
-      domSelection) {
-    nsCOMPtr<nsISelectionPrivate> selPriv(do_QueryInterface(domSelection));
+  // FYI: It's safe to use raw pointer for calling
+  //      Selection::AddSelectionListner() because it only appends the listener
+  //      to its internal array.
+  Selection* selection = mSelCon->GetSelection(SelectionType::eNormal);
+  if (selection) {
     RefPtr<nsCaret> caret = shell->GetCaret();
-    nsCOMPtr<nsISelectionListener> listener;
     if (caret) {
-      listener = do_QueryInterface(caret);
-      if (listener) {
-        selPriv->AddSelectionListener(listener);
-      }
+      selection->AddSelectionListener(caret);
     }
-
-    selPriv->AddSelectionListener(static_cast<nsISelectionListener*>
-                                             (mTextListener));
+    selection->AddSelectionListener(mTextListener);
   }
 
   // If an editor exists from before, prepare it for usage
   if (mTextEditor) {
     nsCOMPtr<nsIContent> content = do_QueryInterface(mTextCtrlElement);
     NS_ENSURE_TRUE(content, NS_ERROR_FAILURE);
 
     // Set the correct direction on the newly created root node
@@ -2203,24 +2199,23 @@ nsTextEditorState::UnbindFromFrame(nsTex
           }
         }
       }
     }
   }
 
   if (mSelCon) {
     if (mTextListener) {
-      RefPtr<nsISelection> domSelection;
-      if (NS_SUCCEEDED(mSelCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
-                                             getter_AddRefs(domSelection))) &&
-          domSelection) {
-        nsCOMPtr<nsISelectionPrivate> selPriv(do_QueryInterface(domSelection));
-
-        selPriv->RemoveSelectionListener(static_cast<nsISelectionListener*>
-                                         (mTextListener));
+      // FYI: It's safe to use raw pointer for calling
+      //      Selection::RemoveSelectionListener() because it only removes the
+      //      listener from its array.
+      Selection* selection =
+        mSelCon->GetSelection(SelectionType::eNormal);
+      if (selection) {
+        selection->RemoveSelectionListener(mTextListener);
       }
     }
 
     mSelCon->SetScrollableFrame(nullptr);
     mSelCon = nullptr;
   }
 
   if (mTextListener)
@@ -2642,21 +2637,21 @@ nsTextEditorState::SetValue(const nsAStr
       }
 
       // Time to mess with our security context... See comments in GetValue()
       // for why this is needed.  Note that we have to do this up here, because
       // otherwise SelectAll() will fail.
       {
         AutoNoJSAPI nojsapi;
 
-        nsCOMPtr<nsISelection> domSel;
-        mSelCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
-                              getter_AddRefs(domSel));
-        SelectionBatcher selectionBatcher(domSel ? domSel->AsSelection() :
-                                                   nullptr);
+        // FYI: It's safe to use raw pointer for selection here because
+        //      SelectionBatcher will grab it with RefPtr.
+        Selection* selection =
+          mSelCon->GetSelection(SelectionType::eNormal);
+        SelectionBatcher selectionBatcher(selection);
 
         if (NS_WARN_IF(!weakFrame.IsAlive())) {
           return true;
         }
 
         valueSetter.Init();
 
         // get the flags, remove readonly, disabled and max-length,
@@ -2689,20 +2684,20 @@ nsTextEditorState::SetValue(const nsAStr
 
             if (insertValue.IsEmpty()) {
               textEditor->DeleteSelection(nsIEditor::eNone, nsIEditor::eStrip);
             } else {
               textEditor->InsertText(insertValue);
             }
           } else {
             AutoDisableUndo disableUndo(textEditor);
-            if (domSel) {
+            if (selection) {
               // Since we don't use undo transaction, we don't need to store
               // selection state.  SetText will set selection to tail.
-              domSel->RemoveAllRanges();
+              selection->RemoveAllRanges();
             }
 
             textEditor->SetText(newValue);
 
             // Call the listener's HandleValueChanged() callback manually, since
             // we don't use the transaction manager in this path and it could be
             // that the editor would bypass calling the listener for that reason.
             mTextListener->HandleValueChanged();