Refactor ContentViewClient (1/6)

content/browser/android/content_video_view.cc

 // 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.
 
 #include "content/browser/android/content_video_view.h"
 
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
 #include "content/browser/android/content_view_core_impl.h"
 #include "content/browser/media/android/browser_media_player_manager.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/content_switches.h"
 #include "jni/ContentVideoView_jni.h"
 
 using base::android::AttachCurrentThread;
 using base::android::CheckException;
 using base::android::JavaParamRef;
+using base::android::JavaRef;
 using base::android::ScopedJavaGlobalRef;
 using base::android::ScopedJavaLocalRef;
 using base::UserMetricsAction;
 using content::RecordAction;
 
 namespace content {
 
 namespace {
 // There can only be one content video view at a time, this holds onto that
 // singleton instance.
@@ skipping 13 matching lines @@
 bool ContentVideoView::RegisterContentVideoView(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 ContentVideoView* ContentVideoView::GetInstance() {
   return g_content_video_view;
 }
 
 ContentVideoView::ContentVideoView(Client* client,
                                    ContentViewCore* content_view_core,
+                                   const JavaRef<jobject>& video_embedder,
                                    const gfx::Size& video_natural_size)
     : client_(client), weak_factory_(this) {
   DCHECK(!g_content_video_view);
   j_content_video_view_ =
-      CreateJavaObject(content_view_core, video_natural_size);
+      CreateJavaObject(content_view_core, video_embedder, video_natural_size);
   g_content_video_view = this;
 }
 
 ContentVideoView::~ContentVideoView() {
   DCHECK(g_content_video_view);
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> content_video_view = GetJavaObject(env);
   if (!content_video_view.is_null()) {
     Java_ContentVideoView_destroyContentVideoView(env, content_video_view,
                                                   true);
@@ skipping 86 matching lines @@
     }
   } else {
     UMA_HISTOGRAM_COUNTS(
         "MobileFullscreenVideo.LandscapeDuration",
         playback_duration_in_milliseconds_before_orientation_change);
     UMA_HISTOGRAM_COUNTS(
         "MobileFullscreenVideo.LandscapeRotation", orientation_changed);
   }
 }
 
 JavaObjectWeakGlobalRef ContentVideoView::CreateJavaObject(

[boliu, 2016/09/26 21:25:24]

issue with existing code, so doesn't need to be fixed here

this is broken. afaict nothing is keeping a strong ref to this (during creation
at least), so this could get collected at any time

[Jinsuk Kim, 2016/09/27 10:08:58]

Acknowledged. I'll leave a TODO for a reminder before landing.

     ContentViewCore* content_view_core,
+    const JavaRef<jobject>& j_content_video_view_embedder,
     const gfx::Size& video_natural_size) {
   JNIEnv* env = AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jobject> j_content_view_core =
       content_view_core->GetJavaObject();
-  if (j_content_view_core.is_null())
+
+  if (j_content_view_core.is_null() || j_content_video_view_embedder.is_null())
     return JavaObjectWeakGlobalRef(env, nullptr);
 
   return JavaObjectWeakGlobalRef(
       env, Java_ContentVideoView_createContentVideoView(
-               env, j_content_view_core, reinterpret_cast<intptr_t>(this),
+               env, j_content_view_core, j_content_video_view_embedder,
+               reinterpret_cast<intptr_t>(this),
                video_natural_size.width(), video_natural_size.height())
                .obj());
 }
 }  // namespace content