Push API: register PushMessagingServiceImpl as GCMAppHandler.

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 f6043b495bd9650c7b37f2ebd8947aa640847c15..830dc7282759f382dd3a72767581bc1cd37fb87f 100644
--- a/chrome/browser/services/gcm/push_messaging_service_impl.cc
+++ b/chrome/browser/services/gcm/push_messaging_service_impl.cc
@@ -7,40 +7,165 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/command_line.h"
+#include "base/prefs/pref_service.h"
+#include "base/strings/string_util.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/services/gcm/gcm_profile_service.h"
+#include "chrome/browser/services/gcm/gcm_profile_service_factory.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"
 
 namespace gcm {
 
+namespace {
+const std::string kAppIdPrefix = "push:";

[jianli, 2014/06/17 17:59:30]

nit: use char array.

[Michael van Ouwerkerk, 2014/06/17 19:22:06]

Done.

+}  // namespace
+
+// static
+void PushMessagingServiceImpl::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterIntegerPref(
+      prefs::kPushMessagingRegistrationCount,
+      0,
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+}
+
+// static
+void PushMessagingServiceImpl::InitializeForProfile(Profile* profile) {
+  // TODO(mvanouwerkerk): Make sure to remove this check at the same time as
+  // push graduates from experimental in Blink.
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableExperimentalWebPlatformFeatures)) {
+    return;
+  }
+  // TODO(johnme): Consider whether push should be enabled in incognito. If it
+  // does get enabled, then be careful that you're reading the pref from the
+  // right profile, as prefs defined in a regular profile are visible in the
+  // corresponding incognito profile unless overrriden.
+  if (!profile || profile->IsOffTheRecord() || profile->GetPrefs()->GetInteger(
+      prefs::kPushMessagingRegistrationCount) <= 0)
+    return;
+  // Create the GCMProfileService, and hence intiantiate this class.
+  GCMProfileService* gcm_service = gcm::GCMProfileServiceFactory::GetForProfile(
+      profile);
+  PushMessagingServiceImpl* push_service =
+      static_cast<PushMessagingServiceImpl*>(
+          gcm_service->push_messaging_service());
+  // Register ourselves as an app handler.
+  gcm_service->driver()->AddAppHandler(kAppIdPrefix, push_service);
+}
+
 PushMessagingServiceImpl::PushMessagingServiceImpl(
-    GCMProfileService* gcm_profile_service)
+    GCMProfileService* gcm_profile_service,
+    Profile* profile)
     : gcm_profile_service_(gcm_profile_service),
-      weak_factory_(this) {}
+      profile_(profile),
+      weak_factory_(this) {
+  // TODO(mvanouwerkerk): Automatically unregister Service Workers from GCM when

[jianli, 2014/06/17 17:59:30]

Not sure I understand the comment here. Do we want to unregister from the
constructor?

[Michael van Ouwerkerk, 2014/06/17 19:22:06]

Ah, sorry I just took that from John's sketch without cleaning it up. We'd want
to decrement the registration counter whenever a SW in uninstalled. We'd also
unregister its Push API GCM registration. And if the counter is 0 we would  call
RemoveAppHandler. This is all for a later patch.

+  // they are uninstalled.
+}
+
+PushMessagingServiceImpl::~PushMessagingServiceImpl() {
+  // TODO(johnme): If it's possible for this to be destroyed before GCMDriver,
+  // then we should call RemoveAppHandler.
+}
 
-PushMessagingServiceImpl::~PushMessagingServiceImpl() {}
+bool PushMessagingServiceImpl::CanHandle(const std::string& app_id) const {
+  // TODO(mvanouwerkerk): Finalize and centralize format of Push API app_id.
+  return StartsWithASCII(app_id, kAppIdPrefix, true);
+}
+
+void PushMessagingServiceImpl::ShutdownHandler() {
+  // TODO(johnme): Do any necessary cleanup.
+}
+
+void PushMessagingServiceImpl::OnMessage(
+    const std::string& app_id,
+    const GCMClient::IncomingMessage& message) {
+  // The Push API only exposes a single string of data in the push event fired
+  // on the Service Worker. When developers send messages using GCM to the Push
+  // API, they must pass a single key-value pair, where the key is "data" and
+  // the value is the string they want to be passed to their Service Worker.
+  // For example, they could send the following JSON using the HTTPS GCM API:
+  // {
+  //     "registration_ids": ["FOO", "BAR"],
+  //     "data": {
+  //         "data": "BAZ",
+  //     },
+  //     "delay_while_idle": true,
+  // }
+  // TODO(johnme): Make sure this is clearly documented for developers.
+  GCMClient::MessageData::const_iterator it = message.data.find("data");
+  if (it != message.data.end()) {
+    const std::string& data ALLOW_UNUSED = it->second;
+    // TODO(mvanouwerkerk): Fire push event with data on the Service Worker
+    // corresponding to app_id (and remove ALLOW_UNUSED above).
+  } else {
+    // Drop the message, as it is invalid.
+    // TODO(mvanouwerkerk): Show a warning in the developer console of the
+    // Service Worker corresponding to app_id.
+    // TODO(johnme): Add diagnostic observers (e.g. UMA and an internals page)
+    // to know when bad things happen.
+  }
+}
+
+void PushMessagingServiceImpl::OnMessagesDeleted(const std::string& app_id) {
+  // TODO(mvanouwerkerk): Fire push error event on the Service Worker
+  // corresponding to app_id.
+}
+
+void PushMessagingServiceImpl::OnSendError(
+    const std::string& app_id,
+    const GCMClient::SendErrorDetails& send_error_details) {
+  NOTREACHED() << "The Push API shouldn't have sent messages upstream";
+}
 
 void PushMessagingServiceImpl::Register(
     const std::string& app_id,
     const std::string& sender_id,
     const content::PushMessagingService::RegisterCallback& callback) {
   // The GCMDriver could be NULL if GCMProfileService has been shut down.
-  if (!gcm_profile_service_->driver())
+  if (!gcm_profile_service_->driver()) {
+    DidRegister(app_id, callback, "", GCMClient::UNKNOWN_ERROR);

[jianli, 2014/06/17 17:59:30]

I don't think we want to an unknown error here. If driver is not present, it is
a serious error. You should throw some assert here.

[Michael van Ouwerkerk, 2014/06/17 19:22:06]

I changed it to GCM_DISABLED wdyt?

     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(),
+                 GetWeakPtr(),

[jianli, 2014/06/17 17:59:30]

Why do we need to introduce a helper function for it? Calling
weak_factory_.GetWeakPtr() is simple enough.

[Michael van Ouwerkerk, 2014/06/17 19:22:06]

Agreed.

+                 app_id,
                  callback));
 }
 
 void PushMessagingServiceImpl::DidRegister(
+    const std::string& app_id,
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& registration_id,
     GCMClient::Result result) {
   GURL endpoint = GURL("https://android.googleapis.com/gcm/send");
-  callback.Run(endpoint, registration_id, result == GCMClient::SUCCESS);
+  bool success = (result == GCMClient::SUCCESS);
+  callback.Run(endpoint, registration_id, success);
+  if (success) {
+    // TODO(johnme): Make sure the pref doesn't get out of sync after crashes.
+    int old_count = profile_->GetPrefs()->GetInteger(
+        prefs::kPushMessagingRegistrationCount);
+    profile_->GetPrefs()->SetInteger(prefs::kPushMessagingRegistrationCount,
+                                     old_count + 1);
+    if (old_count == 0)
+      gcm_profile_service_->driver()->AddAppHandler(kAppIdPrefix, this);

[jianli, 2014/06/17 17:59:30]

I don't think we will hit this because calling GCMDriver::Register will fail
with UNKNOWN_ERROR when no app handler is added.

[Michael van Ouwerkerk, 2014/06/17 19:22:06]

Good catch. We still need to add this as an AppHandler on-demand, but before
calling Register. I've made GetAppHandler public so we can check whether or not
we've added ourselves already.

+  }
+}
+
+// TODO(johnme): Unregister should decrement the pref, and call
+// RemoveAppHandler if the count drops to zero.
+
+base::WeakPtr<PushMessagingServiceImpl> PushMessagingServiceImpl::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
 }
 
 }  // namespace gcm