Implement PermissionService::GetNextPermissionChange.

content/browser/permissions/permission_service_impl.cc

Index: content/browser/permissions/permission_service_impl.cc
diff --git a/content/browser/permissions/permission_service_impl.cc b/content/browser/permissions/permission_service_impl.cc
index 940e7832695a83fba7c8b5c1207174df9bce8c44..13cc0a967dc7c2f9b458f267e3ceffe1a2e2a1a3 100644
--- a/content/browser/permissions/permission_service_impl.cc
+++ b/content/browser/permissions/permission_service_impl.cc
@@ -114,8 +114,7 @@ void PermissionServiceImpl::HasPermission(
     const mojo::Callback<void(PermissionStatus)>& callback) {
   DCHECK(context_->GetBrowserContext());
 
-  callback.Run(GetPermissionStatus(PermissionNameToPermissionType(permission),
-                                   GURL(origin)));
+  callback.Run(GetPermissionStatus(permission, GURL(origin)));
 }
 
 void PermissionServiceImpl::RevokePermission(
@@ -138,8 +137,43 @@ void PermissionServiceImpl::RevokePermission(
   callback.Run(GetPermissionStatus(permission_type, origin_url));
 }
 
-PermissionStatus PermissionServiceImpl::GetPermissionStatus(PermissionType type,
-                                                            GURL origin) {
+void PermissionServiceImpl::GetNextPermissionChange(
+    PermissionName permission,
+    const mojo::String& origin,
+    PermissionStatus last_known_status,
+    const mojo::Callback<void(PermissionStatus)>& callback) {
+  PermissionStatus current_status =
+      GetPermissionStatus(permission, GURL(origin));
+  if (current_status != last_known_status) {
+    callback.Run(current_status);
+    return;
+  }
+
+  PendingSubscription* subscription = new PendingSubscription();
+  int id = pending_subscriptions_.Add(subscription);

[Bernhard Bauer, 2015/03/11 14:06:18]

I don't think you need this map. You could create a scoped_ptr<int> and bind it
into OnPermissionStatusChanged, then set the value to the ID returned by
SubscribePermissionStatusChange. In OnPermissionStatusChanged() you would then
dereference the pointer to get the actual ID.

[mlamouri (slow - plz ping), 2015/03/18 16:24:40]

Didn't use a scoped_ptr but a raw pointer that is then base::Owned by the
callback.

+
+  // If the embedding_origin is empty, we,ll use the |origin| instead.

[Bernhard Bauer, 2015/03/11 14:06:18]

Nit: "we'll". Also, move the code here that falls back to the origin if the
embedding origin is empty, so that it's clear what the comment refers to?

[mlamouri (slow - plz ping), 2015/03/18 16:24:40]

Done.

+  GURL embedding_origin = context_->GetEmbeddingOrigin();
+  subscription->subscription_id =
+      GetContentClient()->browser()->SubscribePermissionStatusChange(
+          PermissionNameToPermissionType(permission),
+          context_->GetBrowserContext(),
+          GURL(origin),
+          embedding_origin.is_empty() ? GURL(origin) : embedding_origin,
+          base::Bind(&PermissionServiceImpl::OnPermissionStatusChanged,
+                     weak_factory_.GetWeakPtr(),
+                     callback,
+                     id));
+}
+
+PermissionStatus PermissionServiceImpl::GetPermissionStatus(

[Bernhard Bauer, 2015/03/11 14:06:18]

Overloading methods is discouraged by the style guide.

[mlamouri (slow - plz ping), 2015/03/18 16:24:40]

Snif. I fixed that. GetPermissionStatusFromType() and
GetPermissionStatusFromName() doesn't sound like the best names but if that's
what the style guide wants.

+    PermissionName permission, const GURL& origin) {
+  return GetPermissionStatus(PermissionNameToPermissionType(permission),
+                             origin);
+}
+
+PermissionStatus PermissionServiceImpl::GetPermissionStatus(
+    PermissionType type, const GURL& origin) {
   // If the embedding_origin is empty we'll use |origin| instead.
   GURL embedding_origin = context_->GetEmbeddingOrigin();
   return GetContentClient()->browser()->GetPermissionStatus(
@@ -148,7 +182,7 @@ PermissionStatus PermissionServiceImpl::GetPermissionStatus(PermissionType type,
 }
 
 void PermissionServiceImpl::ResetPermissionStatus(PermissionType type,
-                                                  GURL origin) {
+                                                  const GURL& origin) {
   // If the embedding_origin is empty we'll use |origin| instead.
   GURL embedding_origin = context_->GetEmbeddingOrigin();
   GetContentClient()->browser()->ResetPermission(
@@ -156,4 +190,17 @@ void PermissionServiceImpl::ResetPermissionStatus(PermissionType type,
       embedding_origin.is_empty() ? origin : embedding_origin);
 }
 
+void PermissionServiceImpl::OnPermissionStatusChanged(
+    const mojo::Callback<void(PermissionStatus)>& callback,

[Bernhard Bauer, 2015/03/11 14:06:18]

Are you going to call the callback?

[mlamouri (slow - plz ping), 2015/03/18 16:24:40]

Oups :)

+    int pending_subscription_id,
+    PermissionStatus status) {
+  PendingSubscription* subscription =
+      pending_subscriptions_.Lookup(pending_subscription_id);
+  GetContentClient()->browser()->UnsubscribePermissionStatusChange(
+      context_->GetBrowserContext(),
+      subscription->subscription_id);
+
+  pending_subscriptions_.Remove(pending_subscription_id);
+}
+
 }  // namespace content