media: Fix permission request/check for EME on Android.

content/browser/media/cdm/browser_cdm_manager.cc

Index: content/browser/media/cdm/browser_cdm_manager.cc
diff --git a/content/browser/media/cdm/browser_cdm_manager.cc b/content/browser/media/cdm/browser_cdm_manager.cc
index 1b78ba3c095c64e01b55f4ccb297918ea186bc22..0d8e9393ade86896e98122d4deff4df629c89ee8 100644
--- a/content/browser/media/cdm/browser_cdm_manager.cc
+++ b/content/browser/media/cdm/browser_cdm_manager.cc
@@ -311,6 +311,8 @@ void BrowserCdmManager::OnSetServerCertificate(
     int cdm_id,
     uint32_t promise_id,
     const std::vector<uint8_t>& certificate) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
   scoped_ptr<SimplePromise> promise(
       new SimplePromise(this, render_frame_id, cdm_id, promise_id));
 
@@ -335,6 +337,8 @@ void BrowserCdmManager::OnCreateSessionAndGenerateRequest(
     uint32_t promise_id,
     CdmHostMsg_CreateSession_InitDataType init_data_type,
     const std::vector<uint8>& init_data) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
   scoped_ptr<NewSessionPromise> promise(
       new NewSessionPromise(this, render_frame_id, cdm_id, promise_id));
 
@@ -365,9 +369,9 @@ void BrowserCdmManager::OnCreateSessionAndGenerateRequest(
 #if defined(OS_ANDROID)
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableInfobarForProtectedMediaIdentifier)) {
-    GenerateRequestIfPermitted(
-        render_frame_id, cdm_id, eme_init_data_type,
-        init_data, promise.Pass(), PERMISSION_STATUS_GRANTED);
+    CreateSessionAndGenerateRequest(render_frame_id, cdm_id, eme_init_data_type,
+                                    init_data, promise.Pass(),
+                                    PERMISSION_STATUS_GRANTED);
     return;
   }
 #endif
@@ -383,13 +387,13 @@ void BrowserCdmManager::OnCreateSessionAndGenerateRequest(
       cdm_security_origin_map_.find(GetId(render_frame_id, cdm_id));
   if (iter == cdm_security_origin_map_.end()) {
     NOTREACHED();
-    promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found.");
+    promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "Origin not found.");
     return;
   }
   GURL security_origin = iter->second;
 
-  RequestSessionPermission(render_frame_id, security_origin, cdm_id,
-                           eme_init_data_type, init_data, promise.Pass());
+  CheckPermissionStatus(render_frame_id, security_origin, cdm_id,
+                        eme_init_data_type, init_data, promise.Pass());
 }
 
 void BrowserCdmManager::OnUpdateSession(int render_frame_id,
@@ -397,6 +401,8 @@ void BrowserCdmManager::OnUpdateSession(int render_frame_id,
                                         uint32_t promise_id,
                                         const std::string& session_id,
                                         const std::vector<uint8>& response) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
   scoped_ptr<SimplePromise> promise(
       new SimplePromise(this, render_frame_id, cdm_id, promise_id));
 
@@ -425,6 +431,8 @@ void BrowserCdmManager::OnCloseSession(int render_frame_id,
                                        int cdm_id,
                                        uint32_t promise_id,
                                        const std::string& session_id) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
   scoped_ptr<SimplePromise> promise(
       new SimplePromise(this, render_frame_id, cdm_id, promise_id));
 
@@ -438,6 +446,7 @@ void BrowserCdmManager::OnCloseSession(int render_frame_id,
 }
 
 void BrowserCdmManager::OnDestroyCdm(int render_frame_id, int cdm_id) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   RemoveCdm(GetId(render_frame_id, cdm_id));
 }
 
@@ -490,52 +499,64 @@ void BrowserCdmManager::RemoveCdm(uint64 id) {
 
   cdm_map_.erase(id);
   cdm_security_origin_map_.erase(id);
-  if (cdm_cancel_permission_map_.count(id)) {
-    cdm_cancel_permission_map_[id].Run();
-    cdm_cancel_permission_map_.erase(id);
-  }
 }
 
-void BrowserCdmManager::RequestSessionPermission(
+void BrowserCdmManager::CheckPermissionStatus(
     int render_frame_id,
     const GURL& security_origin,
     int cdm_id,
     media::EmeInitDataType init_data_type,
     const std::vector<uint8>& init_data,
     scoped_ptr<media::NewSessionCdmPromise> promise) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());

[ddorwin, 2015/03/16 23:58:28]

This seems inconsistent with the need for line 514.

[xhwang, 2015/03/17 01:25:22]

|task_runner_| is not always on the UI thread. Added a comment to make this more
clear.

+
+  // RenderFrameHost::FromID() can only be called from the UI thread.
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        base::Bind(&BrowserCdmManager::RequestSessionPermission, this,
+        base::Bind(&BrowserCdmManager::CheckPermissionStatus, this,
                    render_frame_id, security_origin, cdm_id, init_data_type,
                    init_data, base::Passed(&promise)));
     return;
   }
 
+  // Note: This part of code may not run on the |task_runner_|. Be careful
+  // about thread safty!
+
   RenderFrameHost* rfh =
       RenderFrameHost::FromID(render_process_id_, render_frame_id);
   WebContents* web_contents = WebContents::FromRenderFrameHost(rfh);
-  DCHECK(web_contents);
-  GetContentClient()->browser()->RequestPermission(
-      content::PERMISSION_PROTECTED_MEDIA_IDENTIFIER, web_contents,
-      0,  // bridge id
-      security_origin,
-      // Only implemented for Android infobars which do not support
-      // user gestures.
-      true, base::Bind(&BrowserCdmManager::GenerateRequestIfPermitted, this,
-                       render_frame_id, cdm_id, init_data_type, init_data,
-                       base::Passed(&promise)));
+
+  GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();
+
+  PermissionStatus permission_status =
+      GetContentClient()->browser()->GetPermissionStatus(
+          content::PERMISSION_PROTECTED_MEDIA_IDENTIFIER,
+          web_contents->GetBrowserContext(), security_origin, embedding_origin);
+
+  CreateSessionAndGenerateRequest(render_frame_id, cdm_id, init_data_type,

[ddorwin, 2015/03/16 23:58:28]

It seems odd to do this inside CheckPermissionStatus(). How about returning a
bool and calling this function HasPermission() or something like that. You can
then call CreateSessionAndGenerateRequest() from the caller.

Nevermind - The PostTask above prevents returning a value. :(
Still, I think passing an on_permission_status_cb callback would be better.
Either that or we should rename this function. WDYT?

[xhwang, 2015/03/17 01:25:22]

Good point. Updated.

+                                  init_data, promise.Pass(), permission_status);
 }
 
-void BrowserCdmManager::GenerateRequestIfPermitted(
+void BrowserCdmManager::CreateSessionAndGenerateRequest(
     int render_frame_id,
     int cdm_id,
     media::EmeInitDataType init_data_type,
     const std::vector<uint8>& init_data,
     scoped_ptr<media::NewSessionCdmPromise> promise,
-    PermissionStatus permission) {
-  cdm_cancel_permission_map_.erase(GetId(render_frame_id, cdm_id));
-  if (permission != PERMISSION_STATUS_GRANTED) {
+    PermissionStatus permission_status) {
+  // Make sure cdm->CreateSessionAndGenerateRequest() is called on the
+  // |task_runner_|.
+  if (!task_runner_->RunsTasksOnCurrentThread()) {
+    task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&BrowserCdmManager::CreateSessionAndGenerateRequest, this,
+                   render_frame_id, cdm_id, init_data_type, init_data,
+                   base::Passed(&promise), permission_status));
+    return;
+  }
+
+  if (permission_status != PERMISSION_STATUS_GRANTED) {

[ddorwin, 2015/03/16 23:58:28]

Why do we do this here? Probably because GenerateRequestIfPermitted used to be
called as a callback. Instead, do this in the function that handles the result
of CheckPermissionStatus() (see previous comment).

Does that cause problems for the thread? If so, let's post back to the correct
thread in that function and DCHECK that we are in the right thread in this
function.

[xhwang, 2015/03/17 01:25:22]

Done.

     promise->reject(MediaKeys::NOT_SUPPORTED_ERROR, 0, "Permission denied.");
     return;
   }