[Android] Fix spellcheck JNI crash when a tab is closed.

components/spellcheck/browser/spellchecker_session_bridge_android.cc

 // Copyright 2015 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 "components/spellcheck/browser/spellchecker_session_bridge_android.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "components/spellcheck/common/spellcheck_messages.h"
 #include "components/spellcheck/common/spellcheck_result.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
 #include "jni/SpellCheckerSessionBridge_jni.h"
 
 using base::android::JavaParamRef;
 
 SpellCheckerSessionBridge::SpellCheckerSessionBridge(int render_process_id)
     : render_process_id_(render_process_id),
       java_object_initialization_failed_(false) {}
 
-SpellCheckerSessionBridge::~SpellCheckerSessionBridge() {}
+SpellCheckerSessionBridge::~SpellCheckerSessionBridge() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  // Clean-up java side to avoid any stale JNI callbacks.
+  DisconnectSession();
+}
 
 // static
 bool SpellCheckerSessionBridge::RegisterJNI(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 void SpellCheckerSessionBridge::RequestTextCheck(int route_id,
                                                  int identifier,
                                                  const base::string16& text) {
   // SpellCheckerSessionBridge#create() will return null if spell checker
@@ skipping 24 matching lines @@
     return;
   }
 
   active_request_.reset(new SpellingRequest(route_id, identifier, text));
 
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_SpellCheckerSessionBridge_requestTextCheck(
       env, java_object_, base::android::ConvertUTF16ToJavaString(env, text));
 }
 
 void SpellCheckerSessionBridge::ProcessSpellCheckResults(

[Tobias Sargeant, 2016/09/30 14:13:21]

DCHECK that this occurs on the UI thread, to document that expectation?

[timvolodine, 2016/09/30 14:46:00]

Done.

     JNIEnv* env,
     const JavaParamRef<jobject>& jobj,
     const JavaParamRef<jintArray>& offset_array,
     const JavaParamRef<jintArray>& length_array) {
   std::vector<int> offsets;
   std::vector<int> lengths;
 
   base::android::JavaIntArrayToIntVector(env, offset_array, &offsets);
   base::android::JavaIntArrayToIntVector(env, length_array, &lengths);
 
@@ skipping 14 matching lines @@
 
   active_request_ = std::move(pending_request_);
   if (active_request_) {
     JNIEnv* env = base::android::AttachCurrentThread();
     Java_SpellCheckerSessionBridge_requestTextCheck(
         env, java_object_,
         base::android::ConvertUTF16ToJavaString(env, active_request_->text));
   }
 }
 
 void SpellCheckerSessionBridge::DisconnectSession() {

[Tobias Sargeant, 2016/09/30 14:13:21]

If there's an active request at this point that we'll no longer receive, do we
need to fire a SpellCheckMsg_RespondTextCheck?

If a reconnect can happen, we should probably also clear active_request_ and
pending_request_ here, especially as active_request_ will never return, and so
no other requests will be sent to the java bridge.

[timvolodine, 2016/09/30 14:46:00]

Don't think we need to send an (empty?) IPC in the disconnect case. This is
similar to the case when no spellchecker is available for some reason.

Yes added clearing of both active and pending requests --> done.

-  java_object_.Reset();
+  // Needs to be executed on the same thread as the RequestTextCheck and
+  // ProcessSpellCheckResults methods, which is the UI thread.
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!java_object_.is_null()) {
+    Java_SpellCheckerSessionBridge_disconnect(
+        base::android::AttachCurrentThread(), java_object_);
+    java_object_.Reset();
+  }
 }
 
 SpellCheckerSessionBridge::SpellingRequest::SpellingRequest(
     int route_id,
     int identifier,
     const base::string16& text)
     : route_id(route_id), identifier(identifier), text(text) {}
 
 SpellCheckerSessionBridge::SpellingRequest::~SpellingRequest() {}