Fix endBatchEdit

content/renderer/render_widget.cc

 // Copyright (c) 2012 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 "content/renderer/render_widget.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/auto_reset.h"
@@ skipping 879 matching lines @@
 
 void RenderWidget::SetInputHandler(RenderWidgetInputHandler* input_handler) {
   // Nothing to do here. RenderWidget created the |input_handler| and will take
   // ownership of it. We just verify here that we don't already have an input
   // handler.
   DCHECK(!input_handler_);
 }
 
 void RenderWidget::UpdateTextInputState(ShowIme show_ime,
                                         ChangeSource change_source) {
-  TRACE_EVENT0("renderer", "RenderWidget::UpdateTextInputState");
-  if (ime_event_guard_) {
-    // show_ime should still be effective even if it was set inside the IME
-    // event guard.
-    if (show_ime == ShowIme::IF_NEEDED) {
-      ime_event_guard_->set_show_ime(true);
-    }
-    return;
-  }
-
-  ui::TextInputType new_type = GetTextInputType();
-  if (IsDateTimeInput(new_type))
-    return;  // Not considered as a text input field in WebKit/Chromium.
-
-  blink::WebTextInputInfo new_info;
-  if (GetWebWidget())
-    new_info = GetWebWidget()->textInputInfo();
-  const ui::TextInputMode new_mode =
-      ConvertWebTextInputMode(new_info.inputMode);
-
-  bool new_can_compose_inline = CanComposeInline();
-
-  // Only sends text input params if they are changed or if the ime should be
-  // shown.
-  if (show_ime == ShowIme::IF_NEEDED ||
-      (IsUsingImeThread() && change_source == ChangeSource::FROM_IME) ||
-      (text_input_type_ != new_type || text_input_mode_ != new_mode ||
-       text_input_info_ != new_info ||
-       can_compose_inline_ != new_can_compose_inline)
-#if defined(OS_ANDROID)
-      || text_field_is_dirty_
-#endif
-      ) {
-    TextInputState params;
-    params.type = new_type;
-    params.mode = new_mode;
-    params.flags = new_info.flags;
-    params.value = new_info.value.utf8();
-    params.selection_start = new_info.selectionStart;
-    params.selection_end = new_info.selectionEnd;
-    params.composition_start = new_info.compositionStart;
-    params.composition_end = new_info.compositionEnd;
-    params.can_compose_inline = new_can_compose_inline;
-    params.show_ime_if_needed = (show_ime == ShowIme::IF_NEEDED);
-#if defined(USE_AURA)
-    params.is_non_ime_change = true;
-#endif
-#if defined(OS_ANDROID)
-    params.is_non_ime_change =
-        (change_source == ChangeSource::FROM_NON_IME) || text_field_is_dirty_;
-    params.batch_edit = ime_in_batch_edit_;
-    if (params.is_non_ime_change)
-      OnImeEventSentForAck(new_info);
-    text_field_is_dirty_ = false;
-#endif
-    Send(new ViewHostMsg_TextInputStateChanged(routing_id(), params));
-
-    text_input_info_ = new_info;
-    text_input_type_ = new_type;
-    text_input_mode_ = new_mode;
-    can_compose_inline_ = new_can_compose_inline;
-    text_input_flags_ = new_info.flags;
-  }
+  UpdateTextInputStateInternal(show_ime, change_source, false);
 }
 
 bool RenderWidget::WillHandleGestureEvent(const blink::WebGestureEvent& event) {
   if (owner_delegate_)
     return owner_delegate_->RenderWidgetWillHandleGestureEvent(event);
 
   return false;
 }
 
 bool RenderWidget::WillHandleMouseEvent(const blink::WebMouseEvent& event) {
@@ skipping 637 matching lines @@
 }
 
 void RenderWidget::OnImeBatchEdit(bool begin) {
   if (begin) {
     ime_in_batch_edit_ = true;
     return;
   }
   if (!ime_in_batch_edit_)
     return;
   ime_in_batch_edit_ = false;
+
   DCHECK(!ime_event_guard_);
   UpdateSelectionBounds();
-  UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_IME);
+
+  // Make sure that we do the state update here because the previous state
+  // updates in batch edit got ignored.
+  ForceUpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME);
 }
 #endif
 
+void RenderWidget::ForceUpdateTextInputState(ShowIme show_ime,
+                                             ChangeSource change_source) {
+  UpdateTextInputStateInternal(show_ime, change_source, true);
+}
+
+void RenderWidget::UpdateTextInputStateInternal(ShowIme show_ime,

[aelias_OOO_until_Jul13, 2016/11/01 23:37:30]

Could you move this above RenderWidget::UpdateTextInputState() to keep git from
blaming this whole method as new code?

+                                                ChangeSource change_source,
+                                                bool force_update) {

[aelias_OOO_until_Jul13, 2016/11/01 23:37:30]

I don't think we should add this argument.  It's more natural to use an approach
similar to text_field_is_dirty_ for this as well, see below (although I don't
think we should reuse text_field_is_dirty_ since that's ReplicaInputConnection
thing due for deletion and poorly named for this.)

+  TRACE_EVENT0("renderer", "RenderWidget::UpdateTextInputState");
+  if (ime_event_guard_) {
+    // show_ime should still be effective even if it was set inside the IME
+    // event guard.
+    if (show_ime == ShowIme::IF_NEEDED) {
+      ime_event_guard_->set_show_ime(true);
+    }
+    return;
+  }
+
+  ui::TextInputType new_type = GetTextInputType();
+  if (IsDateTimeInput(new_type))
+    return;  // Not considered as a text input field in WebKit/Chromium.
+
+  blink::WebTextInputInfo new_info;
+  if (GetWebWidget())
+    new_info = GetWebWidget()->textInputInfo();
+  const ui::TextInputMode new_mode =
+      ConvertWebTextInputMode(new_info.inputMode);
+
+  bool new_can_compose_inline = CanComposeInline();
+
+  // Only sends text input params if they are changed or if the ime should be
+  // shown.
+  if (show_ime == ShowIme::IF_NEEDED ||
+      (IsUsingImeThread() && change_source == ChangeSource::FROM_IME) ||
+      (text_input_type_ != new_type || text_input_mode_ != new_mode ||
+       text_input_info_ != new_info ||
+       can_compose_inline_ != new_can_compose_inline || force_update)
+#if defined(OS_ANDROID)
+      || text_field_is_dirty_
+#endif
+      ) {
+    TextInputState params;

[aelias_OOO_until_Jul13, 2016/11/01 23:37:30]

Please put an early return like in your previous patch, but within this if()
statement.  It should look like:

#if defined(OS_ANDROID)
      || text_field_is_dirty_
#endif
      || ime_batched_update_pending_) {
  if (ime_in_batch_edit_ && change_source == ChangeSource::FROM_NON_IME) {
    ime_batched_update_pending_ = true;
    return;
  }

(This means beginBatchEdit+endBatchEdit with nothing in between would not force
any state update, but instead we would track if there is an update actually
needed.)

+    params.type = new_type;
+    params.mode = new_mode;
+    params.flags = new_info.flags;
+    params.value = new_info.value.utf8();
+    params.selection_start = new_info.selectionStart;
+    params.selection_end = new_info.selectionEnd;
+    params.composition_start = new_info.compositionStart;
+    params.composition_end = new_info.compositionEnd;
+    params.can_compose_inline = new_can_compose_inline;
+    params.show_ime_if_needed = (show_ime == ShowIme::IF_NEEDED);
+#if defined(USE_AURA)
+    params.is_non_ime_change = true;
+#endif
+#if defined(OS_ANDROID)
+    params.is_non_ime_change =
+        (change_source == ChangeSource::FROM_NON_IME) || text_field_is_dirty_;
+    params.batch_edit = ime_in_batch_edit_;
+    if (params.is_non_ime_change)
+      OnImeEventSentForAck(new_info);
+    text_field_is_dirty_ = false;
+#endif
+    Send(new ViewHostMsg_TextInputStateChanged(routing_id(), params));
+
+    text_input_info_ = new_info;
+    text_input_type_ = new_type;
+    text_input_mode_ = new_mode;
+    can_compose_inline_ = new_can_compose_inline;
+    text_input_flags_ = new_info.flags;
+  }
+}
+
 void RenderWidget::OnRequestCompositionUpdate(bool immediate_request,
                                               bool monitor_request) {
   monitor_composition_info_ = monitor_request;
   if (!immediate_request)
     return;
   UpdateCompositionInfo(true /* immediate request */);
 }
 
 bool RenderWidget::ShouldHandleImeEvent() {
 #if defined(OS_ANDROID)
@@ skipping 452 matching lines @@
 bool RenderWidget::isPointerLocked() {
   return mouse_lock_dispatcher_->IsMouseLockedTo(
       webwidget_mouse_lock_target_.get());
 }
 
 blink::WebWidget* RenderWidget::GetWebWidget() const {
   return webwidget_internal_;
 }
 
 }  // namespace content