Bug 1343037 part 9. Simplify the setup around the editor state's GetSelectionDirection function. r=ehsan
☠☠ backed out by f584babc1167 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 09 Mar 2017 14:44:05 -0500
changeset 397294 5fc2e304113b49b08c2027ac066a34c1d604713c
parent 397293 b7de9c9c1c31917e36a39fc96fc639d0dd6593af
child 397295 73198c9c3975dd4a292edd1849a4682e57302e4c
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1343037
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 1343037 part 9. Simplify the setup around the editor state's GetSelectionDirection function. r=ehsan Really, there are only two cases we need to worry about. Either IsSelectionCached(), and then our SelectionProperties has the data we want, or not and then we have a non-null mSelCon which has the data we want. MozReview-Commit-ID: AEW9D1zG6sM
dom/html/HTMLInputElement.cpp
dom/html/HTMLTextAreaElement.cpp
dom/html/nsTextEditorState.cpp
dom/html/nsTextEditorState.h
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@@ -6566,39 +6566,25 @@ DirectionToName(nsITextControlFrame::Sel
 void
 HTMLInputElement::GetSelectionDirection(nsAString& aDirection, ErrorResult& aRv)
 {
   if (!SupportsTextSelection()) {
     aDirection.SetIsVoid(true);
     return;
   }
 
-  nsIFormControlFrame* formControlFrame = GetFormControlFrame(true);
   nsTextEditorState* state = GetEditorState();
-  if (!state) {
-    aRv.Throw(NS_ERROR_FAILURE);
+  MOZ_ASSERT(state, "SupportsTextSelection came back true!");
+  nsITextControlFrame::SelectionDirection dir =
+    state->GetSelectionDirection(aRv);
+  if (aRv.Failed()) {
     return;
   }
-
-  nsresult rv = NS_ERROR_FAILURE;
-  if (formControlFrame) {
-    nsITextControlFrame::SelectionDirection dir;
-    rv = state->GetSelectionDirection(&dir);
-    if (NS_SUCCEEDED(rv)) {
-      DirectionToName(dir, aDirection);
-      return;
-    }
-  }
   
-  if (state->IsSelectionCached()) {
-    DirectionToName(state->GetSelectionProperties().GetDirection(), aDirection);
-    return;
-  }
-
-  aRv.Throw(rv);
+  DirectionToName(dir, aDirection);
 }
 
 void
 HTMLInputElement::SetSelectionDirection(const nsAString& aDirection, ErrorResult& aRv)
 {
   if (!SupportsTextSelection()) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
--- a/dom/html/HTMLTextAreaElement.cpp
+++ b/dom/html/HTMLTextAreaElement.cpp
@@ -802,33 +802,22 @@ DirectionToName(nsITextControlFrame::Sel
   } else {
     NS_NOTREACHED("Invalid SelectionDirection value");
   }
 }
 
 void
 HTMLTextAreaElement::GetSelectionDirection(nsAString& aDirection, ErrorResult& aError)
 {
-  nsresult rv = NS_ERROR_FAILURE;
-  nsIFormControlFrame* formControlFrame = GetFormControlFrame(true);
-  if (formControlFrame) {
-    nsITextControlFrame::SelectionDirection dir;
-    rv = mState.GetSelectionDirection(&dir);
-    if (NS_SUCCEEDED(rv)) {
-      DirectionToName(dir, aDirection);
-      return;
-    }
-  }
-
-  if (mState.IsSelectionCached()) {
-    DirectionToName(mState.GetSelectionProperties().GetDirection(), aDirection);
+  nsITextControlFrame::SelectionDirection dir =
+    mState.GetSelectionDirection(aError);
+  if (aError.Failed()) {
     return;
   }
-
-  aError.Throw(rv);
+  DirectionToName(dir, aDirection);
 }
 
 void
 HTMLTextAreaElement::SetSelectionDirection(const nsAString& aDirection,
                                            ErrorResult& aError)
 {
   if (mState.IsSelectionCached()) {
     nsITextControlFrame::SelectionDirection dir = nsITextControlFrame::eNone;
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -1603,49 +1603,51 @@ nsTextEditorState::GetSelectionRange(int
   if (NS_WARN_IF(!root)) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
   nsContentUtils::GetSelectionInTextControl(sel, root,
                                             *aSelectionStart, *aSelectionEnd);
 }
 
-nsresult
-nsTextEditorState::GetSelectionDirection(nsITextControlFrame::SelectionDirection* aDirection)
+nsITextControlFrame::SelectionDirection
+nsTextEditorState::GetSelectionDirection(ErrorResult& aRv)
 {
-  MOZ_ASSERT(mBoundFrame,
-             "Caller didn't flush out frames and check for a frame?");
-  MOZ_ASSERT(aDirection);
+  MOZ_ASSERT(IsSelectionCached() || GetSelectionController(),
+             "How can we not have a cached selection if we have no selection "
+             "controller?");
 
-  // It's not clear that all the checks here are needed, but the previous
-  // version of this code in nsTextControlFrame was doing them, so we keep them
-  // for now.
-
-  nsresult rv = mBoundFrame->EnsureEditorInitialized();
-  NS_ENSURE_SUCCESS(rv, rv);
+  // Note that we may have both IsSelectionCached() _and_
+  // GetSelectionController() if we haven't initialized our editor yet.
+  if (IsSelectionCached()) {
+    return GetSelectionProperties().GetDirection();
+  }
 
   nsISelectionController* selCon = GetSelectionController();
-  NS_ENSURE_TRUE(selCon, NS_ERROR_FAILURE);
 
   nsCOMPtr<nsISelection> selection;
-  rv = selCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
-                            getter_AddRefs(selection));
-  NS_ENSURE_SUCCESS(rv, rv);
-  NS_ENSURE_TRUE(selection, NS_ERROR_FAILURE);
+  nsresult rv = selCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
+                                     getter_AddRefs(selection));
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    aRv.Throw(rv);
+    return nsITextControlFrame::eForward; // Doesn't really matter
+  }
+  if (NS_WARN_IF(!selection)) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return nsITextControlFrame::eForward; // Doesn't really matter
+  }
 
   dom::Selection* sel = selection->AsSelection();
   nsDirection direction = sel->GetSelectionDirection();
   if (direction == eDirNext) {
-    *aDirection = nsITextControlFrame::eForward;
-  } else if (direction == eDirPrevious) {
-    *aDirection = nsITextControlFrame::eBackward;
-  } else {
-    NS_NOTREACHED("Invalid nsDirection enum value");
+    return nsITextControlFrame::eForward;
   }
-  return NS_OK;
+
+  MOZ_ASSERT(direction == eDirPrevious);
+  return nsITextControlFrame::eBackward;
 }
 
 HTMLInputElement*
 nsTextEditorState::GetParentNumberControl(nsFrame* aFrame) const
 {
   MOZ_ASSERT(aFrame);
   nsIContent* content = aFrame->GetContent();
   MOZ_ASSERT(content);
@@ -1716,21 +1718,23 @@ nsTextEditorState::UnbindFromFrame(nsTex
   // Save our selection state if needed.
   // Note that GetSelectionRange will attempt to work with our selection
   // controller, so we should make sure we do it before we start doing things
   // like destroying our editor (if we have one), tearing down the selection
   // controller, and so forth.
   if (!IsSelectionCached()) {
     // Go ahead and cache it now.
     int32_t start = 0, end = 0;
-    nsITextControlFrame::SelectionDirection direction =
-      nsITextControlFrame::eForward;
     IgnoredErrorResult rangeRv;
     GetSelectionRange(&start, &end, rangeRv);
-    GetSelectionDirection(&direction);
+
+    IgnoredErrorResult dirRv;
+    nsITextControlFrame::SelectionDirection direction =
+      GetSelectionDirection(dirRv);
+
     MOZ_ASSERT(aFrame == mBoundFrame);
     SelectionProperties& props = GetSelectionProperties();
     props.SetStart(start);
     props.SetEnd(end);
     props.SetDirection(direction);
     HTMLInputElement* number = GetParentNumberControl(aFrame);
     if (number) {
       // If we are inside a number control, cache the selection on the
--- a/dom/html/nsTextEditorState.h
+++ b/dom/html/nsTextEditorState.h
@@ -273,17 +273,18 @@ public:
   // selection state may be at the moment.
   void SyncUpSelectionPropertiesBeforeDestruction();
 
   // Get the selection range start and end points in our text.
   void GetSelectionRange(int32_t* aSelectionStart, int32_t* aSelectionEnd,
                          mozilla::ErrorResult& aRv);
 
   // Get the selection direction
-  nsresult GetSelectionDirection(nsITextControlFrame::SelectionDirection* aDirection);
+  nsITextControlFrame::SelectionDirection
+    GetSelectionDirection(mozilla::ErrorResult& aRv);
 
   void UpdateEditableState(bool aNotify) {
     if (mRootNode) {
       mRootNode->UpdateEditableState(aNotify);
     }
   }
 
 private: