[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() {
+  // Needs to be destroyed on the same thread as the RequestTextCheck and
+  // ProcessSpellCheckResults are executed, which is the UI thread.
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!java_object_.is_null()) {
+    Java_SpellCheckerSessionBridge_disconnect(
+        base::android::AttachCurrentThread(), java_object_);
+  }
+}
 
 // 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 51 matching lines @@
   content::RenderProcessHost* sender =
       content::RenderProcessHost::FromID(render_process_id_);
 
   if (sender != nullptr) {
     sender->Send(new SpellCheckMsg_RespondTextCheck(
         active_request_->route_id, active_request_->identifier,
         active_request_->text, results));
   }
 
   active_request_ = std::move(pending_request_);
   if (active_request_) {

[Tobias Sargeant, 2016/09/30 09:18:57]

This should handle java_object_ being null. If it's possible to continue after
DisconnectSession, then it should be reusing code from RequestTextCheck to set
up the java bridge object again.

It also seems a bit weird that OnToggleSpellcheck only disconnects, and doesn't
(allow) reconnect.

[timvolodine, 2016/09/30 13:57:54]

I guess the reconnect part is 'on demand' in RequestTextCheck. So now the
DisconnectSession also cleans up the java side and I believe it's not possible
for ProcessSpellCheckResults to be executed after disconnect.

     JNIEnv* env = base::android::AttachCurrentThread();
     Java_SpellCheckerSessionBridge_requestTextCheck(
         env, java_object_,
         base::android::ConvertUTF16ToJavaString(env, active_request_->text));
   }
 }
 
 void SpellCheckerSessionBridge::DisconnectSession() {
   java_object_.Reset();

[Tobias Sargeant, 2016/09/30 09:18:58]

This also needs to call Java_SpellCheckerSessionBridge_disconnect, because the
associated java object isn't necessarily GCed instantly, and after this we've
lost our ability to zero the java side pointer in the destructor.

[timvolodine, 2016/09/30 13:57:55]

yes sounds right, thanks for catching this. if we disconnect and then close tab
the same issue can arise.
Done.

 }
 
 SpellCheckerSessionBridge::SpellingRequest::SpellingRequest(
     int route_id,
     int identifier,
     const base::string16& text)
     : route_id(route_id), identifier(identifier), text(text) {}
 
 SpellCheckerSessionBridge::SpellingRequest::~SpellingRequest() {}