Android: Create Java TextSuggestionHost lazily

This CL addresses the reported bug by undoing the quick fix
http://crrev.com/595608 and lazily initializing Java side
TextSuggestionHost properly. Native side still needs to be
established eagerly to get the IPC to work. Now Java layer
is created before suggestion menu is requested to be shown
for the first time.

Bug: 884214
Change-Id: I1bbe9b2e3a5898d6dd082f8784951215a4a9ab91
Reviewed-on: https://chromium-review.googlesource.com/c/1256383
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596487}

content/browser/android/ime_adapter_android.cc

@@ -14,6 +14,7 @@
 #include "base/android/scoped_java_ref.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "content/browser/android/text_suggestion_host_android.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
@@ -144,6 +145,10 @@ ImeAdapterAndroid::ImeAdapterAndroid(JNIEnv* env,
                                      WebContents* web_contents)
     : RenderWidgetHostConnector(web_contents), rwhva_(nullptr) {
   java_ime_adapter_ = JavaObjectWeakGlobalRef(env, obj);
+
+  // Set up mojo client for TextSuggestionHost in advance. Java side is
+  // initialized lazily right before showing the menu first time.
+  TextSuggestionHostAndroid::Create(env, web_contents);
 }
 
 ImeAdapterAndroid::~ImeAdapterAndroid() {

content/browser/android/text_suggestion_host_android.cc

@@ -34,25 +34,17 @@ const size_t kMaxNumberOfSuggestions = 5;
 
 }  // namespace
 
-jlong JNI_TextSuggestionHost_Init(JNIEnv* env,
-                                  const JavaParamRef<jobject>& obj,
-                                  const JavaParamRef<jobject>& jweb_contents) {
-  WebContents* web_contents = WebContents::FromJavaWebContents(jweb_contents);
-  DCHECK(web_contents);
-  auto* text_suggestion_host =
-      new TextSuggestionHostAndroid(env, obj, web_contents);
+void TextSuggestionHostAndroid::Create(JNIEnv* env, WebContents* web_contents) {
+  auto* text_suggestion_host = new TextSuggestionHostAndroid(env, web_contents);
   text_suggestion_host->Initialize();
-  return reinterpret_cast<intptr_t>(text_suggestion_host);
 }
 
 TextSuggestionHostAndroid::TextSuggestionHostAndroid(
     JNIEnv* env,
-    const JavaParamRef<jobject>& obj,
     WebContents* web_contents)
     : RenderWidgetHostConnector(web_contents),
       WebContentsObserver(web_contents),
       rwhva_(nullptr),
-      java_text_suggestion_host_(JavaObjectWeakGlobalRef(env, obj)),
       suggestion_menu_timeout_(
           base::Bind(&TextSuggestionHostAndroid::OnSuggestionMenuTimeout,
                      base::Unretained(this))) {
@@ -144,6 +136,19 @@ double TextSuggestionHostAndroid::DpToPxIfNeeded(double value) {
   return value;
 }
 
+ScopedJavaLocalRef<jobject>
+TextSuggestionHostAndroid::GetJavaTextSuggestionHost() {
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj = java_text_suggestion_host_.get(env);
+  if (obj.is_null()) {
+    obj = Java_TextSuggestionHost_create(
+        env, WebContentsObserver::web_contents()->GetJavaWebContents(),
+        reinterpret_cast<intptr_t>(this));
+    java_text_suggestion_host_ = JavaObjectWeakGlobalRef(env, obj);
+  }
+  return obj;
+}
+
 void TextSuggestionHostAndroid::ShowSpellCheckSuggestionMenu(
     double caret_x,
     double caret_y,
@@ -155,7 +160,7 @@ void TextSuggestionHostAndroid::ShowSpellCheckSuggestionMenu(
   for (size_t i = 0; i < suggestions.size() && i < kMaxNumberOfSuggestions; ++i)
     suggestion_strings.push_back(suggestions[i]->suggestion);
   JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> obj = java_text_suggestion_host_.get(env);
+  ScopedJavaLocalRef<jobject> obj = GetJavaTextSuggestionHost();
   if (obj.is_null())
     return;
 
@@ -173,7 +178,9 @@ void TextSuggestionHostAndroid::ShowTextSuggestionMenu(
     const std::string& marked_text,
     const std::vector<blink::mojom::TextSuggestionPtr>& suggestions) {
   JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> obj = java_text_suggestion_host_.get(env);
+  ScopedJavaLocalRef<jobject> obj = GetJavaTextSuggestionHost();
+  if (obj.is_null())
+    return;
 
   // Enforce kMaxNumberOfSuggestions here in case the renderer is hijacked and
   // tries to send bad input.

content/browser/android/text_suggestion_host_android.h

@@ -18,13 +18,13 @@ namespace content {
 // misspelled word. This class creates the Android implementation of
 // mojom::TextSuggestionHost, which is used to communicate back-and-forth with
 // Blink side code (these are separate classes due to lifecycle considerations;
-// this class needs to be constructed from Java code, but Mojo code wants to
-// take ownership of mojom::TextSuggestionHost).
+// this class is created by ImeAdapterAndroid ctor and destroyed together with
+// WebContents. Mojo code takes ownership of mojom::TextSuggestionHost).
 class TextSuggestionHostAndroid : public RenderWidgetHostConnector,
                                   public WebContentsObserver {
  public:
+  static void Create(JNIEnv* env, WebContents* web_contents);
   TextSuggestionHostAndroid(JNIEnv* env,
-                            const base::android::JavaParamRef<jobject>& obj,
                             WebContents* web_contents);
   ~TextSuggestionHostAndroid() override;
 
@@ -95,6 +95,7 @@ class TextSuggestionHostAndroid : public RenderWidgetHostConnector,
 
  private:
   RenderFrameHost* GetFocusedFrame();
+  base::android::ScopedJavaLocalRef<jobject> GetJavaTextSuggestionHost();
   const blink::mojom::TextSuggestionBackendPtr& GetTextSuggestionBackend();
   // Used by the spell check menu timer to notify Blink that the timer has
   // expired.

content/public/android/java/src/org/chromium/content/browser/input/TextSuggestionHost.java

@@ -48,11 +48,19 @@ public class TextSuggestionHost implements WindowEventObserver, HideablePopup, U
      * @param webContents {@link WebContents} object.
      * @return {@link TextSuggestionHost} object.
      */
-    public static TextSuggestionHost fromWebContents(WebContents webContents) {
+    @VisibleForTesting
+    static TextSuggestionHost fromWebContents(WebContents webContents) {
         return ((WebContentsImpl) webContents)
                 .getOrSetUserData(TextSuggestionHost.class, UserDataFactoryLazyHolder.INSTANCE);
     }
 
+    @CalledByNative
+    private static TextSuggestionHost create(WebContents webContents, long nativePtr) {
+        TextSuggestionHost host = fromWebContents(webContents);
+        host.setNativePtr(nativePtr);
+        return host;
+    }
+
     /**
      * Create {@link TextSuggestionHost} instance.
      * @param webContents WebContents instance.
@@ -63,11 +71,14 @@ public class TextSuggestionHost implements WindowEventObserver, HideablePopup, U
         mWindowAndroid = mWebContents.getTopLevelNativeWindow();
         mViewDelegate = mWebContents.getViewAndroidDelegate();
         assert mViewDelegate != null;
-        mNativeTextSuggestionHost = nativeInit(mWebContents);
         PopupController.register(mWebContents, this);
         WindowEventObserverManager.from(mWebContents).addObserver(this);
     }
 
+    private void setNativePtr(long nativePtr) {
+        mNativeTextSuggestionHost = nativePtr;
+    }
+
     private float getContentOffsetYPix() {
         return mWebContents.getRenderCoordinates().getContentOffsetYPix();
     }
@@ -221,7 +232,6 @@ public class TextSuggestionHost implements WindowEventObserver, HideablePopup, U
         return mSpellCheckPopupWindow;
     }
 
-    private native long nativeInit(WebContents webContents);
     private native void nativeApplySpellCheckSuggestion(
             long nativeTextSuggestionHostAndroid, String suggestion);
     private native void nativeApplyTextSuggestion(

content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java

@@ -34,7 +34,6 @@ import org.chromium.content.browser.WindowEventObserverManager;
 import org.chromium.content.browser.accessibility.WebContentsAccessibilityImpl;
 import org.chromium.content.browser.framehost.RenderFrameHostDelegate;
 import org.chromium.content.browser.framehost.RenderFrameHostImpl;
-import org.chromium.content.browser.input.TextSuggestionHost;
 import org.chromium.content.browser.selection.SelectionPopupControllerImpl;
 import org.chromium.content_public.browser.AccessibilitySnapshotCallback;
 import org.chromium.content_public.browser.AccessibilitySnapshotNode;
@@ -230,7 +229,6 @@ public class WebContentsImpl implements WebContents, RenderFrameHostDelegate, Wi
 
         ViewEventSinkImpl.from(this).setAccessDelegate(accessDelegate);
         getRenderCoordinates().setDeviceScaleFactor(windowAndroid.getDisplay().getDipScale());
-        TextSuggestionHost.fromWebContents(this);
     }
 
     @Nullable