media: Simplify MediaPermissionDispatcher.

content/renderer/media/media_permission_dispatcher.cc

Index: content/renderer/media/media_permission_dispatcher.cc
diff --git a/content/renderer/media/media_permission_dispatcher.cc b/content/renderer/media/media_permission_dispatcher.cc
index 666d194e76b901963bddba78ea951754384d2a6f..4cde568554ce21a18b58a4dea382bfa99087b814 100644
--- a/content/renderer/media/media_permission_dispatcher.cc
+++ b/content/renderer/media/media_permission_dispatcher.cc
@@ -4,14 +4,48 @@
 
 #include "content/renderer/media/media_permission_dispatcher.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/single_thread_task_runner.h"
+#include "base/thread_task_runner_handle.h"
+#include "content/public/common/service_registry.h"
+#include "content/public/renderer/render_frame.h"
+#include "media/base/bind_to_current_loop.h"
+#include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
+#include "url/gurl.h"
+
+namespace {
+
+using Type = media::MediaPermission::Type;
+
+content::PermissionName MediaPermissionTypeToPermissionName(Type type) {
+  switch (type) {
+    case Type::PROTECTED_MEDIA_IDENTIFIER:
+      return content::PERMISSION_NAME_PROTECTED_MEDIA_IDENTIFIER;
+    case Type::AUDIO_CAPTURE:
+      return content::PERMISSION_NAME_AUDIO_CAPTURE;
+    case Type::VIDEO_CAPTURE:
+      return content::PERMISSION_NAME_VIDEO_CAPTURE;
+  }
+  NOTREACHED();
+  return content::PERMISSION_NAME_PROTECTED_MEDIA_IDENTIFIER;
+}
+
+}  // namespace
 
 namespace content {
 
-MediaPermissionDispatcher::MediaPermissionDispatcher() : next_request_id_(0) {}
+MediaPermissionDispatcher::MediaPermissionDispatcher(RenderFrame* render_frame)
+    : RenderFrameObserver(render_frame),
+      task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      next_request_id_(0),
+      weak_factory_(this) {
+  weak_ptr_ = weak_factory_.GetWeakPtr();

[dcheng, 2016/01/12 21:55:53]

This needs a comment: WeakPtrFactory and WeakPtr thread-safety semantics are
complicated.

"Initialize the WeakPtrFactory since GetWeakPtr() is not thread-safe"?

I don't know, it seems like WeakPtrFactory should have some of this comment too,
but I'm not sure what the best split is.

[Wez, 2016/01/15 18:59:30]

Yeah, there's a tension between allowing GetWeakPtr() to be called from whatever
thread you like, regardless of where the factory or pointer will be used, versus
checking for _concurrent_ calls to GetWeakPtr() from different threads, which is
what's not safe.  I'll send you a CL to add a comment explaining the concurrency
issue at least, dcheng@. :)

I don't think you need a comment here, though; taking a WeakPtr from a
WeakPtrFactory you just initialized is fine - at this point there can't be any
code running on other threads touching this object.

[xhwang, 2016/01/19 20:44:32]

Acknowledged.

+}
 
 MediaPermissionDispatcher::~MediaPermissionDispatcher() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
   // Fire all pending callbacks with |false|.
   for (auto& request : requests_)
     request.second.Run(false);
@@ -19,19 +53,80 @@ MediaPermissionDispatcher::~MediaPermissionDispatcher() {
   requests_.clear();
 }
 
+void MediaPermissionDispatcher::HasPermission(
+    Type type,
+    const GURL& security_origin,
+    const PermissionStatusCB& permission_status_cb) {
+  if (!task_runner_->RunsTasksOnCurrentThread()) {
+    // Use BindToCurrentLoop to bind the callback to the current thread.
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&MediaPermissionDispatcher::HasPermission,
+                              weak_ptr_, type, security_origin,
+                              media::BindToCurrentLoop(permission_status_cb)));
+    return;
+  }
+
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
+  if (!permission_service_.get()) {

[dcheng, 2016/01/12 21:55:53]

No .get() here and elsewhere.

[xhwang, 2016/01/19 20:44:32]

Done.

+    render_frame()->GetServiceRegistry()->ConnectToRemoteService(
+        mojo::GetProxy(&permission_service_));
+  }
+
+  int request_id = RegisterCallback(permission_status_cb);
+  DVLOG(2) << __FUNCTION__ << ": request ID " << request_id;
+
+  permission_service_->HasPermission(
+      MediaPermissionTypeToPermissionName(type), security_origin.spec(),
+      base::Bind(&MediaPermissionDispatcher::OnPermissionStatus, weak_ptr_,
+                 request_id));
+}
+
+void MediaPermissionDispatcher::RequestPermission(
+    Type type,
+    const GURL& security_origin,
+    const PermissionStatusCB& permission_status_cb) {
+  if (!task_runner_->RunsTasksOnCurrentThread()) {
+    // Use BindToCurrentLoop to bind the callback to the current thread.
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&MediaPermissionDispatcher::RequestPermission,
+                              weak_ptr_, type, security_origin,
+                              media::BindToCurrentLoop(permission_status_cb)));
+    return;
+  }
+
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
+  if (!permission_service_.get()) {
+    render_frame()->GetServiceRegistry()->ConnectToRemoteService(
+        mojo::GetProxy(&permission_service_));
+  }
+
+  int request_id = RegisterCallback(permission_status_cb);
+  DVLOG(2) << __FUNCTION__ << ": request ID " << request_id;
+
+  permission_service_->RequestPermission(
+      MediaPermissionTypeToPermissionName(type), security_origin.spec(),
+      blink::WebUserGestureIndicator::isProcessingUserGesture(),
+      base::Bind(&MediaPermissionDispatcher::OnPermissionStatus, weak_ptr_,
+                 request_id));
+}
+
 uint32_t MediaPermissionDispatcher::RegisterCallback(
     const PermissionStatusCB& permission_status_cb) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
   uint32_t request_id = next_request_id_++;
   DCHECK(!requests_.count(request_id));
   requests_[request_id] = permission_status_cb;
+
   return request_id;
 }
 
-void MediaPermissionDispatcher::DeliverResult(uint32_t request_id,
-                                              bool granted) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DVLOG(2) << __FUNCTION__ << ": (" << request_id << ", " << granted << ")";
+void MediaPermissionDispatcher::OnPermissionStatus(uint32_t request_id,
+                                                   PermissionStatus status) {
+  DVLOG(2) << __FUNCTION__ << ": (" << request_id << ", " << status << ")";
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
   RequestMap::iterator iter = requests_.find(request_id);
   if (iter == requests_.end()) {
@@ -42,7 +137,7 @@ void MediaPermissionDispatcher::DeliverResult(uint32_t request_id,
   PermissionStatusCB permission_status_cb = iter->second;
   requests_.erase(iter);
 
-  permission_status_cb.Run(granted);
+  permission_status_cb.Run(status == PERMISSION_STATUS_GRANTED);
 }
 
 }  // namespace content