Route IME Events to Focused RenderWidgets (Android)

content/browser/renderer_host/ime_adapter_android.cc

Index: content/browser/renderer_host/ime_adapter_android.cc
diff --git a/content/browser/renderer_host/ime_adapter_android.cc b/content/browser/renderer_host/ime_adapter_android.cc
index 3a7d18b94da7ce41b8a4981e98e7d87b3eda70cc..775f1da9851fc7ced6aec11b953afab601b7a21e 100644
--- a/content/browser/renderer_host/ime_adapter_android.cc
+++ b/content/browser/renderer_host/ime_adapter_android.cc
@@ -110,6 +110,7 @@ void AppendUnderlineSpan(JNIEnv*,
 
 ImeAdapterAndroid::ImeAdapterAndroid(RenderWidgetHostViewAndroid* rwhva)
     : rwhva_(rwhva) {
+  DCHECK(rwhva_);
 }
 
 ImeAdapterAndroid::~ImeAdapterAndroid() {
@@ -142,7 +143,7 @@ void ImeAdapterAndroid::SetComposingText(JNIEnv* env,
                                          const JavaParamRef<jobject>& text,
                                          const JavaParamRef<jstring>& text_str,
                                          int relative_cursor_pos) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwhi = GetFocusedWidget();
   if (!rwhi)
     return;
 
@@ -172,7 +173,7 @@ void ImeAdapterAndroid::CommitText(JNIEnv* env,
                                    const JavaParamRef<jobject>& text,
                                    const JavaParamRef<jstring>& text_str,
                                    int relative_cursor_pos) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwhi = GetFocusedWidget();
   if (!rwhi)
     return;
 
@@ -195,7 +196,7 @@ void ImeAdapterAndroid::CommitText(JNIEnv* env,
 
 void ImeAdapterAndroid::FinishComposingText(JNIEnv* env,
                                             const JavaParamRef<jobject>&) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwhi = GetFocusedWidget();
   if (!rwhi)
     return;
 
@@ -288,7 +289,7 @@ void ImeAdapterAndroid::DeleteSurroundingText(JNIEnv*,
 bool ImeAdapterAndroid::RequestTextInputStateUpdate(
     JNIEnv* env,
     const JavaParamRef<jobject>&) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwhi = GetFocusedWidget();
   if (!rwhi)
     return false;
   rwhi->Send(new InputMsg_RequestTextInputStateUpdate(rwhi->GetRoutingID()));
@@ -300,7 +301,7 @@ void ImeAdapterAndroid::RequestCursorUpdate(
     const base::android::JavaParamRef<jobject>& obj,
     bool immediate_request,
     bool monitor_request) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwhi = GetFocusedWidget();
   if (!rwhi)
     return;
   rwhi->Send(new InputMsg_RequestCompositionUpdate(
@@ -312,23 +313,22 @@ void ImeAdapterAndroid::ResetImeAdapter(JNIEnv* env,
   java_ime_adapter_.reset();
 }
 
-RenderWidgetHostImpl* ImeAdapterAndroid::GetRenderWidgetHostImpl() {
+RenderWidgetHostImpl* ImeAdapterAndroid::GetFocusedWidget() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK(rwhva_);
-  RenderWidgetHost* rwh = rwhva_->GetRenderWidgetHost();
-  if (!rwh)
-    return nullptr;
-
-  return RenderWidgetHostImpl::From(rwh);
+  return rwhva_->GetFocusedWidget();
 }
 
 RenderFrameHost* ImeAdapterAndroid::GetFocusedFrame() {
-  RenderWidgetHostImpl* rwh = GetRenderWidgetHostImpl();
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  RenderWidgetHostImpl* rwh =
+      RenderWidgetHostImpl::From(rwhva_->GetRenderWidgetHost());

[Charlie Reis, 2017/01/27 01:07:22]

This would effectively revert boliu's https://codereview.chromium.org/22764006.

Then again, I don't understand how that had any effect on null pointer crashes
in the first place.  RenderWidgetHostImpl::From is just a static_cast, and that
can't crash.  Seems like that CL was a no-op.  (Ah, yes,
https://codereview.chromium.org/22882002/ was the one that actually fixed the
crash.)

Ok, I'm fine with this.

Please include a comment about why it uses the page-level widget instead of
GetFocusedWidget (like most other places).

[EhsanK, 2017/01/27 16:16:47]

Acknowledged. Thanks for finding out about those CLs. I actually changed this
part a bit more and reduced the code size and kind of avoided exposing
FrameTreeNode here.

   if (!rwh)
     return nullptr;
+
   RenderViewHost* rvh = RenderViewHost::From(rwh);
   if (!rvh)
     return nullptr;
+
   FrameTreeNode* focused_frame =
       rvh->GetDelegate()->GetFrameTree()->GetFocusedFrame();
   if (!focused_frame)
@@ -338,10 +338,13 @@ RenderFrameHost* ImeAdapterAndroid::GetFocusedFrame() {
 }
 
 WebContents* ImeAdapterAndroid::GetWebContents() {
-  RenderWidgetHostImpl* rwh = GetRenderWidgetHostImpl();
-  if (!rwh)
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  RenderWidgetHostImpl* rwh =
+      RenderWidgetHostImpl::From(rwhva_->GetRenderWidgetHost());

[Charlie Reis, 2017/01/27 01:07:22]

As above, please include a comment.

[EhsanK, 2017/01/27 16:16:47]

Acknowledged.

+  if (!rwh || !rwh->delegate())
     return nullptr;
-  return WebContents::FromRenderViewHost(RenderViewHost::From(rwh));
+
+  return rwh->delegate()->GetAsWebContents();
 }
 
 std::vector<blink::WebCompositionUnderline>