Fix OSK flickering issue

content/renderer/render_widget.cc

Index: content/renderer/render_widget.cc
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 2ece7075add91125890a1c8b7bdfb2b73940e1b4..8556d908b37a49596e74766fe6ce415e8f3dbda4 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -498,7 +498,8 @@ RenderWidget::RenderWidget(CompositorDependencies* compositor_deps,
       display_mode_(blink::WebDisplayModeUndefined),
       handling_input_event_(false),
       handling_event_overscroll_(nullptr),
-      handling_ime_event_(false),
+      ime_guard_count_(0),
+      pending_show_ime_if_needed_(false),
       handling_event_type_(WebInputEvent::Undefined),
       ignore_ack_for_mouse_move_from_debugger_(false),
       closing_(false),
@@ -517,6 +518,7 @@ RenderWidget::RenderWidget(CompositorDependencies* compositor_deps,
       next_output_surface_id_(0),
 #if defined(OS_ANDROID)
       text_field_is_dirty_(false),
+      pending_update_from_non_ime_(false),
 #endif
       popup_origin_scale_for_emulation_(0.f),
       frame_swap_message_queue_(new FrameSwapMessageQueue()),
@@ -1070,25 +1072,11 @@ void RenderWidget::OnHandleInputEvent(const blink::WebInputEvent* input_event,
                                          &event_overscroll);
 
 #if defined(OS_ANDROID)
-  // On Android, when a key is pressed or sent from the Keyboard using IME,
-  // |AdapterInputConnection| generates input key events to make sure all JS
-  // listeners that monitor KeyUp and KeyDown events receive the proper key
-  // code. Since this input key event comes from IME, we need to set the
-  // IME event guard here to make sure it does not interfere with other IME
-  // events.
-  scoped_ptr<ImeEventGuard> ime_event_guard_maybe;
-  if (WebInputEvent::isKeyboardEventType(input_event->type)) {
-    const WebKeyboardEvent& key_event =
-        *static_cast<const WebKeyboardEvent*>(input_event);
-    // Some keys are special and it's essential that no events get blocked.
-    if (key_event.nativeKeyCode != AKEYCODE_TAB &&
-        key_event.nativeKeyCode != AKEYCODE_DPAD_CENTER &&
-        key_event.nativeKeyCode != AKEYCODE_DPAD_LEFT &&
-        key_event.nativeKeyCode != AKEYCODE_DPAD_RIGHT &&
-        key_event.nativeKeyCode != AKEYCODE_DPAD_UP &&
-        key_event.nativeKeyCode != AKEYCODE_DPAD_DOWN)
-      ime_event_guard_maybe.reset(new ImeEventGuard(this));
-  }
+  const bool is_keyboard_event =
+      WebInputEvent::isKeyboardEventType(input_event->type);
+
+  // For non-keyboard events, we want the change source to be FROM_NON_IME.
+  ImeEventGuard guard(this, !is_keyboard_event);
 #endif
 
   base::TimeTicks start_time;
@@ -1877,29 +1865,45 @@ static bool IsDateTimeInput(ui::TextInputType type) {
       type == ui::TEXT_INPUT_TYPE_WEEK;
 }
 
-
-void RenderWidget::StartHandlingImeEvent() {
-  DCHECK(!handling_ime_event_);
-  handling_ime_event_ = true;
+void RenderWidget::OnImeEventGuardStart() {
+  ++ime_guard_count_;
 }
 
-void RenderWidget::FinishHandlingImeEvent() {
-  DCHECK(handling_ime_event_);
-  handling_ime_event_ = false;
+void RenderWidget::OnImeEventGuardFinish(bool from_non_ime) {
+  --ime_guard_count_;
+  CHECK_GE(ime_guard_count_, 0);
+#if defined(OS_ANDROID)
+  pending_update_from_non_ime_ |= from_non_ime;

[no sievers, 2015/10/26 20:13:12]

Since we already have the guard class, it'd be nice to encapsulate these
variables there (also this patch ends up duplicating the 'from_non_ime'
tracking).

One way to do it would be to track the guard instance at the bottom of the frame
instead of the counter in this class:

void RenderWidget::OnImeEventGuardStart(ImeGuard* guard) {
  if (!ime_guard_)
    ime_guard_ = guard;
}

void RenderWidget::OnImeEventGuardFinish(ImeGuard* guard) {
  if (ime_guard_ != guard)
    return;
  ime_guard_ = nullptr;
  UpdateSelectionBounds();
  UpdateTextInputState(guard->show_ime, guard->change_source);
}

void RenderWidget::UpdateTextInputState() {
  if (image_guard_) {
    if (show_ime == SHOW_IME_IF_NEEDED)
      ime_guard_->show_ime_ = show_ime;
    return;
  }

  ui::TextInputType new_type = GetTextInputType();
  ...
}

wdyt?

[Changwan Ryu, 2015/10/28 05:17:07]

Hmm... The lifetime of one ImeEventGuard may be nesting or overlapping that of
another. In case of nesting, the first call to OnImeEventGuardFinish() will
trigger UpdateTextInputState() which isn't intended. Without keeping the count,
we cannot tell whether the current ImeEventGuard is the last one or not.

So if we are to move some of the  variables inside ImeEventGuard, we will still
need to keep ime_guard_count_ and pending_update_from_non_ime_ outside of
ImeEventGuard, and only pending_show_ime_if_needed_ can be moved inside it. I'm
not sure if that can increase the readability here. Please advise.

[Changwan Ryu, 2015/10/28 16:25:51]

My bad. Overlapping is not a possible scenario here.
I've updated the code as you suggested. Looks much cleaner now! PTAL.

+#endif
+  if (ime_guard_count_ > 0)
+    return;
+
   // While handling an ime event, text input state and selection bounds updates
   // are ignored. These must explicitly be updated once finished handling the
   // ime event.
   UpdateSelectionBounds();
 #if defined(OS_ANDROID)
-  UpdateTextInputState(NO_SHOW_IME, FROM_IME);
+  UpdateTextInputState(NO_SHOW_IME,
+                       pending_update_from_non_ime_ ? FROM_NON_IME : FROM_IME);
+  pending_update_from_non_ime_ = false;
 #endif
 }
 
 void RenderWidget::UpdateTextInputState(ShowIme show_ime,
                                         ChangeSource change_source) {
   TRACE_EVENT0("renderer", "RenderWidget::UpdateTextInputState");
-  if (handling_ime_event_)
+  if (ime_guard_count_ > 0) {
+    if (show_ime == SHOW_IME_IF_NEEDED)
+      pending_show_ime_if_needed_ = true;
+
     return;
+  }
+
+  if (pending_show_ime_if_needed_) {
+    show_ime = SHOW_IME_IF_NEEDED;
+    pending_show_ime_if_needed_ = false;
+  }
+
   ui::TextInputType new_type = GetTextInputType();
   if (IsDateTimeInput(new_type))
     return;  // Not considered as a text input field in WebKit/Chromium.
@@ -1965,7 +1969,7 @@ void RenderWidget::UpdateSelectionBounds() {
   TRACE_EVENT0("renderer", "RenderWidget::UpdateSelectionBounds");
   if (!webwidget_)
     return;
-  if (handling_ime_event_)
+  if (ime_guard_count_ > 0)
     return;
 
 #if defined(USE_AURA)