Stop combining text runs which are connected by 'COMMON' blocks.

ui/gfx/text_utils.cc

Index: ui/gfx/text_utils.cc
diff --git a/ui/gfx/text_utils.cc b/ui/gfx/text_utils.cc
index a31ef3d3cc999cd844fb4042b17628b9db976bcf..d9ef200ac148d163f83700e78764eae99072a980 100644
--- a/ui/gfx/text_utils.cc
+++ b/ui/gfx/text_utils.cc
@@ -5,9 +5,32 @@
 #include "ui/gfx/text_utils.h"
 
 #include "base/i18n/char_iterator.h"
+#include "base/logging.h"
+#include "third_party/icu/source/common/unicode/uchar.h"
+#include "third_party/icu/source/common/unicode/utf16.h"
 
 namespace gfx {
 
+namespace {
+
+// Returns true if the code point |c| is a combining mark character in Unicode.
+bool CharIsMark(UChar32 c) {
+  int8_t char_type = u_charType(c);
+  return char_type == U_NON_SPACING_MARK || char_type == U_ENCLOSING_MARK ||
+         char_type == U_COMBINING_SPACING_MARK;
+}
+
+// Gets the code point of |str| at the given code unit position |index|. If
+// |index| is a surrogate code unit, returns the whole code point (unless the
+// code unit is unpaired, in which case it just returns the surrogate value).
+UChar32 GetCodePointAt(const base::string16& str, size_t index) {
+  UChar32 c;
+  U16_GET(str.data(), 0, index, str.size(), c);
+  return c;
+}
+
+}  // namespace
+
 base::string16 RemoveAcceleratorChar(const base::string16& s,
                                      base::char16 accelerator_char,
                                      int* accelerated_char_pos,
@@ -46,4 +69,37 @@ base::string16 RemoveAcceleratorChar(const base::string16& s,
   return accelerator_removed;
 }
 
+size_t FindValidBoundaryBefore(const base::string16& text, size_t index) {
+  size_t length = text.length();
+  DCHECK_LE(index, length);
+  if (index == length)
+    return index;
+
+  // If |index| straddles a combining character sequence, go back until we find
+  // a base character.
+  while (index > 0 && CharIsMark(GetCodePointAt(text, index)))
+    --index;
+
+  // If |index| straddles a UTF-16 surrogate pair, go back.
+  U16_SET_CP_START(text.data(), 0, index);
+  return index;
+}
+
+size_t FindValidBoundaryAfter(const base::string16& text, size_t index) {

[msw, 2015/06/03 00:32:04]

Why did you drop the checked_casts here? Is
https://codereview.chromium.org/681123006 somehow no longer applicable to this
code? Be careful about changing code when you move it, and be sure to call it
out in a CL description when you do so. I wonder why FindValidBoundaryBefore
doesn't need a similar cast.

[xdai1, 2015/06/06 00:05:19]

Sorry my bad, I should not have done that. I saw FindValidBoundaryBefore()
doesn't have this checked_casts, and thought it should be safe to remove it here
as well... 

The reason that FindValidBoundaryBefore() doesn't need this cast is that
U16_SET_CP_START() doesn't take text.length() as an argument, but
U16_SET_CP_LIMIT() does. As described in
https://codereview.chromium.org/681123006, U16_SET_CP_LIMIT() expect the
argument to be an int32, so if we pass a size_t value to it, when compiling with
--Werror=type-limits, it will result in compilation error.

+  size_t length = text.length();
+  DCHECK_LE(index, length);
+  if (index == text.length())
+    return index;
+
+  // If |index| straddles a combining character sequence, go forward until we
+  // find a base character.
+  while (index < length && CharIsMark(GetCodePointAt(text, index))) {
+    ++index;
+  }

[msw, 2015/06/03 00:32:04]

nit: drop curly braces

[xdai1, 2015/06/06 00:05:19]

After bring back the checked_casts, it still requires curly braces here.

+
+  // If |index| straddles a UTF-16 surrogate pair, go forward.
+  U16_SET_CP_LIMIT(text.data(), 0, index, length);
+  return index;
+}
+
 }  // namespace gfx