Simplify handling of BiDi cursor movement

ui/views/controls/textfield/native_textfield_views.cc

Index: ui/views/controls/textfield/native_textfield_views.cc
diff --git a/ui/views/controls/textfield/native_textfield_views.cc b/ui/views/controls/textfield/native_textfield_views.cc
index 8da7db6bf39db3ba702cd835aaffa34d8c5273d5..c606a1b1285407e5e77437db3c842282247d3df6 100644
--- a/ui/views/controls/textfield/native_textfield_views.cc
+++ b/ui/views/controls/textfield/native_textfield_views.cc
@@ -186,8 +186,7 @@ int NativeTextfieldViews::OnPerformDrop(const DropTargetEvent& event) {
   OnBeforeUserAction();
   skip_input_method_cancel_composition_ = true;
 
-  // TODO(msw): Remove final reference to FindCursorPosition.
-  gfx::SelectionModel drop_destination =
+  gfx::SelectionModel drop_destination_model =
       GetRenderText()->FindCursorPosition(event.location());
   string16 text;
   event.data().GetString(&text);
@@ -199,20 +198,12 @@ int NativeTextfieldViews::OnPerformDrop(const DropTargetEvent& event) {
     gfx::SelectionModel selected;
     model_->GetSelectionModel(&selected);
     // Adjust the drop destination if it is on or after the current selection.
-    size_t max_of_selected_range = std::max(selected.selection_start(),
-                                            selected.selection_end());
-    size_t min_of_selected_range = std::min(selected.selection_start(),
-                                            selected.selection_end());
-    size_t selected_range_length = max_of_selected_range -
-                                   min_of_selected_range;
-    size_t drop_destination_end = drop_destination.selection_end();
-    if (max_of_selected_range <= drop_destination_end)
-      drop_destination_end -= selected_range_length;
-    else if (min_of_selected_range <= drop_destination_end)
-      drop_destination_end = min_of_selected_range;
-    model_->DeleteSelectionAndInsertTextAt(text, drop_destination_end);
+    size_t drop_destination = drop_destination_model.caret_pos();
+    drop_destination -=
+        selected.selection().Intersect(ui::Range(0, drop_destination)).length();
+    model_->DeleteSelectionAndInsertTextAt(text, drop_destination);
   } else {
-    model_->MoveCursorTo(drop_destination);
+    model_->MoveCursorTo(drop_destination_model);
     // Drop always inserts text even if the textfield is not in insert mode.
     model_->InsertText(text);
   }
@@ -248,19 +239,12 @@ void NativeTextfieldViews::SelectRect(const gfx::Point& start,
   if (GetTextInputType() == ui::TEXT_INPUT_TYPE_NONE)
     return;
 
-  gfx::SelectionModel start_pos = GetRenderText()->FindCursorPosition(start);
-  gfx::SelectionModel end_pos = GetRenderText()->FindCursorPosition(end);
+  gfx::SelectionModel selection = GetRenderText()->FindCursorPosition(end);
+  selection.set_selection_start(

[msw, 2012/02/21 21:13:39]

I agree with xji's reasoning for keeping SelectionModel::set_selection_start()
private. Change this back to your Patch Set 2 version; sorry for the bad tip.

[benrg, 2012/02/21 23:21:45]

I rewrote it to no longer use set_selection_start, but I didn't restore the more
complicated old logic (which I think  was unnecessary -- see earlier comment).
See also my response to xji.

+      GetRenderText()->FindCursorPosition(start).caret_pos());
 
   OnBeforeUserAction();
-  // Merge selection models of "start_pos" and "end_pos" so that
-  // selection start is the value from "start_pos", while selection end,
-  // caret position, and caret placement are values from "end_pos".
-  if (start_pos.selection_start() == end_pos.selection_end())
-    model_->SelectSelectionModel(end_pos);
-  else
-    model_->SelectRange(ui::Range(start_pos.selection_start(),
-                                  end_pos.selection_end()));
-
+  model_->SelectSelectionModel(selection);
   OnCaretBoundsChanged();
   SchedulePaint();
   OnAfterUserAction();
@@ -464,7 +448,7 @@ bool NativeTextfieldViews::IsIMEComposing() const {
 }
 
 void NativeTextfieldViews::GetSelectedRange(ui::Range* range) const {
-  model_->GetSelectedRange(range);
+  *range = GetRenderText()->selection();
 }
 
 void NativeTextfieldViews::SelectRange(const ui::Range& range) {
@@ -738,8 +722,7 @@ bool NativeTextfieldViews::GetSelectionRange(ui::Range* range) {
 
   gfx::SelectionModel sel;
   model_->GetSelectionModel(&sel);
-  range->set_start(sel.selection_start());
-  range->set_end(sel.selection_end());
+  *range = sel.selection();
   return true;
 }