Refactoring ContentVideoViewContextDelegate.java

content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

Index: content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
index 9b8cde61ef94cdbf6f262c24e2cc6553a9fc680b..f44c61a8d0b5e576a5d5eb408c6a163c54bd5d47 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -268,6 +268,7 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
     /**
      * @return The context used for creating this ContentViewCore.
      */
+    @CalledByNative
     public Context getContext() {
         return mContext;
     }
@@ -2645,6 +2646,11 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
         getContentViewClient().onExternalVideoSurfaceRequested(playerId);

[joth, 2013/04/12 17:56:24]

TODO move this method to ContentVideoClient?

[michaelbai, 2013/04/18 18:21:59]

This is used by Google TV, not sure we should move it to ContentVideoViewClient.

On 2013/04/12 17:56:24, joth wrote:

     }
 
+    @CalledByNative
+    private ContentVideoViewClient getContentVideoViewClient() {
+        return mContentViewClient.getContentVideoViewClient();

[joth, 2013/04/12 17:56:24]

as the native side only holds a weak ref, I think you'll need to keep an
appropriately life-timed  strong ref on the java side to stop it getting
premeturely GCed.
Stashing that ref in ContentViewCore, here, would make sense.
if (mContentViewClient == null) mContentViewClient = 
mContentViewClient.getContentVideoViewClient();
return mContentViewClient;

incude a comment on the member to explain it is required for ensuring correct
lifetime of the java object

(mRetainedJavaScriptObjects is doing something similar, on a larger scale)

[michaelbai, 2013/04/18 18:21:59]

I don't understand why it could be prematurely GCed, ContentVideoViewClient will
be passed all the way to the ContentVideoView which hold a strong ref to it. All
this happened before the native side have weak reference to ContentVideoView.


On 2013/04/12 17:56:24, joth wrote:

+    }
+
     private native int nativeInit(boolean hardwareAccelerated, boolean inputEventsDeliveredAtVSync,
             int webContentsPtr, int windowAndroidPtr);