Support parsing BackgroundSpans and UnderlineSpans in Android IME's commitText()

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 c28f43fbb25cc12f7e27f112c49d6d12c08a90da..3a7d18b94da7ce41b8a4981e98e7d87b3eda70cc 100644
--- a/content/browser/renderer_host/ime_adapter_android.cc
+++ b/content/browser/renderer_host/ime_adapter_android.cc
@@ -148,20 +148,14 @@ void ImeAdapterAndroid::SetComposingText(JNIEnv* env,
 
   base::string16 text16 = ConvertJavaStringToUTF16(env, text_str);
 
-  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));
+  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));
   }
-  // Sort spans by |.startOffset|.
-  std::sort(underlines.begin(), underlines.end());
 
   // relative_cursor_pos is as described in the Android API for
   // InputConnection#setComposingText, whereas the parameters for
@@ -174,7 +168,8 @@ void ImeAdapterAndroid::SetComposingText(JNIEnv* env,
 }
 
 void ImeAdapterAndroid::CommitText(JNIEnv* env,

[rlanday, 2017/01/09 21:55:17]

In theory I think this method should make the Android BackgroundSpans and
UnderlineSpans persist until the text is overwritten...I could do this but I
don't know of anything that actually needs this functionality...

[aelias_OOO_until_Jul13, 2017/01/10 01:45:35]

I think that will likely be needed sooner or later (especially in mixed
voice/typing input modes), could you add a boolean to the
WebCompositionUnderline structure about whether it's persistent or not?

[rlanday, 2017/01/10 05:02:17]

That’s basically what I did in
https://codereview.chromium.org/2611813002

It’s kind of a sizable CL, should I just revive it?

[aelias_OOO_until_Jul13, 2017/01/10 21:46:30]

OK, this patch is large enough, let's land it as is instead of trying to include
more stuff just now.  Maybe the persistency boolean could be added in the same
patch that adds the list of strings for SuggestionSpan (since they're both
touching CompositionUnderline struct).

-                                   const JavaParamRef<jobject>&,
+                                   const JavaParamRef<jobject>& obj,
+                                   const JavaParamRef<jobject>& text,
                                    const JavaParamRef<jstring>& text_str,
                                    int relative_cursor_pos) {
   RenderWidgetHostImpl* rwhi = GetRenderWidgetHostImpl();
@@ -183,6 +178,9 @@ void ImeAdapterAndroid::CommitText(JNIEnv* env,
 
   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.
@@ -191,7 +189,8 @@ void ImeAdapterAndroid::CommitText(JNIEnv* env,
   else
     relative_cursor_pos -= text16.length();
 
-  rwhi->ImeCommitText(text16, gfx::Range::InvalidRange(), relative_cursor_pos);
+  rwhi->ImeCommitText(text16, underlines, gfx::Range::InvalidRange(),
+                      relative_cursor_pos);
 }
 
 void ImeAdapterAndroid::FinishComposingText(JNIEnv* env,
@@ -345,4 +344,23 @@ WebContents* ImeAdapterAndroid::GetWebContents() {
   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