Introduce ThreadedInputConnection behind a switch

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 <utility>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ skipping 695 matching lines @@
     IPC_MESSAGE_HANDLER(ViewMsg_ChangeResizeRect, OnChangeResizeRect)
     IPC_MESSAGE_HANDLER(ViewMsg_WasHidden, OnWasHidden)
     IPC_MESSAGE_HANDLER(ViewMsg_WasShown, OnWasShown)
     IPC_MESSAGE_HANDLER(ViewMsg_Repaint, OnRepaint)
     IPC_MESSAGE_HANDLER(ViewMsg_SetTextDirection, OnSetTextDirection)
     IPC_MESSAGE_HANDLER(ViewMsg_Move_ACK, OnRequestMoveAck)
     IPC_MESSAGE_HANDLER(ViewMsg_UpdateScreenRects, OnUpdateScreenRects)
     IPC_MESSAGE_HANDLER(ViewMsg_SetSurfaceIdNamespace, OnSetSurfaceIdNamespace)
 #if defined(OS_ANDROID)
     IPC_MESSAGE_HANDLER(InputMsg_ImeEventAck, OnImeEventAck)
+    IPC_MESSAGE_HANDLER(InputMsg_RequestTextInputStateUpdate,
+                        OnRequestTextInputStateUpdate)
     IPC_MESSAGE_HANDLER(ViewMsg_ShowImeIfNeeded, OnShowImeIfNeeded)
 #endif
     IPC_MESSAGE_HANDLER(ViewMsg_HandleCompositorProto, OnHandleCompositorProto)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 bool RenderWidget::Send(IPC::Message* message) {
   // Don't send any messages after the browser has told us to close, and filter
@@ skipping 431 matching lines @@
   if (IsDateTimeInput(new_type))
     return;  // Not considered as a text input field in WebKit/Chromium.
 
   blink::WebTextInputInfo new_info;
   if (webwidget_)
     new_info = webwidget_->textInputInfo();
   const ui::TextInputMode new_mode = ConvertInputMode(new_info.inputMode);
 
   bool new_can_compose_inline = CanComposeInline();
 
+  bool use_ime_thread = false;
+#if defined(OS_ANDROID)
+  use_ime_thread = base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kUseImeThread);
+#endif
+
   // Only sends text input params if they are changed or if the ime should be
   // shown.
   if (show_ime == ShowIme::IF_NEEDED ||
+      (use_ime_thread && 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
       ) {
     ViewHostMsg_TextInputState_Params params;
     params.type = new_type;
     params.mode = new_mode;
@@ skipping 528 matching lines @@
 
 #if defined(OS_ANDROID)
 void RenderWidget::OnImeEventSentForAck(const blink::WebTextInputInfo& info) {
   text_input_info_history_.push_back(info);
 }
 
 void RenderWidget::OnImeEventAck() {
   DCHECK_GE(text_input_info_history_.size(), 1u);
   text_input_info_history_.pop_front();
 }
+
+void RenderWidget::OnRequestTextInputStateUpdate() {
+  ImeEventGuard guard(this);

[aelias_OOO_until_Jul13, 2016/01/21 07:43:05]

I don't like that there's nothing to distinguish this ImeEventGuard from all the
others.  You have an implicit contract that a bunch of
not-specifically-enumerated renderer methods issue FROM_IME state updates, and
if ChromiumInputConnection doesn't consume the exact right number, the IME
thread either hangs or is being permanently one update behind.  And it's
difficult to prove/enforce that the implicit contract is met.

I suggest you make the change that the OnRequestTextInputStateUpdate's reply has
a boolean that *only it* has.  It can either be the existing from_ime (if you
eliminate all other uses of it), or it can be a new third boolean if that's
easier.  And the IME thread should *only* block on this boolean.  In
ChromiumInputConnection, everywhere you block the IME thread waiting for a state
update, you should first send an OnRequestTextInputStateUpdate IPC.  (That will
cause two IPCs for every IME update, but I don't think we should worry about
optimizing IPC count right now.)

[Changwan Ryu, 2016/01/22 10:22:17]

That's a great idea. Done.

+}
 #endif
 
 bool RenderWidget::ShouldHandleImeEvent() {
 #if defined(OS_ANDROID)
   if (!webwidget_)
     return false;
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kUseImeThread))
+    return true;
 
   // We cannot handle IME events if there is any chance that the event we are
   // receiving here from the browser is based on the state that is different
   // from our current one as indicated by |text_input_info_|.
   // The states the browser might be in are:
   // text_input_info_history_[0] - current state ack'd by browser
   // text_input_info_history_[1...N] - pending state changes
   for (size_t i = 0u; i < text_input_info_history_.size() - 1u; ++i) {
     if (text_input_info_history_[i] != text_input_info_)
       return false;
@@ skipping 372 matching lines @@
    STATIC_ASSERT_WTI_ENUM_MATCH(DoubleTapZoom, DOUBLE_TAP_ZOOM);
    STATIC_ASSERT_WTI_ENUM_MATCH(Auto,      AUTO);
 
    content::TouchAction content_touch_action =
        static_cast<content::TouchAction>(web_touch_action);
   Send(new InputHostMsg_SetTouchAction(routing_id_, content_touch_action));
 }
 
 void RenderWidget::didUpdateTextOfFocusedElementByNonUserInput() {
 #if defined(OS_ANDROID)
-  text_field_is_dirty_ = true;
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kUseImeThread))
+    text_field_is_dirty_ = true;
 #endif
 }
 
 scoped_ptr<WebGraphicsContext3DCommandBufferImpl>
 RenderWidget::CreateGraphicsContext3D(GpuChannelHost* gpu_channel_host) {
   // Explicitly disable antialiasing for the compositor. As of the time of
   // this writing, the only platform that supported antialiasing for the
   // compositor was Mac OS X, because the on-screen OpenGL context creation
   // code paths on Windows and Linux didn't yet have multisampling support.
   // Mac OS X essentially always behaves as though it's rendering offscreen.
@@ skipping 68 matching lines @@
 void RenderWidget::RegisterVideoHoleFrame(RenderFrameImpl* frame) {
   video_hole_frames_.AddObserver(frame);
 }
 
 void RenderWidget::UnregisterVideoHoleFrame(RenderFrameImpl* frame) {
   video_hole_frames_.RemoveObserver(frame);
 }
 #endif  // defined(VIDEO_HOLE)
 
 }  // namespace content