Almost finish moving context_menu_node_ from RenderViewImpl to RenderFrameImpl.

content/browser/renderer_host/ime_adapter_android.cc

Index: content/browser/renderer_host/ime_adapter_android.cc
===================================================================
--- content/browser/renderer_host/ime_adapter_android.cc	(revision 255633)
+++ content/browser/renderer_host/ime_adapter_android.cc	(working copy)
@@ -11,9 +11,14 @@
 #include "base/android/scoped_java_ref.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "content/browser/frame_host/frame_tree.h"
+#include "content/browser/frame_host/frame_tree_node.h"
+#include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
 #include "content/common/view_messages.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "jni/ImeAdapter_jni.h"
 #include "third_party/WebKit/public/web/WebCompositionUnderline.h"
@@ -99,7 +104,9 @@
   NativeWebKeyboardEvent event(static_cast<blink::WebInputEvent::Type>(type),
                                0 /* modifiers */, time_ms / 1000.0, key_code,
                                text, false /* is_system_key */);
-  rwhva_->SendKeyEvent(event);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::SendSyntheticKeyEventOnUI, this, event));

[aurimas (slooooooooow), 2014/03/07 18:04:40]

I was under impression that most of these calls in Java were made on UI thread.
Which ones did you find where not? I would rather fix this in Java and add
DCHECKs where for the correct thread.

[jam, 2014/03/07 18:15:45]

sure that works too. i didn't know how they're called as I'm not familiar with
the java side.

http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...

shows the failures that I saw:

org.chromium.content.browser.ClipboardTest#testCopyDocumentFragment
org.chromium.content.browser.input.ImeTest#testImeCopy
org.chromium.content.browser.input.ImeTest#testImeCut
org.chromium.content.browser.input.ImeTest#testImePaste
org.chromium.content.browser.input.InsertionHandleTest#testPasteAtInsertionHandle

crashed in patchset 17 before I made these changes.

I also searched crbug.com for ime_adapter_android and found crbug.com/310458
which also looked like ImeAdapterAndroid::SendKeyEvent was being called on the
wrong thread. since it appeared that all of them are called the same way, I
converted them all to use posttask.

   return true;
 }
 
@@ -117,55 +124,42 @@
   // one we expect to test if it's handled or unhandled, so skip handling the
   // "real" event in the browser.
   event.skip_in_browser = key_down_text_insertion;
-  rwhva_->SendKeyEvent(event);
-  if (key_down_text_insertion) {
-    // Send a Char event, but without an os_event since we don't want to
-    // roundtrip back to java such synthetic event.
-    NativeWebKeyboardEvent char_event(blink::WebInputEvent::Char, modifiers,
-                                      time_ms, key_code, unicode_char,
-                                      is_system_key);
-    char_event.skip_in_browser = key_down_text_insertion;
-    rwhva_->SendKeyEvent(char_event);
-  }
+
+  // Send a Char event, but without an os_event since we don't want to
+  // roundtrip back to java such synthetic event.
+  NativeWebKeyboardEvent char_event(blink::WebInputEvent::Char, modifiers,
+                                    time_ms, key_code, unicode_char,
+                                    is_system_key);
+  char_event.skip_in_browser = key_down_text_insertion;
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::SendKeyEventOnUI, this, event,
+                 char_event, key_down_text_insertion));
+
   return true;
 }
 
 void ImeAdapterAndroid::SetComposingText(JNIEnv* env, jobject, jstring text,
                                          int new_cursor_pos) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
-  if (!rwhi)
-    return;
-
   base::string16 text16 = ConvertJavaStringToUTF16(env, text);
-  std::vector<blink::WebCompositionUnderline> underlines;
-  underlines.push_back(
-      blink::WebCompositionUnderline(0, text16.length(), SK_ColorBLACK,
-                                      false));
-  // new_cursor_position is as described in the Android API for
-  // InputConnection#setComposingText, whereas the parameters for
-  // ImeSetComposition are relative to the start of the composition.
-  if (new_cursor_pos > 0)
-    new_cursor_pos = text16.length() + new_cursor_pos - 1;
-
-  rwhi->ImeSetComposition(text16, underlines, new_cursor_pos, new_cursor_pos);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::SetComposingTextOnUI, this, text16,
+                 new_cursor_pos));
 }
 
 void ImeAdapterAndroid::CommitText(JNIEnv* env, jobject, jstring text) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
-  if (!rwhi)
-    return;
-
   base::string16 text16 = ConvertJavaStringToUTF16(env, text);
-  rwhi->ImeConfirmComposition(text16, gfx::Range::InvalidRange(), false);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::CommitTextOnUI, this, text16));
 }
 
 void ImeAdapterAndroid::FinishComposingText(JNIEnv* env, jobject) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
-  if (!rwhi)
-    return;
-
-  rwhi->ImeConfirmComposition(base::string16(), gfx::Range::InvalidRange(),
-                              true);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::FinishComposingTextsOnUI, this));
 }
 
 void ImeAdapterAndroid::AttachImeAdapter(JNIEnv* env, jobject java_object) {
@@ -189,86 +183,184 @@
   }
 }
 
+void ImeAdapterAndroid::RenderWidgetGone() {
+  rwhva_ = NULL;
+}
+
 void ImeAdapterAndroid::SetEditableSelectionOffsets(JNIEnv*, jobject,
                                                     int start, int end) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
-  if (!rwhi)
-    return;
-
-  rwhi->Send(new ViewMsg_SetEditableSelectionOffsets(rwhi->GetRoutingID(),
-                                                     start, end));
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::SetEditableSelectionOffsetsOnUI, this,
+                 start, end));
 }
 
 void ImeAdapterAndroid::SetComposingRegion(JNIEnv*, jobject,
                                            int start, int end) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::SetComposingRegiontOnUI, this,
+                 start, end));
+}
+
+void ImeAdapterAndroid::DeleteSurroundingText(JNIEnv*, jobject,
+                                              int before, int after) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::DeleteSurroundingTextOnUI, this,
+                 before, after));
+}
+
+void ImeAdapterAndroid::Unselect(JNIEnv* env, jobject) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::UnselectOnUI, this));
+}
+
+void ImeAdapterAndroid::SelectAll(JNIEnv* env, jobject) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::SelectAllOnUI, this));
+}
+
+void ImeAdapterAndroid::Cut(JNIEnv* env, jobject) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::CutOnUI, this));
+}
+
+void ImeAdapterAndroid::Copy(JNIEnv* env, jobject) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::CopyOnUI, this));
+}
+
+void ImeAdapterAndroid::Paste(JNIEnv* env, jobject) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImeAdapterAndroid::PasteOnUI, this));
+}
+
+void ImeAdapterAndroid::ResetImeAdapter(JNIEnv* env, jobject) {
+  java_ime_adapter_.reset();
+}
+
+void ImeAdapterAndroid::SendSyntheticKeyEventOnUI(
+    const NativeWebKeyboardEvent& event) {
+  rwhva_->SendKeyEvent(event);
+}
+
+void ImeAdapterAndroid::SendKeyEventOnUI(
+    const NativeWebKeyboardEvent& event,
+    const NativeWebKeyboardEvent& char_event,
+    bool key_down_text_insertion) {
+  rwhva_->SendKeyEvent(event);
+  if (key_down_text_insertion)
+    rwhva_->SendKeyEvent(char_event);
+}
+
+void ImeAdapterAndroid::SetComposingTextOnUI(const base::string16& text16,
+                                             int new_cursor_pos) {
   RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
   if (!rwhi)
     return;
-
+  
   std::vector<blink::WebCompositionUnderline> underlines;
   underlines.push_back(
-      blink::WebCompositionUnderline(0, end - start, SK_ColorBLACK, false));
+      blink::WebCompositionUnderline(0, text16.length(), SK_ColorBLACK,
+                                      false));
+  // new_cursor_position is as described in the Android API for
+  // InputConnection#setComposingText, whereas the parameters for
+  // ImeSetComposition are relative to the start of the composition.
+  if (new_cursor_pos > 0)
+    new_cursor_pos = text16.length() + new_cursor_pos - 1;
 
-  rwhi->Send(new ViewMsg_SetCompositionFromExistingText(
-      rwhi->GetRoutingID(), start, end, underlines));
+  rwhi->ImeSetComposition(text16, underlines, new_cursor_pos, new_cursor_pos);
 }
 
-void ImeAdapterAndroid::DeleteSurroundingText(JNIEnv*, jobject,
-                                              int before, int after) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+void ImeAdapterAndroid::CommitTextOnUI(const base::string16& text16)  {
+    RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
   if (!rwhi)
     return;
-
-  rwhi->Send(new ViewMsg_ExtendSelectionAndDelete(rwhi->GetRoutingID(),
-                                                  before, after));
+  
+  rwhi->ImeConfirmComposition(text16, gfx::Range::InvalidRange(), false);
 }
 
-void ImeAdapterAndroid::Unselect(JNIEnv* env, jobject) {
+void ImeAdapterAndroid::FinishComposingTextsOnUI() {
   RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
   if (!rwhi)
     return;
 
-  rwhi->Unselect();
+  rwhi->ImeConfirmComposition(base::string16(), gfx::Range::InvalidRange(),
+                              true);
 }
 
-void ImeAdapterAndroid::SelectAll(JNIEnv* env, jobject) {
+void ImeAdapterAndroid::SetEditableSelectionOffsetsOnUI(int start, int end) {
   RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
   if (!rwhi)
     return;
 
-  rwhi->SelectAll();
+  rwhi->Send(new ViewMsg_SetEditableSelectionOffsets(rwhi->GetRoutingID(),
+                                                     start, end));
 }
 
-void ImeAdapterAndroid::Cut(JNIEnv* env, jobject) {
+void ImeAdapterAndroid::SetComposingRegiontOnUI(int start, int end) {
   RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
   if (!rwhi)
     return;
 
-  rwhi->Cut();
+  std::vector<blink::WebCompositionUnderline> underlines;
+  underlines.push_back(
+      blink::WebCompositionUnderline(0, end - start, SK_ColorBLACK, false));
+
+  rwhi->Send(new ViewMsg_SetCompositionFromExistingText(
+      rwhi->GetRoutingID(), start, end, underlines));
 }
 
-void ImeAdapterAndroid::Copy(JNIEnv* env, jobject) {
+void ImeAdapterAndroid::DeleteSurroundingTextOnUI(int before, int after) {
   RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
   if (!rwhi)
     return;
 
-  rwhi->Copy();
+  rwhi->Send(new ViewMsg_ExtendSelectionAndDelete(rwhi->GetRoutingID(),
+                                                  before, after));
 }
 
-void ImeAdapterAndroid::Paste(JNIEnv* env, jobject) {
+void ImeAdapterAndroid::UnselectOnUI() {
   RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
-  if (!rwhi)
-    return;
+  if (rwhi)
+    rwhi->Unselect();
+}
 
-  rwhi->Paste();
+void ImeAdapterAndroid::SelectAllOnUI() {
+  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  if (rwhi)
+    rwhi->SelectAll();
 }
 
-void ImeAdapterAndroid::ResetImeAdapter(JNIEnv* env, jobject) {
-  java_ime_adapter_.reset();
+void ImeAdapterAndroid::CutOnUI() {
+  RenderFrameHost* rfh = GetFocusedFrame();
+  if (rfh)
+    rfh->Cut();
 }
 
+void ImeAdapterAndroid::CopyOnUI() {
+  RenderFrameHost* rfh = GetFocusedFrame();
+  if (rfh)
+    rfh->Copy();
+}
+
+void ImeAdapterAndroid::PasteOnUI() {
+  RenderFrameHost* rfh = GetFocusedFrame();
+  if (rfh)
+    rfh->Paste();
+}
+
 RenderWidgetHostImpl* ImeAdapterAndroid::GetRenderWidgetHostImpl() {
-  DCHECK(rwhva_);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!rwhva_)
+    return NULL;
+
   RenderWidgetHost* rwh = rwhva_->GetRenderWidgetHost();
   if (!rwh)
     return NULL;
@@ -276,4 +368,21 @@
   return RenderWidgetHostImpl::From(rwh);
 }
 
+RenderFrameHost* ImeAdapterAndroid::GetFocusedFrame() {
+  RenderWidgetHostImpl* rwh = GetRenderWidgetHostImpl();
+  if (!rwh)
+    return NULL;
+  if (!rwh->IsRenderView())
+    return NULL;
+  RenderViewHost* rvh = RenderViewHost::From(rwh);
+  RenderFrameHostImpl* rfh =
+      static_cast<RenderFrameHostImpl*>(rvh->GetMainFrame());
+  FrameTreeNode* focused_frame =
+      rfh->frame_tree_node()->frame_tree()->GetFocusedFrame();
+  if (!focused_frame)
+    return NULL;
+
+  return focused_frame->current_frame_host();
+}
+
 }  // namespace content