Prevent the IME candidate window from flickering on Chrome OS

ui/views/controls/textfield/textfield_model.cc

 // Copyright 2014 The Chromium 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 "ui/views/controls/textfield/textfield_model.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "base/macros.h"
@@ skipping 264 matching lines @@
   DCHECK(base::MessageLoopForUI::IsCurrent());
   return &kill_buffer;
 }
 
 // Helper method to set the kill buffer.
 void SetKillBuffer(const base::string16& buffer) {
   base::string16* kill_buffer = GetKillBuffer();
   *kill_buffer = buffer;
 }
 
+void SelectRangeInCompositionText(gfx::RenderText* render_text,
+                                  size_t cursor,
+                                  const gfx::Range& range) {
+  DCHECK(render_text);
+  DCHECK(range.IsValid());
+  uint32_t start = range.GetMin();
+  uint32_t end = range.GetMax();
+#if defined(OS_CHROMEOS)
+  // Swap |start| and |end| so that GetCaretBounds() can always return the same
+  // value during conversion.
+  // TODO(yusukes): Check if this works for other platforms. If it is, use this
+  // on all platforms.
+  std::swap(start, end);
+#endif
+  render_text->SelectRange(gfx::Range(cursor + start, cursor + end));

[oshima, 2017/05/22 09:55:21]

what's the reason why we don't change the UI side to align the window to left
instead?

[Yusuke Sato, 2017/05/22 16:55:02]

That's another option, but strictly speaking, if we do that, the new behavior
won't match chrome.input.ime API document at
https://developer.chrome.com/extensions/input_ime#type-WindowPosition which
explicitly says that the window position follows the cursor, not the selection
range. We could add yet another WindowPosition type like "selection" and let the
Japanese IME migrate to the new mode, but this seems overkill given that the
Blink side (RWHVA) already implements the existing "cursor" mode very well.

So instead of changing the UI behavior, this CL tried to improve Textfield as
good as RWHVA regarding the "cursor" mode support. WDYT?

+}
+
 }  // namespace
 
 using internal::Edit;
 using internal::DeleteEdit;
 using internal::InsertEdit;
 using internal::ReplaceEdit;
 using internal::MergeType;
 using internal::DO_NOT_MERGE;
 using internal::FORCE_MERGE;
 using internal::MERGEABLE;
@@ skipping 354 matching lines @@
     render_text_->SetCompositionRange(gfx::Range::InvalidRange());
   gfx::Range emphasized_range = GetFirstEmphasizedRange(composition);
   if (emphasized_range.IsValid()) {
     // This is a workaround due to the lack of support in RenderText to draw
     // a thick underline. In a composition returned from an IME, the segment
     // emphasized by a thick underline usually represents the target clause.
     // Because the target clause is more important than the actual selection
     // range (or caret position) in the composition here we use a selection-like
     // marker instead to show this range.
     // TODO(yukawa, msw): Support thick underlines and remove this workaround.
-    render_text_->SelectRange(gfx::Range(
-        cursor + emphasized_range.GetMin(),
-        cursor + emphasized_range.GetMax()));
+    SelectRangeInCompositionText(render_text_.get(), cursor, emphasized_range);
   } else if (!composition.selection.is_empty()) {
-    render_text_->SelectRange(gfx::Range(
-        cursor + composition.selection.GetMin(),
-        cursor + composition.selection.GetMax()));
+    SelectRangeInCompositionText(render_text_.get(), cursor,
+                                 composition.selection);
   } else {
     render_text_->SetCursorPosition(cursor + composition.selection.end());
   }
 }
 
 void TextfieldModel::ConfirmCompositionText() {
   DCHECK(HasCompositionText());
   base::string16 composition = text().substr(
       composition_range_.start(), composition_range_.length());
   // TODO(oshima): current behavior on ChromeOS is a bit weird and not
@@ skipping 162 matching lines @@
   render_text_->SetCursorPosition(new_cursor_pos);
   // TODO(oshima): Select text that was just undone, like Mac (but not GTK).
 }
 
 // static
 void TextfieldModel::ClearKillBuffer() {
   SetKillBuffer(base::string16());
 }
 
 }  // namespace views