android: Fix ContentViewCore and RenderWidgetHostViewAndroid lifetime issue

content/browser/android/content_view_core_impl.cc

 // Copyright 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/android/content_view_core_impl.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
@@ skipping 1661 matching lines @@
     const base::string16& result) {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   if (obj.is_null())
     return;
   ScopedJavaLocalRef<jstring> jresult = ConvertUTF16ToJavaString(env, result);
   Java_ContentViewCore_onSmartClipDataExtracted(
       env, obj.obj(), jresult.obj());
 }
 
+void ContentViewCoreImpl::OnDetachedFromWebContents(WebContents* web_contents) {

[Ted C, 2014/01/23 01:28:42]

Looking more at this, why don't you do this in ContentViewCoreImpl's destructor?

ContentViewCoreImpl is owned by the WebContents and will deleted when it is.  If
you look at ContentViewCoreImpl::ContentViewUserData, that sets up the
relationship and should be ensuring this is deleted before the web contents.

Maybe that would simplify this a bit?

[powei, 2014/01/23 01:38:02]

My first instinct was to put it in the destructor as well, but that did not
work.

I think the order is that the destructor of WebContentsImpl is called, then
ContentViewUserData goes out of scope and CVC gets deleted.  

[Ted C, 2014/01/23 02:00:15]

Hmm...that is unfortunate.  If that is the case, I'd rather make this class (or
even WebContentsViewAndroid) a WebContentsObserver instead of piggy backing on
the java bridge class.  There is actually no requirement that this class have a
WebContentsObserverAndroid attached to it (granted they always do), but I think
it would be cleaner for one of these two classes to maintain the state
internally instead of relying on a hidden property.

[powei, 2014/01/23 02:38:40]

Done.  Turns out ContentViewCoreImpl is already a WebContentsObserver.  Color me
embarrassed.

+  DCHECK_EQ(GetWebContents(), web_contents);
+
+  WebContentsViewAndroid* wcva =
+    static_cast<WebContentsViewAndroid*>(GetWebContents()->GetView());

[Ted C, 2014/01/23 01:28:42]

+2

[powei, 2014/01/23 01:38:02]

Done.

+  DCHECK(wcva);
+  wcva->SetContentViewCore(NULL);
+}
+
 // This is called for each ContentView.
 jlong Init(JNIEnv* env, jobject obj,
            jboolean hardware_accelerated,
            jlong native_web_contents,
            jlong view_android,
            jlong window_android) {
   ContentViewCoreImpl* view = new ContentViewCoreImpl(
       env, obj, hardware_accelerated,
       reinterpret_cast<WebContents*>(native_web_contents),
       reinterpret_cast<ui::ViewAndroid*>(view_android),
       reinterpret_cast<ui::WindowAndroid*>(window_android));
   return reinterpret_cast<intptr_t>(view);
 }
 
 bool RegisterContentViewCore(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace content