diff options
author | boliu <boliu@chromium.org> | 2015-04-08 10:07:05 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-08 17:08:36 +0000 |
commit | 684ba03ae718cb7ffa9dc0720d011f941c084da3 (patch) | |
tree | 3a16c9ddd227675c713c2c1c5eb753fadc2217aa | |
parent | 54dea7dc6a96261555f9be42d648f46e6d709e47 (diff) | |
download | chromium_src-684ba03ae718cb7ffa9dc0720d011f941c084da3.zip chromium_src-684ba03ae718cb7ffa9dc0720d011f941c084da3.tar.gz chromium_src-684ba03ae718cb7ffa9dc0720d011f941c084da3.tar.bz2 |
Null check ContentViewCore::GetJavaObject
ContentViewCoreImpl holds a weak reference to the java peer. This means
reference returned by GetJavaObject may be null.
Go through all callers of this API and make sure they correctly handle
null reference.
BUG=469803
Review URL: https://codereview.chromium.org/1067023005
Cr-Commit-Position: refs/heads/master@{#324243}
9 files changed, 68 insertions, 26 deletions
diff --git a/android_webview/lib/main/aw_main_delegate.cc b/android_webview/lib/main/aw_main_delegate.cc index 27e5da9..8520816 100644 --- a/android_webview/lib/main/aw_main_delegate.cc +++ b/android_webview/lib/main/aw_main_delegate.cc @@ -195,7 +195,7 @@ AwMessagePortService* AwMainDelegate::CreateAwMessagePortService() { content::ExternalVideoSurfaceContainer* AwMainDelegate::CreateExternalVideoSurfaceContainer( content::WebContents* web_contents) { - return new ExternalVideoSurfaceContainerImpl(web_contents); + return ExternalVideoSurfaceContainerImpl::Create(web_contents); } #endif diff --git a/android_webview/native/external_video_surface_container_impl.cc b/android_webview/native/external_video_surface_container_impl.cc index 84d5ad4..64270ef 100644 --- a/android_webview/native/external_video_surface_container_impl.cc +++ b/android_webview/native/external_video_surface_container_impl.cc @@ -14,15 +14,23 @@ using content::ContentViewCore; namespace android_webview { -ExternalVideoSurfaceContainerImpl::ExternalVideoSurfaceContainerImpl( +// static +ExternalVideoSurfaceContainerImpl* ExternalVideoSurfaceContainerImpl::Create( content::WebContents* web_contents) { ContentViewCore* cvc = ContentViewCore::FromWebContents(web_contents); - if (cvc) { - JNIEnv* env = AttachCurrentThread(); - jobject_.Reset( - Java_ExternalVideoSurfaceContainer_create( - env, reinterpret_cast<intptr_t>(this), cvc->GetJavaObject().obj())); - } + if (!cvc) + return nullptr; + base::android::ScopedJavaLocalRef<jobject> jcvc = cvc->GetJavaObject(); + if (jcvc.is_null()) + return nullptr; + return new ExternalVideoSurfaceContainerImpl(jcvc); +} + +ExternalVideoSurfaceContainerImpl::ExternalVideoSurfaceContainerImpl( + base::android::ScopedJavaLocalRef<jobject> java_content_view_core) { + JNIEnv* env = AttachCurrentThread(); + jobject_.Reset(Java_ExternalVideoSurfaceContainer_create( + env, reinterpret_cast<intptr_t>(this), java_content_view_core.obj())); } ExternalVideoSurfaceContainerImpl::~ExternalVideoSurfaceContainerImpl() { diff --git a/android_webview/native/external_video_surface_container_impl.h b/android_webview/native/external_video_surface_container_impl.h index 29e7889..48ccd94 100644 --- a/android_webview/native/external_video_surface_container_impl.h +++ b/android_webview/native/external_video_surface_container_impl.h @@ -20,7 +20,8 @@ class ExternalVideoSurfaceContainerImpl typedef base::Callback<void(int, jobject)> SurfaceCreatedCB; typedef base::Callback<void(int)> SurfaceDestroyedCB; - ExternalVideoSurfaceContainerImpl(content::WebContents* contents); + static ExternalVideoSurfaceContainerImpl* Create( + content::WebContents* web_contents); // ExternalVideoSurfaceContainer implementation. void RequestExternalVideoSurface( @@ -39,6 +40,8 @@ class ExternalVideoSurfaceContainerImpl void SurfaceDestroyed(JNIEnv* env, jobject obj, jint player_id); private: + explicit ExternalVideoSurfaceContainerImpl( + base::android::ScopedJavaLocalRef<jobject> java_content_view_core); ~ExternalVideoSurfaceContainerImpl() override; base::android::ScopedJavaGlobalRef<jobject> jobject_; diff --git a/chromecast/browser/android/external_video_surface_container_impl.cc b/chromecast/browser/android/external_video_surface_container_impl.cc index e1f7e8b..e830575 100644 --- a/chromecast/browser/android/external_video_surface_container_impl.cc +++ b/chromecast/browser/android/external_video_surface_container_impl.cc @@ -11,17 +11,24 @@ namespace chromecast { namespace shell { - -ExternalVideoSurfaceContainerImpl::ExternalVideoSurfaceContainerImpl( +// static +ExternalVideoSurfaceContainerImpl* ExternalVideoSurfaceContainerImpl::Create( content::WebContents* web_contents) { content::ContentViewCore* cvc = content::ContentViewCore::FromWebContents(web_contents); - if (cvc) { - JNIEnv* env = base::android::AttachCurrentThread(); - jobject_.Reset( - Java_ExternalVideoSurfaceContainer_create( - env, reinterpret_cast<intptr_t>(this), cvc->GetJavaObject().obj())); - } + if (!cvc) + return nullptr; + base::android::ScopedJavaLocalRef<jobject> jcvc = cvc->GetJavaObject(); + if (jcvc.is_null()) + return nullptr; + return new ExternalVideoSurfaceContainerImpl(jcvc); +} + +ExternalVideoSurfaceContainerImpl::ExternalVideoSurfaceContainerImpl( + base::android::ScopedJavaLocalRef<jobject> java_content_view_core) { + JNIEnv* env = base::android::AttachCurrentThread(); + jobject_.Reset(Java_ExternalVideoSurfaceContainer_create( + env, reinterpret_cast<intptr_t>(this), java_content_view_core.obj())); } ExternalVideoSurfaceContainerImpl::~ExternalVideoSurfaceContainerImpl() { diff --git a/chromecast/browser/android/external_video_surface_container_impl.h b/chromecast/browser/android/external_video_surface_container_impl.h index 434071d..d864d06 100644 --- a/chromecast/browser/android/external_video_surface_container_impl.h +++ b/chromecast/browser/android/external_video_surface_container_impl.h @@ -21,7 +21,8 @@ class ExternalVideoSurfaceContainerImpl typedef base::Callback<void(int, jobject)> SurfaceCreatedCB; typedef base::Callback<void(int)> SurfaceDestroyedCB; - ExternalVideoSurfaceContainerImpl(content::WebContents* contents); + static ExternalVideoSurfaceContainerImpl* Create( + content::WebContents* web_contents); // ExternalVideoSurfaceContainer implementation. void RequestExternalVideoSurface( @@ -40,6 +41,8 @@ class ExternalVideoSurfaceContainerImpl void SurfaceDestroyed(JNIEnv* env, jobject obj, jint player_id); private: + explicit ExternalVideoSurfaceContainerImpl( + base::android::ScopedJavaLocalRef<jobject> java_content_view_core); ~ExternalVideoSurfaceContainerImpl() override; base::android::ScopedJavaGlobalRef<jobject> jobject_; diff --git a/chromecast/browser/cast_content_browser_client.cc b/chromecast/browser/cast_content_browser_client.cc index 2fc59c2d..9fe57ce 100644 --- a/chromecast/browser/cast_content_browser_client.cc +++ b/chromecast/browser/cast_content_browser_client.cc @@ -327,7 +327,7 @@ void CastContentBrowserClient::GetAdditionalMappedFilesForChildProcess( content::ExternalVideoSurfaceContainer* CastContentBrowserClient::OverrideCreateExternalVideoSurfaceContainer( content::WebContents* web_contents) { - return new ExternalVideoSurfaceContainerImpl(web_contents); + return ExternalVideoSurfaceContainerImpl::Create(web_contents); } #endif // defined(OS_ANDROID) && defined(VIDEO_HOLE) diff --git a/components/web_contents_delegate_android/validation_message_bubble_android.cc b/components/web_contents_delegate_android/validation_message_bubble_android.cc index 754b608..a577636 100644 --- a/components/web_contents_delegate_android/validation_message_bubble_android.cc +++ b/components/web_contents_delegate_android/validation_message_bubble_android.cc @@ -17,12 +17,15 @@ using content::RenderWidgetHost; namespace { -inline ContentViewCore* GetContentViewCoreFrom(RenderWidgetHost* widget_host) { - return ContentViewCore::FromWebContents( - content::WebContents::FromRenderViewHost( +base::android::ScopedJavaLocalRef<jobject> GetJavaContentViewCoreFrom( + RenderWidgetHost* widget_host) { + ContentViewCore* content_view_core = + ContentViewCore::FromWebContents(content::WebContents::FromRenderViewHost( content::RenderViewHost::From(widget_host))); + if (!content_view_core) + return base::android::ScopedJavaLocalRef<jobject>(); + return content_view_core->GetJavaObject(); } - } namespace web_contents_delegate_android { @@ -32,11 +35,16 @@ ValidationMessageBubbleAndroid::ValidationMessageBubbleAndroid( const gfx::Rect& anchor_in_root_view, const base::string16& main_text, const base::string16& sub_text) { + base::android::ScopedJavaLocalRef<jobject> java_content_view_core = + GetJavaContentViewCoreFrom(widget_host); + if (java_content_view_core.is_null()) + return; + JNIEnv* env = base::android::AttachCurrentThread(); java_validation_message_bubble_.Reset( Java_ValidationMessageBubble_createAndShow( env, - GetContentViewCoreFrom(widget_host)->GetJavaObject().obj(), + java_content_view_core.obj(), anchor_in_root_view.x(), anchor_in_root_view.y(), anchor_in_root_view.width(), @@ -52,10 +60,15 @@ ValidationMessageBubbleAndroid::~ValidationMessageBubbleAndroid() { void ValidationMessageBubbleAndroid::SetPositionRelativeToAnchor( RenderWidgetHost* widget_host, const gfx::Rect& anchor_in_root_view) { + base::android::ScopedJavaLocalRef<jobject> java_content_view_core = + GetJavaContentViewCoreFrom(widget_host); + if (java_content_view_core.is_null()) + return; + Java_ValidationMessageBubble_setPositionRelativeToAnchor( base::android::AttachCurrentThread(), java_validation_message_bubble_.obj(), - GetContentViewCoreFrom(widget_host)->GetJavaObject().obj(), + java_content_view_core.obj(), anchor_in_root_view.x(), anchor_in_root_view.y(), anchor_in_root_view.width(), diff --git a/content/browser/android/content_video_view.cc b/content/browser/android/content_video_view.cc index 1081bd6..2a5ce04 100644 --- a/content/browser/android/content_video_view.cc +++ b/content/browser/android/content_video_view.cc @@ -198,11 +198,17 @@ ScopedJavaLocalRef<jobject> ContentVideoView::GetJavaObject(JNIEnv* env) { JavaObjectWeakGlobalRef ContentVideoView::CreateJavaObject() { ContentViewCore* content_view_core = manager_->GetContentViewCore(); JNIEnv* env = AttachCurrentThread(); + + base::android::ScopedJavaLocalRef<jobject> j_content_view_core = + content_view_core->GetJavaObject(); + if (j_content_view_core.is_null()) + return JavaObjectWeakGlobalRef(env, nullptr); + return JavaObjectWeakGlobalRef( env, Java_ContentVideoView_createContentVideoView( env, - content_view_core->GetJavaObject().obj(), + j_content_view_core.obj(), reinterpret_cast<intptr_t>(this)).obj()); } } // namespace content diff --git a/content/public/browser/android/content_view_core.h b/content/public/browser/android/content_view_core.h index f64f672..e528090 100644 --- a/content/public/browser/android/content_view_core.h +++ b/content/public/browser/android/content_view_core.h @@ -45,6 +45,8 @@ class CONTENT_EXPORT ContentViewCore { static ContentViewCore* GetNativeContentViewCore(JNIEnv* env, jobject obj); virtual WebContents* GetWebContents() const = 0; + + // May return null reference. virtual base::android::ScopedJavaLocalRef<jobject> GetJavaObject() = 0; virtual ui::ViewAndroid* GetViewAndroid() const = 0; virtual ui::WindowAndroid* GetWindowAndroid() const = 0; |