Permissions: glue between Mojo and Blink for permission observing.

content/child/permissions/permission_dispatcher.h

Index: content/child/permissions/permission_dispatcher.h
diff --git a/content/child/permissions/permission_dispatcher.h b/content/child/permissions/permission_dispatcher.h
index 6066c44b7228ddb2f81f06fd444f11555c59c582..f59f563c8e11f2db9abf17332623f91979ff8f44 100644
--- a/content/child/permissions/permission_dispatcher.h
+++ b/content/child/permissions/permission_dispatcher.h
@@ -7,10 +7,11 @@
 
 #include <string>
 
-#include "base/compiler_specific.h"
+#include "base/callback_forward.h"
 #include "base/id_map.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "content/child/permissions/permission_observers_registry.h"
 #include "content/common/permission_service.mojom.h"
 #include "third_party/WebKit/public/platform/modules/permissions/WebPermissionClient.h"
 
@@ -23,8 +24,11 @@ class ServiceRegistry;
 // from workers and frames independently. When called outside of the main
 // thread, QueryPermissionForWorker is meant to be called. It will handle the
 // thread jumping.
-class PermissionDispatcher : public blink::WebPermissionClient {
+class PermissionDispatcher : public blink::WebPermissionClient,
+                             public PermissionObserversRegistry {
  public:
+  static bool IsObservable(blink::WebPermissionType type);

[whywhat, 2015/04/02 16:20:23]

Comment?

[mlamouri (slow - plz ping), 2015/04/02 17:02:43]

done.

+
   // The caller must guarantee that |service_registry| will have a lifetime
   // larger than this instance of PermissionDispatcher.
   explicit PermissionDispatcher(ServiceRegistry* service_registry);
@@ -34,25 +38,58 @@ class PermissionDispatcher : public blink::WebPermissionClient {
   virtual void queryPermission(blink::WebPermissionType type,
                                const blink::WebURL& origin,
                                blink::WebPermissionQueryCallback* callback);
+  virtual void startListening(blink::WebPermissionType type,
+                              const blink::WebURL& origin,
+                              blink::WebPermissionObserver* observer);
+  virtual void stopListening(blink::WebPermissionObserver* observer);
 
+  // The following methods must be called by workers on the main thread.
   void QueryPermissionForWorker(blink::WebPermissionType type,
                                 const std::string& origin,
                                 blink::WebPermissionQueryCallback* callback,
                                 int worker_thread_id);
+  void StartListeningForWorker(

[whywhat, 2015/04/02 16:20:24]

why there's GetNextPermissionChangeForWorker but no StopListeningForWorker and
vice versa (no GetNextPermissionChange)?

If these are only helpers for PermissionDispatcherThreadProxy, maybe make them
private but friends to the proxy class?

[mlamouri (slow - plz ping), 2015/04/02 17:02:43]

These three methods, as said in the comments are meant to be called from worker
threads. We usually avoid |friend| in Chromium. I don't really care much but I
generally would prefer if PermissionDispatcher has no knowledge of
PermissionDispatcherThreadProxy.

+      blink::WebPermissionType type,
+      const std::string& origin,
+      int worker_thread_id,
+      const base::Callback<void(blink::WebPermissionStatus)>& callback);
+  void GetNextPermissionChangeForWorker(
+      blink::WebPermissionType type,
+      const std::string& origin,
+      blink::WebPermissionStatus status,
+      int worker_thread_id,
+      const base::Callback<void(blink::WebPermissionStatus)>& callback);
+
+ private:
+  // Called from the main thread in order to run the callback in the thread it
+  // was created on.
+  static void RunCallbackOnWorkerThread(

[whywhat, 2015/04/02 16:20:23]

nit: I think the comment needs to clarify that it's not called from the main
thread but posted by the main thread on the worker thread.
Reading the comment I assumed that this is a helper method that posts the
callback on the right thread and wanted to suggest a rename to
PostCallbackOnWorkerThread until I read the implementation.

[mlamouri (slow - plz ping), 2015/04/02 17:02:43]

Done.

+      blink::WebPermissionQueryCallback* callback,
+      scoped_ptr<blink::WebPermissionStatus> status);
+
+  // Helper method that returns an initialized PermissionServicePtr.
+  PermissionServicePtr& GetPermissionServicePtr();
 
- protected:
   void QueryPermissionInternal(blink::WebPermissionType type,
                                const std::string& origin,
                                blink::WebPermissionQueryCallback* callback,
                                int worker_thread_id);
 
   void OnQueryPermission(int request_id, PermissionStatus status);
-
-  // Called from the main thread in order to run the callback in the thread it
-  // was created on.
-  static void RunCallbackOnWorkerThread(
-      blink::WebPermissionQueryCallback* callback,
-      scoped_ptr<blink::WebPermissionStatus> status);
+  void OnPermissionChanged(blink::WebPermissionType type,
+                           const std::string& origin,
+                           blink::WebPermissionObserver* observer,
+                           PermissionStatus status);
+  void OnPermissionChangedForWorker(
+      int worker_thread_id,
+      const base::Callback<void(blink::WebPermissionStatus)>& callback,
+      PermissionStatus status);
+
+  // Helper to be used on the main thread.
+  void GetNextPermissionChange(blink::WebPermissionType type,
+                               const std::string& origin,
+                               blink::WebPermissionObserver* observer,
+                               PermissionStatus current_status);
 
   // Saves some basic information about the callback in order to be able to run
   // it in the right thread.