Define PushMessagingApplicationId and replace ad-hoc code.

BUG=350377

Review URL: https://codereview.chromium.org/404353003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285571 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/services/gcm/push_messaging_application_id.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/services/gcm/push_messaging_application_id.h"
+
+#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+
+namespace {
+const char kSeparator = '#';  // Ok as only the origin of the url is used.
+}
+
+namespace gcm {
+
+const char kPushMessagingApplicationIdPrefix[] = "push";
+
+// static
+PushMessagingApplicationId PushMessagingApplicationId::Parse(
+    const std::string& id) {
+  std::vector<std::string> parts;
+  base::SplitString(id, kSeparator, &parts);
+
+  if (parts.size() != 3 || parts[0] != kPushMessagingApplicationIdPrefix)
+    return PushMessagingApplicationId();
+
+  GURL origin = GURL(parts[1]);
+  if (!origin.is_valid() || origin.GetOrigin() != origin)
+    return PushMessagingApplicationId();
+
+  int64 service_worker_registration_id;
+  if (!base::StringToInt64(parts[2], &service_worker_registration_id))
+    return PushMessagingApplicationId();
+
+  return PushMessagingApplicationId(origin, service_worker_registration_id);
+}
+
+bool PushMessagingApplicationId::IsValid() {
+  return origin.is_valid() && origin.GetOrigin() == origin &&
+         service_worker_registration_id >= 0;
+}
+
+std::string PushMessagingApplicationId::ToString() const {
+  return (std::string(kPushMessagingApplicationIdPrefix) + kSeparator +
+          origin.spec() + kSeparator +
+          base::Int64ToString(service_worker_registration_id));
+}
+
+}  // namespace gcm

chrome/browser/services/gcm/push_messaging_application_id.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SERVICES_GCM_PUSH_MESSAGING_APPLICATION_ID_H_
+#define CHROME_BROWSER_SERVICES_GCM_PUSH_MESSAGING_APPLICATION_ID_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "url/gurl.h"
+
+namespace gcm {
+
+// The prefix used for all push messaging application ids.
+extern const char kPushMessagingApplicationIdPrefix[];
+
+// Type used to identify a web app from a Push API perspective.
+struct PushMessagingApplicationId {
+ public:
+  static PushMessagingApplicationId Parse(const std::string& id);
+
+  PushMessagingApplicationId()
+      : origin(GURL::EmptyGURL()), service_worker_registration_id(-1) {}
+  PushMessagingApplicationId(const GURL& origin,
+                             int64 service_worker_registration_id)
+      : origin(origin),
+        service_worker_registration_id(service_worker_registration_id) {}
+
+  bool IsValid();
+  std::string ToString() const;
+
+  const GURL origin;
+  const int64 service_worker_registration_id;
+};
+
+}  // namespace gcm
+
+#endif  // CHROME_BROWSER_SERVICES_GCM_PUSH_MESSAGING_APPLICATION_ID_H_

chrome/browser/services/gcm/push_messaging_application_id_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/services/gcm/push_messaging_application_id.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(PushMessagingApplicationIdTest, ConstructorValidity) {
+  EXPECT_TRUE(gcm::PushMessagingApplicationId(GURL("https://www.example.com/"),
+                                              1).IsValid());
+  EXPECT_TRUE(gcm::PushMessagingApplicationId(GURL("https://www.example.com"),
+                                              1).IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId(GURL(""), 1).IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId(GURL("foo"), 1).IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId(
+                   GURL("https://www.example.com/foo"), 1).IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId(
+                   GURL("https://www.example.com/#foo"), 1).IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId(GURL("https://www.example.com/"),
+                                               -1).IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId().IsValid());
+}
+
+TEST(PushMessagingApplicationIdTest, ToString) {
+  EXPECT_EQ(gcm::PushMessagingApplicationId(GURL("https://www.example.com/"), 1)
+                .ToString(),
+            "push#https://www.example.com/#1");
+  EXPECT_EQ(gcm::PushMessagingApplicationId(GURL("https://www.example.com"), 1)
+                .ToString(),
+            "push#https://www.example.com/#1");
+}
+
+TEST(PushMessagingApplicationIdTest, ParseValidity) {
+  EXPECT_TRUE(gcm::PushMessagingApplicationId::Parse(
+                  "push#https://www.example.com/#1").IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId::Parse("").IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId::Parse(
+                   "sync#https://www.example.com/#1").IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId::Parse("push#foo#1").IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId::Parse(
+                   "push#https://www.example.com/foo#1").IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId::Parse(
+                   "push#https://www.example.com/#one").IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId::Parse(
+                   "push#https://www.example.com/#foo#1").IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId::Parse(
+                   "push#https://www.example.com/#1#1").IsValid());
+  EXPECT_FALSE(gcm::PushMessagingApplicationId::Parse(
+                   "push#https://www.example.com/#-1").IsValid());
+}

chrome/browser/services/gcm/push_messaging_service_impl.cc

@@ -14,6 +14,7 @@
 #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/browser/services/gcm/push_messaging_application_id.h"
 #include "chrome/browser/services/gcm/push_messaging_permission_context.h"
 #include "chrome/browser/services/gcm/push_messaging_permission_context_factory.h"
 #include "chrome/common/chrome_switches.h"
@@ -26,7 +27,6 @@
 namespace gcm {
 
 namespace {
-const char kAppIdPrefix[] = "push:";
 const int kMaxRegistrations = 1000000;
 }  // namespace
 
@@ -63,7 +63,8 @@ void PushMessagingServiceImpl::InitializeForProfile(Profile* profile) {
       static_cast<PushMessagingServiceImpl*>(
           gcm_service->push_messaging_service());
   // Register ourselves as an app handler.
-  gcm_service->driver()->AddAppHandler(kAppIdPrefix, push_service);
+  gcm_service->driver()->AddAppHandler(kPushMessagingApplicationIdPrefix,
+                                       push_service);
 }
 
 PushMessagingServiceImpl::PushMessagingServiceImpl(
@@ -80,8 +81,7 @@ 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);
+  return PushMessagingApplicationId::Parse(app_id).IsValid();
 }
 
 void PushMessagingServiceImpl::ShutdownHandler() {
@@ -104,8 +104,11 @@ void PushMessagingServiceImpl::OnMessage(
   //     "delay_while_idle": true,
   // }
   // TODO(johnme): Make sure this is clearly documented for developers.
+  PushMessagingApplicationId application_id =
+      PushMessagingApplicationId::Parse(app_id);
+  DCHECK(application_id.IsValid());
   GCMClient::MessageData::const_iterator it = message.data.find("data");
-  if (it != message.data.end()) {
+  if (application_id.IsValid() && 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).
@@ -130,7 +133,8 @@ void PushMessagingServiceImpl::OnSendError(
 }
 
 void PushMessagingServiceImpl::Register(
-    const std::string& app_id,
+    const GURL& origin,
+    int64 service_worker_registration_id,
     const std::string& sender_id,
     int renderer_id,
     int render_frame_id,
@@ -140,10 +144,13 @@ void PushMessagingServiceImpl::Register(
     NOTREACHED() << "There is no GCMDriver. Has GCMProfileService shut down?";
   }
 
+  PushMessagingApplicationId application_id =
+      PushMessagingApplicationId(origin, service_worker_registration_id);
+  DCHECK(application_id.IsValid());
+
   if (profile_->GetPrefs()->GetInteger(
           prefs::kPushMessagingRegistrationCount) >= kMaxRegistrations) {
     RegisterEnd(
-        app_id,
         callback,
         std::string(),
         content::PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_LIMIT_REACHED);
@@ -152,8 +159,10 @@ void PushMessagingServiceImpl::Register(
 
   // If this is registering for the first time then the driver does not have
   // this as an app handler and registration would fail.
-  if (gcm_profile_service_->driver()->GetAppHandler(kAppIdPrefix) != this)
-    gcm_profile_service_->driver()->AddAppHandler(kAppIdPrefix, this);
+  if (gcm_profile_service_->driver()->GetAppHandler(
+          kPushMessagingApplicationIdPrefix) != this)
+    gcm_profile_service_->driver()->AddAppHandler(
+        kPushMessagingApplicationIdPrefix, this);
 
   content::RenderFrameHost* render_frame_host =
       content::RenderFrameHost::FromID(renderer_id, render_frame_id);
@@ -182,7 +191,6 @@ void PushMessagingServiceImpl::Register(
 
   if (permission_context == NULL) {
     RegisterEnd(
-        app_id,
         callback,
         std::string(),
         content::PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_PERMISSION_DENIED);
@@ -196,13 +204,12 @@ void PushMessagingServiceImpl::Register(
       user_gesture,
       base::Bind(&PushMessagingServiceImpl::DidRequestPermission,
                  weak_factory_.GetWeakPtr(),
+                 application_id,
                  sender_id,
-                 app_id,
                  callback));
 }
 
 void PushMessagingServiceImpl::RegisterEnd(
-    const std::string& app_id,
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& registration_id,
     content::PushMessagingStatus status) {
@@ -218,7 +225,6 @@ void PushMessagingServiceImpl::RegisterEnd(
 }
 
 void PushMessagingServiceImpl::DidRegister(
-    const std::string& app_id,
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& registration_id,
     GCMClient::Result result) {
@@ -226,17 +232,16 @@ void PushMessagingServiceImpl::DidRegister(
       result == GCMClient::SUCCESS
           ? content::PUSH_MESSAGING_STATUS_OK
           : content::PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_SERVICE_ERROR;
-  RegisterEnd(app_id, callback, registration_id, status);
+  RegisterEnd(callback, registration_id, status);
 }
 
 void PushMessagingServiceImpl::DidRequestPermission(
+    const PushMessagingApplicationId& application_id,
     const std::string& sender_id,
-    const std::string& app_id,
     const content::PushMessagingService::RegisterCallback& register_callback,
     bool allow) {
   if (!allow) {
     RegisterEnd(
-        app_id,
         register_callback,
         std::string(),
         content::PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_PERMISSION_DENIED);
@@ -250,11 +255,10 @@ void PushMessagingServiceImpl::DidRequestPermission(
   std::vector<std::string> sender_ids(1, sender_id);
 
   gcm_profile_service_->driver()->Register(
-      app_id,
+      application_id.ToString(),
       sender_ids,
       base::Bind(&PushMessagingServiceImpl::DidRegister,
                  weak_factory_.GetWeakPtr(),
-                 app_id,
                  register_callback));
 }
 

chrome/browser/services/gcm/push_messaging_service_impl.h

@@ -21,6 +21,7 @@ class PrefRegistrySyncable;
 namespace gcm {
 
 class GCMProfileService;
+struct PushMessagingApplicationId;
 
 class PushMessagingServiceImpl : public content::PushMessagingService,
                                  public GCMAppHandler {
@@ -47,7 +48,8 @@ class PushMessagingServiceImpl : public content::PushMessagingService,
 
   // content::PushMessagingService implementation:
   virtual void Register(
-      const std::string& app_id,
+      const GURL& origin,
+      int64 service_worker_registration_id,
       const std::string& sender_id,
       int renderer_id,
       int render_frame_id,
@@ -56,20 +58,18 @@ class PushMessagingServiceImpl : public content::PushMessagingService,
 
  private:
   void RegisterEnd(
-      const std::string& app_id,
       const content::PushMessagingService::RegisterCallback& callback,
       const std::string& registration_id,
       content::PushMessagingStatus status);
 
   void DidRegister(
-      const std::string& app_id,
       const content::PushMessagingService::RegisterCallback& callback,
       const std::string& registration_id,
       GCMClient::Result result);
 
   void DidRequestPermission(
+      const PushMessagingApplicationId& application_id,
       const std::string& sender_id,
-      const std::string& app_id,
       const content::PushMessagingService::RegisterCallback& callback,
       bool allow);
 

chrome/chrome_browser.gypi

@@ -1179,6 +1179,8 @@
       'browser/services/gcm/gcm_profile_service.h',
       'browser/services/gcm/gcm_profile_service_factory.cc',
       'browser/services/gcm/gcm_profile_service_factory.h',
+      'browser/services/gcm/push_messaging_application_id.cc',
+      'browser/services/gcm/push_messaging_application_id.h',
       'browser/services/gcm/push_messaging_infobar_delegate.cc',
       'browser/services/gcm/push_messaging_infobar_delegate.h',
       'browser/services/gcm/push_messaging_permission_context.cc',

chrome/chrome_tests_unit.gypi

@@ -1290,6 +1290,7 @@
         'browser/services/gcm/fake_signin_manager.h',
         'browser/services/gcm/gcm_account_tracker_unittest.cc',
         'browser/services/gcm/gcm_profile_service_unittest.cc',
+        'browser/services/gcm/push_messaging_application_id_unittest.cc',
         'browser/sessions/persistent_tab_restore_service_unittest.cc',
         'browser/sessions/restore_on_startup_policy_handler_unittest.cc',
         'browser/sessions/session_backend_unittest.cc',

content/browser/push_messaging_message_filter.cc

@@ -84,10 +84,8 @@ void PushMessagingMessageFilter::DoRegister(
         PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_SERVICE_NOT_AVAILABLE));
     return;
   }
-  // TODO(mvanouwerkerk): Is this the app_id format we want to use?
-  std::string app_id =
-      origin.spec() + " " + base::Int64ToString(service_worker_registration_id);
-  service()->Register(app_id,
+  service()->Register(origin,
+                      service_worker_registration_id,
                       sender_id,
                       render_process_id_,
                       render_frame_id,

content/content_browser.gypi

@@ -169,6 +169,7 @@
       'public/browser/power_save_blocker.h',
       'public/browser/profiler_controller.h',
       'public/browser/profiler_subscriber.h',
+      'public/browser/push_messaging_service.h',
       'public/browser/quota_permission_context.h',
       'public/browser/render_frame_host.h',
       'public/browser/render_process_host.h',

content/public/browser/push_messaging_service.h

@@ -24,7 +24,8 @@ class CONTENT_EXPORT PushMessagingService {
       RegisterCallback;
 
   virtual ~PushMessagingService() {}
-  virtual void Register(const std::string& app_id,
+  virtual void Register(const GURL& origin,
+                        int64 service_worker_registration_id,
                         const std::string& sender_id,
                         int renderer_id,
                         int render_frame_id,