Refactoring ContentVideoViewContextDelegate.java

content/browser/android/content_video_view.h

 // Copyright (c) 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.
 
 #ifndef CONTENT_BROWSER_ANDROID_CONTENT_VIDEO_VIEW_H_
 #define CONTENT_BROWSER_ANDROID_CONTENT_VIDEO_VIEW_H_
 
 #include <jni.h>
 
 #include "base/android/scoped_java_ref.h"
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/timer.h"
 
 namespace content {
 
+class ContentViewCoreImpl;
 class MediaPlayerManagerAndroid;
 
 // Native mirror of ContentVideoView.java. This class is responsible for
 // creating the Java video view and pass all the player status change to
 // it. It accepts media control from Java class, and forwards it to
 // MediaPlayerManagerAndroid.
 class ContentVideoView {
  public:
   // Construct a ContentVideoView object. The |manager| will handle all the
   // playback controls from the Java class.
   explicit ContentVideoView(MediaPlayerManagerAndroid* manager);
   ~ContentVideoView();
 
   static bool RegisterContentVideoView(JNIEnv* env);
 
   // Getter method called by the Java class to get the media information.
   int GetVideoWidth(JNIEnv*, jobject obj) const;
   int GetVideoHeight(JNIEnv*, jobject obj) const;
   int GetDurationInMilliSeconds(JNIEnv*, jobject obj) const;
   int GetCurrentPosition(JNIEnv*, jobject obj) const;
   bool IsPlaying(JNIEnv*, jobject obj);
   void UpdateMediaMetadata(JNIEnv*, jobject obj);
 
   // Method to create and destroy the Java view.
   void DestroyContentVideoView();
-  void CreateContentVideoView();
+  void CreateContentVideoView(
+      base::android::ScopedJavaLocalRef<jobject> content_video_view_client);
 
   // Called when the Java fullscreen view is destroyed. If
   // |release_media_player| is true, |manager_| needs to release the player
   // as we are quitting the app.
   void ExitFullscreen(JNIEnv*, jobject, jboolean release_media_player);
 
   // Media control method called by the Java class.
   void SeekTo(JNIEnv*, jobject obj, jint msec);
   void Play(JNIEnv*, jobject obj);
   void Pause(JNIEnv*, jobject obj);
 
   // Called by the Java class to pass the surface object to the player.
   void SetSurface(JNIEnv*, jobject obj, jobject surface);
 
   // Method called by |manager_| to inform the Java class about player status
   // change.
   void UpdateMediaMetadata();
   void OnMediaPlayerError(int errorType);
   void OnVideoSizeChanged(int width, int height);
   void OnBufferingUpdate(int percent);
   void OnPlaybackComplete();
 
  private:
   // Object that manages the fullscreen media player. It is responsible for
   // handling all the playback controls.
   MediaPlayerManagerAndroid* manager_;
 
   // Reference to the Java object.
   base::android::ScopedJavaGlobalRef<jobject> j_content_video_view_;

[joth, 2013/04/05 01:56:40]

this is a problem: holding a strong ref to the java object will stop it being
GC'd so long as we're in "full screen" (which may mean nothing, depending on how
the app is structured). this in turn may mean the WebView itself never gets
GCed. (As AwContentsClient indirectly references it).

So this should be a WeakJavaRef thingy, and all the usage of it use the "promote
to ScopedLocalRef and check that for null" pattern.

Although: it's only by fixing this problem that you will cause the
possibly-too-early GC of ContentVideoViewClient problem I mentioned above, so
perhaps best to solve this piece in a separate CL. (could raise a bug for it, it
should be a very rare corner leak case so no immediate priority)

[michaelbai, 2013/04/05 21:06:56]

Why WebView should be GCed? I think we need to back to WebView once exit full
screen video.  

 
   DISALLOW_COPY_AND_ASSIGN(ContentVideoView);
 };
 
 } // namespace content
 
 #endif  // CONTENT_BROWSER_ANDROID_CONTENT_VIDEO_VIEW_H_