Clean up of SelectionModel

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

Index: chrome/browser/ui/views/omnibox/omnibox_view_views.cc
===================================================================
--- chrome/browser/ui/views/omnibox/omnibox_view_views.cc	(revision 103758)
+++ chrome/browser/ui/views/omnibox/omnibox_view_views.cc	(working copy)
@@ -237,7 +237,7 @@
       GetSelectionBounds(&start, &end);
       if (start != end || start < length) {
         OnBeforePossibleChange();
-        textfield_->SelectSelectionModel(gfx::SelectionModel(length, length));
+        SelectRange(length, length);
         OnAfterPossibleChange();
         handled = true;
       }
@@ -407,8 +407,7 @@
   if (start == string16::npos || (current_text[start] != '?')) {

[msw, 2011/10/04 07:25:41]

you can remove the braces for these conditional blocks.

[xji, 2011/10/05 01:23:42]

Done.

     SetUserText(ASCIIToUTF16("?"));
   } else {
-    textfield_->SelectSelectionModel(gfx::SelectionModel(current_text.size(),
-                                                         start + 1));
+    SelectRange(current_text.size(), start + 1);
   }
 }
 
@@ -423,17 +422,17 @@
 
 void OmniboxViewViews::GetSelectionBounds(string16::size_type* start,
                                           string16::size_type* end) {
-  gfx::SelectionModel sel;
-  textfield_->GetSelectionModel(&sel);
-  *start = static_cast<size_t>(sel.selection_end());
-  *end = static_cast<size_t>(sel.selection_start());
+  ui::Range range;
+  textfield_->GetSelectedRange(&range);
+  *start = static_cast<size_t>(range.end());
+  *end = static_cast<size_t>(range.start());
 }
 
 void OmniboxViewViews::SelectAll(bool reversed) {
   if (reversed)
-    textfield_->SelectSelectionModel(gfx::SelectionModel(GetTextLength(), 0));
+    SelectRange(GetTextLength(), 0);
   else
-    textfield_->SelectSelectionModel(gfx::SelectionModel(0, GetTextLength()));
+    SelectRange(0, GetTextLength());
 }
 
 void OmniboxViewViews::RevertAll() {
@@ -449,15 +448,12 @@
 
   // Don't inline autocomplete when the caret/selection isn't at the end of
   // the text, or in the middle of composition.
-  gfx::SelectionModel sel;
-  textfield_->GetSelectionModel(&sel);
-  size_t max_of_selection = std::max(sel.selection_start(),
-                                     sel.selection_end());
+  ui::Range sel;
+  textfield_->GetSelectedRange(&sel);
   bool no_inline_autocomplete =
-      max_of_selection < GetTextLength() || textfield_->IsIMEComposing();
+      sel.GetMax() < GetTextLength() || textfield_->IsIMEComposing();
 
-  bool is_sel_empty = (sel.selection_start() == sel.selection_end());
-  model_->StartAutocomplete(!is_sel_empty, no_inline_autocomplete);
+  model_->StartAutocomplete(!sel.is_empty(), no_inline_autocomplete);
 }
 
 void OmniboxViewViews::ClosePopup() {
@@ -472,12 +468,8 @@
 void OmniboxViewViews::OnTemporaryTextMaybeChanged(
     const string16& display_text,
     bool save_original_selection) {
-  if (save_original_selection) {
-    gfx::SelectionModel sel;
-    textfield_->GetSelectionModel(&sel);
-    saved_temporary_selection_.set_start(sel.selection_start());
-    saved_temporary_selection_.set_end(sel.selection_end());
-  }
+  if (save_original_selection)
+    textfield_->GetSelectedRange(&saved_temporary_selection_);
 
   SetWindowTextAndCaretPos(display_text, display_text.length());
   TextChanged();
@@ -495,28 +487,20 @@
 }
 
 void OmniboxViewViews::OnRevertTemporaryText() {
-  gfx::SelectionModel sel(saved_temporary_selection_.start(),
-                          saved_temporary_selection_.end());
-  textfield_->SelectSelectionModel(sel);
+  textfield_->SelectRange(saved_temporary_selection_);
   TextChanged();
 }
 
 void OmniboxViewViews::OnBeforePossibleChange() {
   // Record our state.
   text_before_change_ = GetText();
-  gfx::SelectionModel sel;
-  textfield_->GetSelectionModel(&sel);
-  sel_before_change_.set_start(sel.selection_start());
-  sel_before_change_.set_end(sel.selection_end());
+  textfield_->GetSelectedRange(&sel_before_change_);
   ime_composing_before_change_ = textfield_->IsIMEComposing();
 }
 
 bool OmniboxViewViews::OnAfterPossibleChange() {
-  gfx::SelectionModel sel;
-  textfield_->GetSelectionModel(&sel);
   ui::Range new_sel;
-  new_sel.set_start(sel.selection_start());
-  new_sel.set_end(sel.selection_end());
+  textfield_->GetSelectedRange(&new_sel);
 
   // See if the text or selection have changed since OnBeforePossibleChange().
   const string16 new_text = GetText();
@@ -697,8 +681,7 @@
                                                const ui::Range& range) {
   if (text != GetText())
     textfield_->SetText(text);
-  textfield_->SelectSelectionModel(gfx::SelectionModel(
-      range.start(), range.end()));
+  textfield_->SelectRange(range);
 }
 
 string16 OmniboxViewViews::GetSelectedText() const {
@@ -706,6 +689,10 @@
   return textfield_->GetSelectedText();
 }
 
+void OmniboxViewViews::SelectRange(size_t caret, size_t end) {
+  const ui::Range range(caret, end);
+  textfield_->SelectRange(range);

[msw, 2011/10/04 07:25:41]

You could optionally inline this without additional lines at each callsite via:
textfield_->SelectRange(ui::Range([start], [end]));
I usually wouldn't add a convenience function for something that convenient.

[xji, 2011/10/05 01:23:42]

Removed this convient function.
It was there originally, but I agree with you that it is not needed.

+}
 
 AutocompletePopupView* OmniboxViewViews::CreatePopupView(
     View* location_bar) {