media: Simplify MediaPermissionDispatcher.

content/renderer/media/media_permission_dispatcher.h

Index: content/renderer/media/media_permission_dispatcher.h
diff --git a/content/renderer/media/media_permission_dispatcher.h b/content/renderer/media/media_permission_dispatcher.h
index 701afe6d088987a5c94754a1a0cd396ab1a13f11..6a303c576c1a0f3b9d8d2fa47fb95d19b578db61 100644
--- a/content/renderer/media/media_permission_dispatcher.h
+++ b/content/renderer/media/media_permission_dispatcher.h
@@ -2,46 +2,59 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Base class for MediaPermission implementations in content. This class allows
-// multiple pending PermissionService calls by managing callbacks for its
-// subclasses. This class is not thread safe and should only be used on one
-// thread.
-
 #ifndef CONTENT_RENDERER_MEDIA_MEDIA_PERMISSION_DISPATCHER_H_
 #define CONTENT_RENDERER_MEDIA_MEDIA_PERMISSION_DISPATCHER_H_
 
 #include <map>
 
+#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/threading/thread_checker.h"
 #include "content/common/content_export.h"
+#include "content/common/permission_service.mojom.h"
+#include "content/public/renderer/render_frame_observer.h"
 #include "media/base/media_permission.h"
 
+namespace base {
+class SingleThreadTaskRunner;
+}
+
 namespace content {
 
-class CONTENT_EXPORT MediaPermissionDispatcher : public media::MediaPermission {
+// MediaPermission implementation.
+class CONTENT_EXPORT MediaPermissionDispatcher : public media::MediaPermission,
+                                                 public RenderFrameObserver {
  public:
-  MediaPermissionDispatcher();
+  explicit MediaPermissionDispatcher(RenderFrame* render_frame);
   ~MediaPermissionDispatcher() override;
 
- protected:
-  // Register callbacks for PermissionService calls.
-  uint32_t RegisterCallback(const PermissionStatusCB& permission_status_cb);
-
-  // Deliver the permission status |granted| to the callback identified by
-  // |request_id|.
-  void DeliverResult(uint32_t request_id, bool granted);
-
-  base::ThreadChecker& thread_checker() { return thread_checker_; }
+  // media::MediaPermission implementation.
+  // Note: Can be called on any thread. The |permission_status_cb| will always
+  // be fired on the thread where these methods are called.

[Wez, 2015/11/25 00:44:48]

How is it safe to call "on any thread"? The dispatcher is an owned object, not
ref-counted, so how can callers on threads A & B, for example, make sure not to
try to call it while it's being deleted on thread C?

Is there some fundamental assumption re the calling code that you need to
document here?

[xhwang, 2015/11/25 01:18:02]

|this| shouldn't worry about how it's owned, right? I think it's the caller's
responsibility to make sure that it never call into a deleted object. In our
case, media players are always destroyed before RenderFrameImpl is destructed so
this is okay.

Actually the reason I am making this change is that I need to use
MediaPermissionDispatcher in other places as well, e.g. in the GPU process. In
that case we'll have a different ownership model. It should always be the
responsibility of the caller (in collaboration with the owner) to make sure
these calls are safe. Does that make sense?

+  void HasPermission(Type type,
+                     const GURL& security_origin,
+                     const PermissionStatusCB& permission_status_cb) override;
+  void RequestPermission(
+      Type type,
+      const GURL& security_origin,
+      const PermissionStatusCB& permission_status_cb) override;
 
  private:
   // Map of request IDs and pending PermissionStatusCBs.
   typedef std::map<uint32_t, PermissionStatusCB> RequestMap;
 
-  base::ThreadChecker thread_checker_;
+  // Register PermissionStatusCBs. Returns |request_id| that can be used to make
+  // PermissionService calls.
+  uint32_t RegisterCallback(const PermissionStatusCB& permission_status_cb);
+
+  // Callback for |permission_service_| calls.
+  void OnPermissionStatus(uint32_t request_id, PermissionStatus status);
 
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   uint32_t next_request_id_;
   RequestMap requests_;
+  PermissionServicePtr permission_service_;
+
+  base::WeakPtrFactory<MediaPermissionDispatcher> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaPermissionDispatcher);
 };