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

content/browser/media/cdm/browser_cdm_manager.cc

 // Copyright 2014 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/media/cdm/browser_cdm_manager.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 293 matching lines @@
   }
 
   AddCdm(render_frame_id, cdm_id, key_system, security_origin);
 }
 
 void BrowserCdmManager::OnSetServerCertificate(
     int render_frame_id,
     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));
 
   BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id);
   if (!cdm) {
     promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found.");
     return;
   }
 
   if (certificate.empty()) {
     promise->reject(MediaKeys::INVALID_ACCESS_ERROR, 0, "Empty certificate.");
     return;
   }
 
   cdm->SetServerCertificate(&certificate[0], certificate.size(),
                             promise.Pass());
 }
 
 void BrowserCdmManager::OnCreateSessionAndGenerateRequest(
     int render_frame_id,
     int cdm_id,
     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));
 
   if (init_data.size() > kMaxInitDataLength) {
     LOG(WARNING) << "InitData for ID: " << cdm_id
                  << " too long: " << init_data.size();
     promise->reject(MediaKeys::INVALID_ACCESS_ERROR, 0, "Init data too long.");
     return;
   }
 
@@ skipping 10 matching lines @@
     default:
       NOTREACHED();
       promise->reject(MediaKeys::INVALID_ACCESS_ERROR, 0,
                       "Invalid init data type.");
       return;
   }
 
 #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
 
   BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id);
   if (!cdm) {
     DLOG(WARNING) << "No CDM found for: " << render_frame_id << ", " << cdm_id;
     promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found.");
     return;
   }
 
   std::map<uint64, GURL>::const_iterator iter =
       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,
                                         int cdm_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));
 
   BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id);
   if (!cdm) {
     promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found.");
     return;
   }
 
   if (response.size() > kMaxSessionResponseLength) {
     LOG(WARNING) << "Response for ID " << cdm_id
                  << " is too long: " << response.size();
     promise->reject(MediaKeys::INVALID_ACCESS_ERROR, 0, "Response too long.");
     return;
   }
 
   if (response.empty()) {
     promise->reject(MediaKeys::INVALID_ACCESS_ERROR, 0, "Response is empty.");
     return;
   }
 
   cdm->UpdateSession(session_id, &response[0], response.size(), promise.Pass());
 }
 
 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));
 
   BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id);
   if (!cdm) {
     promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found.");
     return;
   }
 
   cdm->CloseSession(session_id, promise.Pass());
 }
 
 void BrowserCdmManager::OnDestroyCdm(int render_frame_id, int cdm_id) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   RemoveCdm(GetId(render_frame_id, cdm_id));
 }
 
 // Use a weak pointer here instead of |this| to avoid circular references.
 #define BROWSER_CDM_MANAGER_CB(func)                                   \
   base::Bind(&BrowserCdmManager::func, weak_ptr_factory_.GetWeakPtr(), \
              render_frame_id, cdm_id)
 
 void BrowserCdmManager::AddCdm(int render_frame_id,
                                int cdm_id,
@@ skipping 32 matching lines @@
 
   for (size_t i = 0; i < ids_to_remove.size(); ++i)
     RemoveCdm(ids_to_remove[i]);
 }
 
 void BrowserCdmManager::RemoveCdm(uint64 id) {
   DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
   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;
   }
 
   BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id);
   if (!cdm) {
     promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found.");
     return;
   }
 
@@ skipping 15 matching lines @@
   }
 
   // Only the temporary session type is supported in browser CDM path.
   // TODO(xhwang): Add SessionType support if needed.
   cdm->CreateSessionAndGenerateRequest(media::MediaKeys::TEMPORARY_SESSION,
                                        init_data_type_string, &init_data[0],
                                        init_data.size(), promise.Pass());
 }
 
 }  // namespace content