Simplify handling of BiDi cursor movement

ui/gfx/selection_model.h

Index: ui/gfx/selection_model.h
diff --git a/ui/gfx/selection_model.h b/ui/gfx/selection_model.h
index 4ef3ca0f1f9014c6374602b9ad84aec4696b304d..f699e641033e16024131afa31741dcf26f934ca7 100644
--- a/ui/gfx/selection_model.h
+++ b/ui/gfx/selection_model.h
@@ -6,92 +6,80 @@
 #define UI_GFX_SELECTION_MODEL_H_
 #pragma once
 
-#include <stdlib.h>
+#include <iosfwd>
 
+#include "ui/base/range/range.h"
 #include "ui/base/ui_export.h"
 
 namespace gfx {
 
-// TODO(xji): publish bidi-editing guide line and replace the place holder.
-// SelectionModel is used to represent the logical selection and visual
-// position of cursor.
+// VisualCursorDirection and LogicalCursorDirection represent directions of
+// motion of the cursor in BiDi text. The combinations that make sense are:
 //
-// For bi-directional text, the mapping between visual position and logical
-// position is not one-to-one. For example, logical text "abcDEF" where capital
-// letters stand for Hebrew, the visual display is "abcFED". According to the
-// bidi editing guide (http://bidi-editing-guideline):
-// 1. If pointing to the right half of the cell of a LTR character, the current
-// position must be set after this character and the caret must be displayed
-// after this character.
-// 2. If pointing to the right half of the cell of a RTL character, the current
-// position must be set before this character and the caret must be displayed
-// before this character.
-//
-// Pointing to the right half of 'c' and pointing to the right half of 'D' both
-// set the logical cursor position to 3. But the cursor displayed visually at
-// different places:
-// Pointing to the right half of 'c' displays the cursor right of 'c' as
-// "abc|FED".
-// Pointing to the right half of 'D' displays the cursor right of 'D' as
-// "abcFED|".
-// So, besides the logical selection start point and end point, we need extra
-// information to specify to which character and on which edge of the character
-// the visual cursor is bound to. For example, the visual cursor is bound to
-// the trailing side of the 2nd character 'c' when pointing to right half of
-// 'c'. And it is bound to the leading edge of the 3rd character 'D' when
-// pointing to right of 'D'.
+//  base::i18n::TextDirection  VisualCursorDirection  LogicalCursorDirection
+//       LEFT_TO_RIGHT             CURSOR_LEFT           CURSOR_BACKWARD
+//       LEFT_TO_RIGHT             CURSOR_RIGHT          CURSOR_FORWARD
+//       RIGHT_TO_LEFT             CURSOR_RIGHT          CURSOR_BACKWARD
+//       RIGHT_TO_LEFT             CURSOR_LEFT           CURSOR_FORWARD
+enum VisualCursorDirection {
+  CURSOR_LEFT,
+  CURSOR_RIGHT
+};
+enum LogicalCursorDirection {
+  CURSOR_BACKWARD,
+  CURSOR_FORWARD
+};
+
 class UI_EXPORT SelectionModel {
  public:
-  enum CaretPlacement {
-    LEADING,
-    TRAILING,
-  };
-
+  // Create a default SelectionModel to be overwritten later.
   SelectionModel();
-  explicit SelectionModel(size_t pos);
-  SelectionModel(size_t end, size_t pos, CaretPlacement status);
-  SelectionModel(size_t start, size_t end, size_t pos, CaretPlacement status);
-
-  virtual ~SelectionModel();
-
-  size_t selection_start() const { return selection_start_; }
-  size_t selection_end() const { return selection_end_; }
-  size_t caret_pos() const { return caret_pos_; }
-  CaretPlacement caret_placement() const { return caret_placement_; }
-
-  bool Equals(const SelectionModel& sel) const;
+  // Create a SelectionModel representing a caret |position| without a
+  // selection. The |affinity| is meaningful only when the caret is positioned
+  // between bidi runs that are not visually contiguous: in that case, it
+  // indicates the run to which the caret is attached for display purposes.
+  SelectionModel(size_t position, LogicalCursorDirection affinity);
+  // Create a SelectionModel representing a selection (which may be empty).
+  // The caret position is the end of the range.
+  SelectionModel(ui::Range selection, LogicalCursorDirection affinity);
+
+  const ui::Range& selection() const { return selection_; }
+  size_t caret_pos() const { return selection_.end(); }
+  LogicalCursorDirection caret_affinity() const { return caret_affinity_; }
+
+  bool operator==(const SelectionModel& sel) const;
+  bool operator!=(const SelectionModel& sel) { return !(*this == sel); }
 
  private:
   friend class RenderText;
 
-  void Init(size_t start, size_t end, size_t pos, CaretPlacement status);
-
-  void set_selection_start(size_t pos) { selection_start_ = pos; }
-  void set_selection_end(size_t pos) { selection_end_ = pos; }
-  void set_caret_pos(size_t pos) { caret_pos_ = pos; }
-  void set_caret_placement(CaretPlacement placement) {
-    caret_placement_ = placement;
-  }
-
-  // Logical selection start. If there is non-empty selection, if
-  // selection_start_ is less than selection_end_, the selection starts visually
-  // at the leading edge of the selection_start_. If selection_start_ is greater
-  // than selection_end_, the selection starts visually at the trailing edge of
-  // selection_start_'s previous grapheme. So, we do not need extra information
-  // for visual bounding.
-  size_t selection_start_;
-
-  // The logical cursor position that next character will be inserted into.
-  // It is also the end of the selection.
-  size_t selection_end_;
-
-  // The following two fields are used to guide cursor visual position.
-  // The index of the character that cursor is visually attached to.
-  size_t caret_pos_;
-  // The visual placement of the cursor, relative to its associated character.
-  CaretPlacement caret_placement_;
+  void set_selection_start(size_t pos) { selection_.set_start(pos); }
+
+  // Logical selection. The logical caret position is the end of the selection.
+  ui::Range selection_;
+
+  // The logical direction from the caret position (selection_.end()) to the
+  // character it is attached to for display purposes. This matters only when
+  // the surrounding characters are not visually contiguous, which happens only
+  // in bidi text (and only at bidi run boundaries). The text is treated as
+  // though it was surrounded on both sides by runs in the dominant text
+  // direction. For example, supposing the dominant direction is LTR and the
+  // logical text is "abcDEF", where DEF is right-to-left text, the visual
+  // cursor will display as follows:
+  //    caret position    CURSOR_BACKWARD affinity    CURSOR_FORWARD affinity
+  //          0                  |abcFED                     |abcFED
+  //          1                  a|bcFED                     a|bcFED
+  //          2                  ab|cFED                     ab|cFED
+  //          3                  abc|FED                     abcFED|
+  //          4                  abcFE|D                     abcFE|D
+  //          5                  abcF|ED                     abcF|ED
+  //          6                  abc|FED                     abcFED|
+  LogicalCursorDirection caret_affinity_;
 };
 
+UI_EXPORT std::ostream& operator<<(std::ostream& out,

[Alexei Svitkine (slow), 2012/02/14 20:16:31]

I think the convention is to have a ToString() function. e.g. see
ui/gfx/range.h.

[benrg, 2012/02/14 23:39:02]

I looked at Range when writing this, and it defines operator<<. I see that
Insets, Point, Rect, and Size all define ToString instead. But when I tried
switching to ToString, render_text_unittest.cc complained about the missing
operator<<. What should I do? This overload is only here to make test failures
easier to read.

+                                   const SelectionModel& sel);
+
 }  // namespace gfx
 
 #endif  // UI_GFX_SELECTION_MODEL_H_