Fix browser crash when calling MediaPlayerBridge::PrepareAsync

media/base/android/media_player_bridge.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 "media/base/android/media_player_bridge.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ skipping 100 matching lines @@
   }
 
   JNIEnv* env = base::android::AttachCurrentThread();
   CHECK(env);
 
   Java_MediaPlayerBridge_setSurface(
       env, j_media_player_bridge_.obj(), surface.j_surface().obj());
 }
 
 void MediaPlayerBridge::Prepare() {
-  if (j_media_player_bridge_.is_null())
-    CreateJavaMediaPlayerBridge();
+  DCHECK(j_media_player_bridge_.is_null());
+  CreateJavaMediaPlayerBridge();
   if (url_.SchemeIsFileSystem()) {
     manager()->GetMediaResourceGetter()->GetPlatformPathFromFileSystemURL(
             url_, base::Bind(&MediaPlayerBridge::SetDataSource,
                              weak_this_.GetWeakPtr()));
   } else {
     SetDataSource(url_.spec());
   }
 }
 
 void MediaPlayerBridge::SetDataSource(const std::string& url) {
   if (j_media_player_bridge_.is_null())
     return;
 
   JNIEnv* env = base::android::AttachCurrentThread();
   CHECK(env);
 
   // Create a Java String for the URL.
   ScopedJavaLocalRef<jstring> j_url_string = ConvertUTF8ToJavaString(env, url);
   ScopedJavaLocalRef<jstring> j_cookies = ConvertUTF8ToJavaString(
       env, cookies_);
 
   jobject j_context = base::android::GetApplicationContext();
   DCHECK(j_context);
 
+  bool success = false;
   if (Java_MediaPlayerBridge_setDataSource(

[scherkus (not reviewing), 2013/11/15 23:14:05]

I'd flip this around to use early returns

if (!Java_MediaPlayerBridge_setDataSource(...)) {
  OnMediaError(MEDIA_ERROR_FORMAT);
  return;
}

manager()->RequestMediaResources(...);

if (!Java_MediaPlayerBridge_prepareAsync(...)) {
  OnMediaError(MEDIA_ERROR_FORMAT);
  return;
}



it's a bit more verborse but clearer imo

[qinmin, 2013/11/16 00:00:19]

Done.

       env, j_media_player_bridge_.obj(), j_context, j_url_string.obj(),
       j_cookies.obj(), hide_url_log_)) {
     manager()->RequestMediaResources(player_id());

[scherkus (not reviewing), 2013/11/15 23:14:05]

do we need to call ReleaseMediaResources() if prepareAsync() fails?

[qinmin, 2013/11/16 00:00:19]

No, we shouldn't. 
The java MediaPlayer instance still lives if we simply call
ReleaseMediaResources(). So we need the manager to still take this player into
account when allocating resources.

Calling Release() will also work, but it feels wierd setDataSource() can delete
the player instance 

On 2013/11/15 23:14:05, scherkus wrote:

-    Java_MediaPlayerBridge_prepareAsync(
+    success = Java_MediaPlayerBridge_prepareAsync(
         env, j_media_player_bridge_.obj());
-  } else {
+  }
+
+  if (!success)
     OnMediaError(MEDIA_ERROR_FORMAT);
-  }
 }
 
 void MediaPlayerBridge::OnCookiesRetrieved(const std::string& cookies) {
   cookies_ = cookies;
   ExtractMediaMetadata(url_.spec());
 }
 
 void MediaPlayerBridge::ExtractMediaMetadata(const std::string& url) {
   manager()->GetMediaResourceGetter()->ExtractMediaMetadata(
       url, cookies_, base::Bind(&MediaPlayerBridge::OnMediaMetadataExtracted,
@@ skipping 254 matching lines @@
 
 GURL MediaPlayerBridge::GetUrl() {
   return url_;
 }
 
 GURL MediaPlayerBridge::GetFirstPartyForCookies() {
   return first_party_for_cookies_;
 }
 
 }  // namespace media