Use Mojo Permission Service when requesting Notifications permission.

content/renderer/notification_permission_dispatcher.cc

Index: content/renderer/notification_permission_dispatcher.cc
diff --git a/content/renderer/notification_permission_dispatcher.cc b/content/renderer/notification_permission_dispatcher.cc
index 92d5529122b2dcbef19ce47de6de3a2ae09a24d0..1a725cee7719c3b67b67c34f94a15ef9ab0bd509 100644
--- a/content/renderer/notification_permission_dispatcher.cc
+++ b/content/renderer/notification_permission_dispatcher.cc
@@ -4,11 +4,12 @@
 
 #include "content/renderer/notification_permission_dispatcher.h"
 
-#include "content/common/platform_notification_messages.h"
+#include "base/bind.h"
+#include "content/public/common/service_registry.h"
+#include "content/public/renderer/render_frame.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/web/WebNotificationPermissionCallback.h"
 #include "third_party/WebKit/public/web/WebSecurityOrigin.h"
-#include "url/gurl.h"
 
 namespace content {
 
@@ -22,30 +23,41 @@ NotificationPermissionDispatcher::~NotificationPermissionDispatcher() {
 void NotificationPermissionDispatcher::RequestPermission(
     const blink::WebSecurityOrigin& origin,
     blink::WebNotificationPermissionCallback* callback) {
-  int request_id = pending_requests_.Add(callback);
-  Send(new PlatformNotificationHostMsg_RequestPermission(
-      routing_id(), GURL(origin.toString()), request_id));
-}
+  if (!permission_service_.get()) {
+    render_frame()->GetServiceRegistry()->ConnectToRemoteService(
+        &permission_service_);
+  }
 
-bool NotificationPermissionDispatcher::OnMessageReceived(
-    const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(NotificationPermissionDispatcher, message)
-    IPC_MESSAGE_HANDLER(PlatformNotificationMsg_PermissionRequestComplete,
-                        OnPermissionRequestComplete);
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
+  int request_id = pending_requests_.Add(callback);
 
-  return handled;
+  permission_service_->RequestPermission(

[Peter Beverloo, 2014/11/24 18:37:34]

Is it possible that the service is not connected here yet? What happens in that
case, does the call wait until it is connected?

[mlamouri (slow - plz ping), 2014/11/25 12:19:55]

permission_service_ is a pipe. It is created when ConnectToRemoveService is
created. If there is no service on the other side, the call will simply go
nowhere.

[Peter Beverloo, 2014/11/25 13:10:34]

Can we add a DCHECK to verify that the service is connected, then?

[mlamouri (slow - plz ping), 2014/11/25 13:30:22]

The DCHECK will only check that the pipe is created which will always be the
case. Please look at qsr@ explanation here for more detailed information:
https://codereview.chromium.org/755673002/

+      PERMISSION_NAME_NOTIFICATIONS,
+      origin.toString().utf8(),
+      base::Bind(&NotificationPermissionDispatcher::OnPermissionRequestComplete,
+                 base::Unretained(this),

[Peter Beverloo, 2014/11/24 18:37:33]

Just to confirm: is this safe because |permission_service_| is a member of
|this|, which will be disconnected once this class gets destroyed?

[mlamouri (slow - plz ping), 2014/11/25 12:19:55]

Correct.

+                 request_id));
 }
 
 void NotificationPermissionDispatcher::OnPermissionRequestComplete(
-    int request_id, blink::WebNotificationPermission result) {
+    int request_id, PermissionStatus status) {
   blink::WebNotificationPermissionCallback* callback =
       pending_requests_.Lookup(request_id);
   DCHECK(callback);
 
-  callback->permissionRequestComplete(result);
+  blink::WebNotificationPermission permission;
+  switch (status) {
+    case PERMISSION_STATUS_GRANTED:
+      permission = blink::WebNotificationPermissionAllowed;
+      break;
+    case PERMISSION_STATUS_DENIED:
+      permission = blink::WebNotificationPermissionDenied;
+      break;
+    case PERMISSION_STATUS_ASK:
+      permission = blink::WebNotificationPermissionDefault;

[Peter Beverloo, 2014/11/24 18:37:34]

\o/

Please add 434547 to the BUG= line of your description, and add a little comment
to point out that this is in fact changing Web Exposed behavior.

[mlamouri (slow - plz ping), 2014/11/25 12:19:55]

Done.

+      break;
+  }

[Tom Sepez, 2014/11/24 18:58:06]

Nit:
default: NOTREACHED(); break; or similar?

[mlamouri (slow - plz ping), 2014/11/25 12:19:55]

PermissionStatus has three states. There will be a warning (that will break
compilation) if another one is added. I don't think we need a default: case
then.

+
+  callback->permissionRequestComplete(permission);
   pending_requests_.Remove(request_id);
 }