Extract a TextRange class for selection (flutter#21722)

Extracts a TextRange class with a base and extent, and start(), end(),
collapsed(), and length() getters.

The possibility of reversed base and extent in selections and composing
ranges makes reasoning about them complex and increases the chances of
errors in the code. This change migrates most uses of base and extent in
the text model to start()/end() or position(). The position() method is
intended purely as an aid to readability to indicate that a collapsed
selection is expected at the call site; it also enforces a debug-time
assertion that this is the case.

Author: cbracken

shell/platform/common/cpp/BUILD.gn

@@ -37,14 +37,19 @@ copy("publish_headers") {
 }
 
 source_set("common_cpp_input") {
-  public = [ "text_input_model.h" ]
+  public = [
+    "text_input_model.h",
+    "text_range.h",
+  ]
 
   sources = [ "text_input_model.cc" ]
 
   configs += [ ":desktop_library_implementation" ]
 
   public_configs = [ "//flutter:config" ]
 
+  deps = [ "//flutter/fml:fml" ]
+
   if (is_win) {
     # For wstring_conversion. See issue #50053.
     defines = [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ]
@@ -137,6 +142,7 @@ if (enable_unittests) {
       "json_message_codec_unittests.cc",
       "json_method_codec_unittests.cc",
       "text_input_model_unittests.cc",
+      "text_range_unittests.cc",
     ]
 
     deps = [

shell/platform/common/cpp/text_input_model.cc

@@ -38,27 +38,24 @@ void TextInputModel::SetText(const std::string& text) {
   std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>
       utf16_converter;
   text_ = utf16_converter.from_bytes(text);
-  selection_base_ = 0;
-  selection_extent_ = 0;
+  selection_ = TextRange(0);
 }
 
 bool TextInputModel::SetSelection(size_t base, size_t extent) {
-  auto max_pos = text_.length();
+  size_t max_pos = text_.length();
   if (base > max_pos || extent > max_pos) {
     return false;
   }
-  selection_base_ = base;
-  selection_extent_ = extent;
+  selection_ = TextRange(base, extent);
   return true;
 }
 
 bool TextInputModel::DeleteSelected() {
-  if (selection_base_ == selection_extent_) {
+  if (selection_.collapsed()) {
     return false;
   }
-  text_.erase(selection_start(), selection_end() - selection_start());
-  selection_base_ = selection_start();
-  selection_extent_ = selection_base_;
+  text_.erase(selection_.start(), selection_.length());
+  selection_ = TextRange(selection_.start());
   return true;
 }
 
@@ -78,9 +75,9 @@ void TextInputModel::AddCodePoint(char32_t c) {
 
 void TextInputModel::AddText(const std::u16string& text) {
   DeleteSelected();
-  text_.insert(selection_extent_, text);
-  selection_extent_ += text.length();
-  selection_base_ = selection_extent_;
+  size_t position = selection_.position();
+  text_.insert(position, text);
+  selection_ = TextRange(position + text.length());
 }
 
 void TextInputModel::AddText(const std::string& text) {
@@ -90,38 +87,36 @@ void TextInputModel::AddText(const std::string& text) {
 }
 
 bool TextInputModel::Backspace() {
-  // If there's a selection, delete it.
   if (DeleteSelected()) {
     return true;
   }
-  // There's no selection; delete the preceding codepoint.
-  if (selection_base_ != 0) {
-    int count = IsTrailingSurrogate(text_.at(selection_base_ - 1)) ? 2 : 1;
-    text_.erase(selection_base_ - count, count);
-    selection_base_ -= count;
-    selection_extent_ = selection_base_;
+  // If there's no selection, delete the preceding codepoint.
+  size_t position = selection_.position();
+  if (position != 0) {
+    int count = IsTrailingSurrogate(text_.at(position - 1)) ? 2 : 1;
+    text_.erase(position - count, count);
+    selection_ = TextRange(position - count);
     return true;
   }
   return false;
 }
 
 bool TextInputModel::Delete() {
-  // If there's a selection, delete it.
   if (DeleteSelected()) {
     return true;
   }
-  // There's no selection; delete the following codepoint.
-  if (selection_base_ != text_.length()) {
-    int count = IsLeadingSurrogate(text_.at(selection_base_)) ? 2 : 1;
-    text_.erase(selection_base_, count);
-    selection_extent_ = selection_base_;
+  // If there's no selection, delete the preceding codepoint.
+  size_t position = selection_.position();
+  if (position != text_.length()) {
+    int count = IsLeadingSurrogate(text_.at(position)) ? 2 : 1;
+    text_.erase(position, count);
     return true;
   }
   return false;
 }
 
 bool TextInputModel::DeleteSurrounding(int offset_from_cursor, int count) {
-  auto start = selection_extent_;
+  size_t start = selection_.extent();
   if (offset_from_cursor < 0) {
     for (int i = 0; i < -offset_from_cursor; i++) {
       // If requested start is before the available text then reduce the
@@ -150,65 +145,53 @@ bool TextInputModel::DeleteSurrounding(int offset_from_cursor, int count) {
   text_.erase(start, end - start);
 
   // Cursor moves only if deleted area is before it.
-  if (offset_from_cursor <= 0) {
-    selection_base_ = start;
-  }
-
-  // Clear selection.
-  selection_extent_ = selection_base_;
+  selection_ = TextRange(offset_from_cursor <= 0 ? start : selection_.start());
 
   return true;
 }
 
 bool TextInputModel::MoveCursorToBeginning() {
-  if (selection_base_ == 0 && selection_extent_ == 0)
+  if (selection_.collapsed() && selection_.position() == 0)
     return false;
-
-  selection_base_ = 0;
-  selection_extent_ = 0;
+  selection_ = TextRange(0);
   return true;
 }
 
 bool TextInputModel::MoveCursorToEnd() {
-  auto max_pos = text_.length();
-  if (selection_base_ == max_pos && selection_extent_ == max_pos)
+  size_t max_pos = text_.length();
+  if (selection_.collapsed() && selection_.position() == max_pos)
     return false;
-
-  selection_base_ = max_pos;
-  selection_extent_ = max_pos;
+  selection_ = TextRange(max_pos);
   return true;
 }
 
 bool TextInputModel::MoveCursorForward() {
-  // If about to move set to the end of the highlight (when not selecting).
-  if (selection_base_ != selection_extent_) {
-    selection_base_ = selection_end();
-    selection_extent_ = selection_base_;
+  // If there's a selection, move to the end of the selection.
+  if (!selection_.collapsed()) {
+    selection_ = TextRange(selection_.end());
     return true;
   }
-  // If not at the end, move the extent forward.
-  if (selection_extent_ != text_.length()) {
-    int count = IsLeadingSurrogate(text_.at(selection_base_)) ? 2 : 1;
-    selection_base_ += count;
-    selection_extent_ = selection_base_;
+  // Otherwise, move the cursor forward.
+  size_t position = selection_.position();
+  if (position != text_.length()) {
+    int count = IsLeadingSurrogate(text_.at(position)) ? 2 : 1;
+    selection_ = TextRange(position + count);
     return true;
   }
   return false;
 }
 
 bool TextInputModel::MoveCursorBack() {
-  // If about to move set to the beginning of the highlight
-  // (when not selecting).
-  if (selection_base_ != selection_extent_) {
-    selection_base_ = selection_start();
-    selection_extent_ = selection_base_;
+  // If there's a selection, move to the beginning of the selection.
+  if (!selection_.collapsed()) {
+    selection_ = TextRange(selection_.start());
     return true;
   }
-  // If not at the start, move the beginning backward.
-  if (selection_base_ != 0) {
-    int count = IsTrailingSurrogate(text_.at(selection_base_ - 1)) ? 2 : 1;
-    selection_base_ -= count;
-    selection_extent_ = selection_base_;
+  // Otherwise, move the cursor backward.
+  size_t position = selection_.position();
+  if (position != 0) {
+    int count = IsTrailingSurrogate(text_.at(position - 1)) ? 2 : 1;
+    selection_ = TextRange(position - count);
     return true;
   }
   return false;
@@ -221,9 +204,9 @@ std::string TextInputModel::GetText() const {
 }
 
 int TextInputModel::GetCursorOffset() const {
-  // Measure the length of the current text up to the cursor.
+  // Measure the length of the current text up to the selection extent.
   // There is probably a much more efficient way of doing this.
-  auto leading_text = text_.substr(0, selection_extent_);
+  auto leading_text = text_.substr(0, selection_.extent());
   std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>
       utf8_converter;
   return utf8_converter.to_bytes(leading_text).size();

shell/platform/common/cpp/text_input_model.h

@@ -5,11 +5,13 @@
 #ifndef FLUTTER_SHELL_PLATFORM_CPP_TEXT_INPUT_MODEL_H_
 #define FLUTTER_SHELL_PLATFORM_CPP_TEXT_INPUT_MODEL_H_
 
-#include <algorithm>
 #include <memory>
 #include <string>
 
+#include "flutter/shell/platform/common/cpp/text_range.h"
+
 namespace flutter {
+
 // Handles underlying text input state, using a simple ASCII model.
 //
 // Ignores special states like "insert mode" for now.
@@ -104,10 +106,10 @@ class TextInputModel {
   int GetCursorOffset() const;
 
   // The position where the selection starts.
-  int selection_base() const { return selection_base_; }
+  int selection_base() const { return selection_.base(); }
 
   // The position of the cursor.
-  int selection_extent() const { return selection_extent_; }
+  int selection_extent() const { return selection_.extent(); }
 
  private:
   // Deletes the current selection, if any.
@@ -117,18 +119,7 @@ class TextInputModel {
   bool DeleteSelected();
 
   std::u16string text_;
-  size_t selection_base_ = 0;
-  size_t selection_extent_ = 0;
-
-  // Returns the left hand side of the selection.
-  size_t selection_start() const {
-    return std::min(selection_base_, selection_extent_);
-  }
-
-  // Returns the right hand side of the selection.
-  size_t selection_end() const {
-    return std::max(selection_base_, selection_extent_);
-  }
+  TextRange selection_ = TextRange(0);
 };
 
 }  // namespace flutter

shell/platform/common/cpp/text_range.h

@@ -0,0 +1,53 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <algorithm>
+
+#include "flutter/fml/logging.h"
+
+// A directional range of text.
+//
+// A |TextRange| describes a range of text with |base| and |extent| positions.
+// In the case where |base| == |extent|, the range is said to be collapsed, and
+// when |base| > |extent|, the range is said to be reversed.
+class TextRange {
+ public:
+  explicit TextRange(size_t position) : base_(position), extent_(position) {}
+  explicit TextRange(size_t base, size_t extent)
+      : base_(base), extent_(extent) {}
+  TextRange(const TextRange&) = default;
+  TextRange& operator=(const TextRange&) = default;
+
+  virtual ~TextRange() = default;
+
+  // Returns the base position of the range.
+  size_t base() const { return base_; }
+
+  // Returns the extent position of the range.
+  size_t extent() const { return extent_; }
+
+  // Returns the lesser of the base and extent positions.
+  size_t start() const { return std::min(base_, extent_); }
+
+  // Returns the greater of the base and extent positions.
+  size_t end() const { return std::max(base_, extent_); }
+
+  // Returns the position of a collapsed range.
+  //
+  // Asserts that the range is of length 0.
+  size_t position() const {
+    FML_DCHECK(base_ == extent_);
+    return extent_;
+  }
+
+  // Returns the length of the range.
+  size_t length() const { return end() - start(); }
+
+  // Returns true if the range is of length 0.
+  bool collapsed() const { return base_ == extent_; }
+
+ private:
+  size_t base_;
+  size_t extent_;
+};

shell/platform/common/cpp/text_range_unittests.cc

@@ -0,0 +1,53 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/platform/common/cpp/text_range.h"
+
+#include "gtest/gtest.h"
+
+namespace flutter {
+
+TEST(TextRange, TextRangeFromPositionZero) {
+  TextRange range(0);
+  EXPECT_EQ(range.base(), size_t(0));
+  EXPECT_EQ(range.extent(), size_t(0));
+  EXPECT_EQ(range.start(), size_t(0));
+  EXPECT_EQ(range.end(), size_t(0));
+  EXPECT_EQ(range.length(), size_t(0));
+  EXPECT_EQ(range.position(), size_t(0));
+  EXPECT_TRUE(range.collapsed());
+}
+
+TEST(TextRange, TextRangeFromPositionNonZero) {
+  TextRange range(3);
+  EXPECT_EQ(range.base(), size_t(3));
+  EXPECT_EQ(range.extent(), size_t(3));
+  EXPECT_EQ(range.start(), size_t(3));
+  EXPECT_EQ(range.end(), size_t(3));
+  EXPECT_EQ(range.length(), size_t(0));
+  EXPECT_EQ(range.position(), size_t(3));
+  EXPECT_TRUE(range.collapsed());
+}
+
+TEST(TextRange, TextRangeFromRange) {
+  TextRange range(3, 7);
+  EXPECT_EQ(range.base(), size_t(3));
+  EXPECT_EQ(range.extent(), size_t(7));
+  EXPECT_EQ(range.start(), size_t(3));
+  EXPECT_EQ(range.end(), size_t(7));
+  EXPECT_EQ(range.length(), size_t(4));
+  EXPECT_FALSE(range.collapsed());
+}
+
+TEST(TextRange, TextRangeFromReversedRange) {
+  TextRange range(7, 3);
+  EXPECT_EQ(range.base(), size_t(7));
+  EXPECT_EQ(range.extent(), size_t(3));
+  EXPECT_EQ(range.start(), size_t(3));
+  EXPECT_EQ(range.end(), size_t(7));
+  EXPECT_EQ(range.length(), size_t(4));
+  EXPECT_FALSE(range.collapsed());
+}
+
+}  // namespace flutter