modification to RenderText for inheritance/SelectionModel

views/controls/textfield/textfield_views_model.cc

Index: views/controls/textfield/textfield_views_model.cc
===================================================================
--- views/controls/textfield/textfield_views_model.cc	(revision 93971)
+++ views/controls/textfield/textfield_views_model.cc	(working copy)
@@ -380,10 +380,11 @@
   render_text_->MoveCursorRight(break_type, select);
 }
 
-bool TextfieldViewsModel::MoveCursorTo(size_t pos, bool select) {
+bool TextfieldViewsModel::MoveCursorTo(const gfx::SelectionModel& selection,

[msw, 2011/08/01 05:02:23]

We'll need to reconsider this function similarly to the RenderText function,
since |selection| may conflict with |select|, and its behavior is unclear.

+                                       bool select) {
   if (HasCompositionText())
     ConfirmCompositionText();
-  return render_text_->MoveCursorTo(pos, select);
+  return render_text_->MoveCursorTo(selection, select);
 }
 
 bool TextfieldViewsModel::MoveCursorTo(const gfx::Point& point, bool select) {
@@ -393,22 +394,25 @@
 }
 
 std::vector<gfx::Rect> TextfieldViewsModel::GetSelectionBounds() const {
-  return render_text_->GetSubstringBounds(render_text_->GetSelection());
+  return render_text_->GetSubstringBounds(render_text_->GetSelectionStart(),
+                                          render_text_->GetCursorPosition());
 }
 
 string16 TextfieldViewsModel::GetSelectedText() const {
-  ui::Range selection = render_text_->GetSelection();
-  return GetText().substr(selection.GetMin(), selection.length());
+  return GetText().substr(render_text_->MinOfSelection(),
+      std::abs(static_cast<long>(render_text_->GetCursorPosition() -

[msw, 2011/08/01 05:02:23]

why the static cast to long? Shouldn't this be a size_t cast outside the
std::abs if needed?

[xji, 2011/08/01 23:38:42]

is (size_t - size_t) a size_t? then, you need to cast it to signed before
std::abs.

[msw, 2011/08/02 01:29:06]

The call to std::abs implicitly casts the subtraction result to a signed type,
and call to substr implicitly casts the std::abs result to an unsigned type; I
don't think any explicit casting is necessary.

However, my mentor recommends explicitly casting each of the render_text_ values
to longs (or ints) and then subtracting them (he seems to think this is more
readable).

This was a little easier with a ui::Range, but do as you please, I don't really
think it matters all that much.

+                                 render_text_->GetSelectionStart())));
 }
 
 void TextfieldViewsModel::GetSelectedRange(ui::Range* range) const {
-  *range = render_text_->GetSelection();
+  *range = ui::Range(render_text_->GetSelectionStart(),
+                     render_text_->GetCursorPosition());
 }
 
 void TextfieldViewsModel::SelectRange(const ui::Range& range) {
   if (HasCompositionText())
     ConfirmCompositionText();
-  render_text_->SetSelection(range);
+  render_text_->SetSelection(range.start(), range.end());
 }
 
 void TextfieldViewsModel::SelectAll() {
@@ -492,8 +496,8 @@
     // than beginning, unlike Delete/Backspace.
     // TODO(oshima): Change Delete/Backspace to use DeleteSelection,
     // update DeleteEdit and remove this trick.
-    ui::Range selection = render_text_->GetSelection();
-    render_text_->SetSelection(ui::Range(selection.end(), selection.start()));
+    render_text_->SetSelection(render_text_->GetCursorPosition(),
+                               render_text_->GetSelectionStart());
     DeleteSelection();
     return true;
   }
@@ -519,14 +523,14 @@
 }
 
 bool TextfieldViewsModel::HasSelection() const {
-  return !render_text_->GetSelection().is_empty();
+  return !render_text_->EmptySelection();
 }
 
 void TextfieldViewsModel::DeleteSelection() {
   DCHECK(!HasCompositionText());
   DCHECK(HasSelection());
-  ui::Range selection = render_text_->GetSelection();
-  ExecuteAndRecordDelete(selection.start(), selection.end(), false);
+  ExecuteAndRecordDelete(render_text_->GetSelectionStart(),
+                         render_text_->GetCursorPosition(), false);
 }
 
 void TextfieldViewsModel::DeleteSelectionAndInsertTextAt(
@@ -568,9 +572,9 @@
   // TODO(msw): Support multiple composition underline ranges.
 
   if (composition.selection.IsValid())
-    render_text_->SetSelection(ui::Range(
+    render_text_->SetSelection(
         std::min(range.start() + composition.selection.start(), range.end()),
-        std::min(range.start() + composition.selection.end(), range.end())));
+        std::min(range.start() + composition.selection.end(), range.end()));
   else
     render_text_->SetCursorPosition(range.end());
 }
@@ -640,7 +644,7 @@
     CancelCompositionText();
   } else if (!HasSelection()) {
     size_t cursor = GetCursorPosition();
-    render_text_->SetSelection(ui::Range(cursor + text.length(), cursor));
+    render_text_->SetSelection(cursor + text.length(), cursor);
   }
   // Edit history is recorded in InsertText.
   InsertTextInternal(text, mergeable);
@@ -682,7 +686,7 @@
 
 void TextfieldViewsModel::ExecuteAndRecordReplaceSelection(
     MergeType merge_type, const string16& new_text) {
-  size_t new_text_start = render_text_->GetSelection().GetMin();
+  size_t new_text_start = render_text_->MinOfSelection();
   size_t new_cursor_pos = new_text_start + new_text.length();
   ExecuteAndRecordReplace(merge_type,
                           GetCursorPosition(),
@@ -696,8 +700,9 @@
                                                   size_t new_cursor_pos,
                                                   const string16& new_text,
                                                   size_t new_text_start) {
-  size_t old_text_start = render_text_->GetSelection().GetMin();
-  bool backward = render_text_->GetSelection().is_reversed();
+  size_t old_text_start = render_text_->MinOfSelection();
+  bool backward = render_text_->GetSelectionStart() >
+                  render_text_->GetCursorPosition();
   Edit* edit = new ReplaceEdit(merge_type,
                                GetSelectedText(),
                                old_cursor_pos,