Route IME Events to Focused RenderWidgets (Android)

content/browser/renderer_host/ime_adapter_android.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/browser/renderer_host/ime_adapter_android.h"
 
 #include <algorithm>
 #include <android/input.h>
 #include <vector>
 
@@ skipping 124 matching lines @@
           time_ms / 1000.0, key_code, scan_code, is_system_key, unicode_char);
   rwhva_->SendKeyEvent(event);
   return true;
 }
 
 void ImeAdapterAndroid::SetComposingText(JNIEnv* env,
                                          const JavaParamRef<jobject>& obj,
                                          const JavaParamRef<jobject>& text,
                                          const JavaParamRef<jstring>& text_str,
                                          int relative_cursor_pos) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwhi = GetFocusedWidget();
   if (!rwhi)
     return;
 
   base::string16 text16 = ConvertJavaStringToUTF16(env, text_str);
 
   std::vector<blink::WebCompositionUnderline> underlines =
       GetUnderlinesFromSpans(env, obj, text, text16);
 
   // Default to plain underline if we didn't find any span that we care about.
   if (underlines.empty()) {
     underlines.push_back(blink::WebCompositionUnderline(
         0, text16.length(), SK_ColorBLACK, false, SK_ColorTRANSPARENT));
   }
 
   // relative_cursor_pos is as described in the Android API for
   // InputConnection#setComposingText, whereas the parameters for
   // ImeSetComposition are relative to the start of the composition.
   if (relative_cursor_pos > 0)
     relative_cursor_pos = text16.length() + relative_cursor_pos - 1;
 
   rwhi->ImeSetComposition(text16, underlines, gfx::Range::InvalidRange(),
                           relative_cursor_pos, relative_cursor_pos);
 }
 
 void ImeAdapterAndroid::CommitText(JNIEnv* env,
                                    const JavaParamRef<jobject>& obj,
                                    const JavaParamRef<jobject>& text,
                                    const JavaParamRef<jstring>& text_str,
                                    int relative_cursor_pos) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwhi = GetFocusedWidget();
   if (!rwhi)
     return;
 
   base::string16 text16 = ConvertJavaStringToUTF16(env, text_str);
 
   std::vector<blink::WebCompositionUnderline> underlines =
       GetUnderlinesFromSpans(env, obj, text, text16);
 
   // relative_cursor_pos is as described in the Android API for
   // InputConnection#commitText, whereas the parameters for
   // ImeConfirmComposition are relative to the end of the composition.
   if (relative_cursor_pos > 0)
     relative_cursor_pos--;
   else
     relative_cursor_pos -= text16.length();
 
   rwhi->ImeCommitText(text16, underlines, gfx::Range::InvalidRange(),
                       relative_cursor_pos);
 }
 
 void ImeAdapterAndroid::FinishComposingText(JNIEnv* env,
                                             const JavaParamRef<jobject>&) {
-  RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwhi = GetFocusedWidget();
   if (!rwhi)
     return;
 
   rwhi->ImeFinishComposingText(true);
 }
 
 void ImeAdapterAndroid::AttachImeAdapter(
     JNIEnv* env,
     const JavaParamRef<jobject>& java_object) {
   java_ime_adapter_ = JavaObjectWeakGlobalRef(env, java_object);
@@ skipping 72 matching lines @@
                                               int after) {
   RenderFrameHostImpl* rfh =
       static_cast<RenderFrameHostImpl*>(GetFocusedFrame());
   if (rfh)
     rfh->DeleteSurroundingText(before, after);
 }
 
 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()));
   return true;
 }
 
 void ImeAdapterAndroid::RequestCursorUpdate(
     JNIEnv* env,
     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(
       rwhi->GetRoutingID(), immediate_request, monitor_request));
 }
 
 void ImeAdapterAndroid::ResetImeAdapter(JNIEnv* env,
                                         const JavaParamRef<jobject>&) {
   java_ime_adapter_.reset();
 }
 
-RenderWidgetHostImpl* ImeAdapterAndroid::GetRenderWidgetHostImpl() {
+RenderWidgetHostImpl* ImeAdapterAndroid::GetPageRenderWidgetHostImpl() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(rwhva_);
   RenderWidgetHost* rwh = rwhva_->GetRenderWidgetHost();

[Charlie Reis, 2017/01/25 22:00:22]

Isn't this the RWH associated with RenderWidgetHostViewAndroid?  For an OOPIF,
wouldn't that be a subframe widget rather than a "page" (main frame?) widget? 
I'm confused about calling this "page" level.

[EhsanK, 2017/01/26 19:16:29]

I think |rwhva_| is always a RenderWidgetHostViewAndroid (that is the type as
defined in the header); and that should always be the page's view? Also
RenderWidgetHostViewAndroid is the only thing which creates the
ImeAdapterAndroid (AFAIK) and passes its own reference as a RWHVA.

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

Ah yes, I was confused about how RWHVs work (with platform views at the top and
ChildFrame views for OOPIFs).

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

Acknowledged.

   if (!rwh)
     return nullptr;
 
   return RenderWidgetHostImpl::From(rwh);
 }
 
+RenderWidgetHostImpl* ImeAdapterAndroid::GetFocusedWidget() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);

[EhsanK, 2017/01/26 19:16:29]

If it wasn't for this threading DCHECK, I could remove GetFocusedWidget() and
use rwhva_->GetFocusedWidget() instead.

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

That DCHECK isn't strictly necessary, but given the amount of thought we're
putting in to which method is the right one to call, it's probably worth having
this helper method for it.

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

Acknowledged.

+  DCHECK(rwhva_);

[EhsanK, 2017/01/25 19:27:47]

This DCHECK here and everywhere else in this file seems unnecessary.  I only
added it to conform to the way code was before (specifically with
GetPageRenderWidgetHostImpl). It is also inconsistently used since in
SendKeyEvent we do not DCHECK.

This class is created by RWHVAndroid which passes a reference to itself in the
CTOR of this class. I would suggest we remove all such DCHECKs (either in this
CL or in another) and if needed, add one DCHECK to the CTOR. WDYT?

[Charlie Reis, 2017/01/25 22:00:22]

Looks like the only previous use of it is in GetPageRenderWidgetHostImpl, and I
agree that it's unnecessary there or here.  Moving it to the constructor in this
CL sounds good.

[EhsanK, 2017/01/26 19:16:29]

Acknowledged.

+  return rwhva_->GetFocusedWidget();

[Charlie Reis, 2017/01/25 22:00:22]

I'm getting confused looking at RenderWidgetHostViewBase::GetFocusedWidget(),
where this is implemented.  We pass the RWHV's widget into
WebContentsImpl::GetFocusedRenderWidgetHost, which returns that same widget if
it's not equal to the main frame's current widget (with a comment about popup
menus).

Wouldn't this be broken for OOPIFs?  In other words, if an OOPIF's RWHV calls
GetFocusedWidget, it will always get back its own RWH.  Am I reading that
incorrectly?

[EhsanK, 2017/01/26 19:16:29]

Since |rwhva_| is a RenderWidgetHostViewAndroid and is for the page, I believe
what we are doing here is correct. This is supposed to return the widget inside
the page (or under the tab's view) which takes keyboard focus. This is also the
same thing we have been doing for other platforms.

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

Thanks-- that was the same mixup I had above.

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

Acknowledged.

+}
+
 RenderFrameHost* ImeAdapterAndroid::GetFocusedFrame() {
-  RenderWidgetHostImpl* rwh = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwh = GetPageRenderWidgetHostImpl();

[Charlie Reis, 2017/01/25 22:00:22]

What's the reason for GetFocusedFrame() use GetPageRenderWidgetHostImpl()
instead of GetFocusedWidget()?  Is there a case that GetFocusedWidget() would
return null while there's a focused frame, or some other case that they differ?

[EhsanK, 2017/01/26 19:16:29]

I don't think we can get to RenderFrameHostImpl from RenderWidgetHostImpl at all
(can we?).
Yes, I believe there is. In WebContentsImpl::GetFocusedRenderWidgetHost() we
will get nullptr if the focused widget's view is nullptr. Therefore,
GetFocusedFrame()->GetRenderWidgetHost() could be non-nullptr when
GetFocusedWidget() returns null. I think in those cases we should not proceed
with keyboard input IPCs.

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

Ok.

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

Acknowledged.

   if (!rwh)
     return nullptr;
   RenderViewHost* rvh = RenderViewHost::From(rwh);
   if (!rvh)
     return nullptr;
   FrameTreeNode* focused_frame =
       rvh->GetDelegate()->GetFrameTree()->GetFocusedFrame();
   if (!focused_frame)
     return nullptr;
 
   return focused_frame->current_frame_host();
 }
 
 WebContents* ImeAdapterAndroid::GetWebContents() {
-  RenderWidgetHostImpl* rwh = GetRenderWidgetHostImpl();
+  RenderWidgetHostImpl* rwh = GetPageRenderWidgetHostImpl();

[Charlie Reis, 2017/01/25 22:00:22]

Again, just wondering whether we need GetPageRenderWidgetHostImpl at all.  It's
ok if so.

[EhsanK, 2017/01/26 19:16:29]

We actually don't since
RenderWidgetHostImpl::From(rwhva_->GetRenderWidgetHost()) could replace that. I
also used rwh->delegate()->GetAsWebContents() here since it will need only one
cast from RenderWidgetHostDelegate to WebContents. The way it is now casts
RenderWidgetHostOwnerDelegate to RenderViewHost and then RenderViewHostDelegate
to WebContents. WDYT?

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

Sure.

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

Acknowledged.

   if (!rwh)
     return nullptr;
   return WebContents::FromRenderViewHost(RenderViewHost::From(rwh));
 }
 
 std::vector<blink::WebCompositionUnderline>
 ImeAdapterAndroid::GetUnderlinesFromSpans(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj,
     const base::android::JavaParamRef<jobject>& text,
     const base::string16& text16) {
   std::vector<blink::WebCompositionUnderline> underlines;
   // Iterate over spans in |text|, dispatch those that we care about (e.g.,
   // BackgroundColorSpan) to a matching callback (e.g.,
   // AppendBackgroundColorSpan()), and populate |underlines|.
   Java_ImeAdapter_populateUnderlinesFromSpans(
       env, obj, text, reinterpret_cast<jlong>(&underlines));
 
   // Sort spans by |.startOffset|.
   std::sort(underlines.begin(), underlines.end());
 
   return underlines;
 }
 
 }  // namespace content