Implement a permission check for push.

chrome/browser/services/gcm/push_messaging_service_impl.cc

Index: chrome/browser/services/gcm/push_messaging_service_impl.cc
diff --git a/chrome/browser/services/gcm/push_messaging_service_impl.cc b/chrome/browser/services/gcm/push_messaging_service_impl.cc
index 0306d111fe66e95f69e299af9d68056c0989f568..c02ab3f19fd146508dd4d67b56d05ded3953f41a 100644
--- a/chrome/browser/services/gcm/push_messaging_service_impl.cc
+++ b/chrome/browser/services/gcm/push_messaging_service_impl.cc
@@ -10,13 +10,18 @@
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
+#include "chrome/browser/content_settings/permission_request_id.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/services/gcm/gcm_permission_context.h"
+#include "chrome/browser/services/gcm/gcm_permission_context_factory.h"
 #include "chrome/browser/services/gcm/gcm_profile_service.h"
 #include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
+#include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/gcm_driver/gcm_driver.h"
 #include "components/pref_registry/pref_registry_syncable.h"
+#include "content/public/browser/web_contents.h"
 
 namespace gcm {
 
@@ -127,6 +132,8 @@ void PushMessagingServiceImpl::OnSendError(
 void PushMessagingServiceImpl::Register(
     const std::string& app_id,
     const std::string& sender_id,
+    const int renderer_id,
+    const int routing_id,
     const content::PushMessagingService::RegisterCallback& callback) {
   if (!gcm_profile_service_->driver()) {
     NOTREACHED() << "There is no GCMDriver. Has GCMProfileService shut down?";
@@ -135,6 +142,7 @@ void PushMessagingServiceImpl::Register(
   if (profile_->GetPrefs()->GetInteger(
           prefs::kPushMessagingRegistrationCount) >= kMaxRegistrations) {
     DidRegister(app_id, callback, "", GCMClient::UNKNOWN_ERROR);
+    // The page doesn't exist any more.

[jianli, 2014/06/20 17:19:30]

Not sure I understand this comment.

[Miguel Garcia, 2014/06/20 20:50:13]

No wonder, it's an artifact of a bad merge. Removed

     return;
   }
 
@@ -143,12 +151,36 @@ void PushMessagingServiceImpl::Register(
   if (gcm_profile_service_->driver()->GetAppHandler(kAppIdPrefix) != this)
     gcm_profile_service_->driver()->AddAppHandler(kAppIdPrefix, this);
 
-  std::vector<std::string> sender_ids(1, sender_id);
-  gcm_profile_service_->driver()->Register(
-      app_id,
-      sender_ids,
-      base::Bind(&PushMessagingServiceImpl::DidRegister,
+  content::WebContents* web_contents =
+      tab_util::GetWebContentsByID(renderer_id, routing_id);
+
+  // The page doesn't exist any more.
+  if (!web_contents)
+    return;
+
+  // TODO(miguelg) need to send this over IPC when bubble support is
+  // implemented.
+  int bridge_id = -1;
+
+  const PermissionRequestID id(renderer_id, routing_id, bridge_id, GURL());
+
+  GURL embedder = web_contents->GetURL();
+  gcm::GCMPermissionContext* permission_context =
+      gcm::GCMPermissionContextFactory::GetForProfile(profile_);

[jianli, 2014/06/20 17:19:30]

It seems that GCMPermissionContext is only tied with PushMessagingServiceImpl.
Do we really need to make it KeyedService based?

[Miguel Garcia, 2014/06/20 20:50:13]

All other permissions seems to be tied to it so they can be fetched through a
keyed factory and so that they can behave in the right way when being called
from an incognito tab. I think it makes sense to keep it like this.

+
+  if (permission_context == NULL) {
+    DidRegister(app_id, callback, "", GCMClient::UNKNOWN_ERROR);

[fgorski, 2014/06/20 14:14:15]

nit: std::string()

you are using it in earlier files as opposed to ""

[Miguel Garcia, 2014/06/20 20:50:13]

Done.

+    return;
+  }
+
+  permission_context->RequestPermission(
+      web_contents,
+      id,
+      embedder,
+      false,
+      base::Bind(&PushMessagingServiceImpl::DidRequestPermission,
                  weak_factory_.GetWeakPtr(),
+                 sender_id,
                  app_id,
                  callback));
 }
@@ -170,6 +202,31 @@ void PushMessagingServiceImpl::DidRegister(
   }
 }
 
+void PushMessagingServiceImpl::DidRequestPermission(
+    const std::string& sender_id,
+    const std::string& app_id,
+    const content::PushMessagingService::RegisterCallback& register_callback,
+    bool allow) {
+  if (!allow) {
+    DidRegister(app_id, register_callback, "", GCMClient::UNKNOWN_ERROR);

[fgorski, 2014/06/20 14:14:15]

nit: std::string()

[fgorski, 2014/06/20 14:14:15]

in case you think there will be a specific error you want to give to developers
in response you can add a todo to extend the result enum accordingly.

[Miguel Garcia, 2014/06/20 20:50:12]

Done.

[Miguel Garcia, 2014/06/20 20:50:13]

Great, added the TODO and will be extending the enum in a subsuquent CL, in a
first iteration it will be useful for UMA collection, we are still figuring out
how to properly expose errors in the API

+    return;
+  }
+
+  // The GCMDriver could be NULL if GCMProfileService has been shut down.
+  if (!gcm_profile_service_->driver())
+    return;
+
+  std::vector<std::string> sender_ids(1, sender_id);
+
+  gcm_profile_service_->driver()->Register(
+      app_id,
+      sender_ids,
+      base::Bind(&PushMessagingServiceImpl::DidRegister,
+                 weak_factory_.GetWeakPtr(),
+                 app_id,
+                 register_callback));
+}
+
 // TODO(johnme): Unregister should decrement the pref, and call
 // RemoveAppHandler if the count drops to zero.