Push API: Include origin in generated app_ids

chrome/browser/push_messaging/push_messaging_app_identifier.cc

Index: chrome/browser/push_messaging/push_messaging_app_identifier.cc
diff --git a/chrome/browser/push_messaging/push_messaging_app_identifier.cc b/chrome/browser/push_messaging/push_messaging_app_identifier.cc
index feb069a006cdea7024aaf665d1c6bc2b3091624a..0c431e17a6fc8ca549f1f8474ac196f53d30a4bc 100644
--- a/chrome/browser/push_messaging/push_messaging_app_identifier.cc
+++ b/chrome/browser/push_messaging/push_messaging_app_identifier.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/push_messaging/push_messaging_app_identifier.h"
 
+#include <string.h>
+
 #include "base/guid.h"
 #include "base/logging.h"
 #include "base/prefs/pref_service.h"
@@ -16,11 +18,38 @@
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 
+const char kPushMessagingAppIdentifierPrefix[] = "wp:";
+
 namespace {
+
+// sizeof is strlen + 1 since it's null-terminated.
+const size_t kPrefixLength = sizeof(kPushMessagingAppIdentifierPrefix) - 1;
+
 const char kSeparator = '#';  // Ok as only the origin of the url is used.
-}  // namespace
+const size_t kGuidLength = 36;  // "%08X-%04X-%04X-%04X-%012llX"
 
-const char kPushMessagingAppIdentifierPrefix[] = "wp:";
+std::string MakePrefValue(const GURL& origin,
+                          int64_t service_worker_registration_id) {
+  return origin.spec() + kSeparator
+                       + base::Int64ToString(service_worker_registration_id);
+}
+
+bool GetOriginAndSWRFromPrefValue(
+    const std::string& pref_value, GURL* origin,
+    int64_t* service_worker_registration_id) {
+  std::vector<std::string> parts;
+  base::SplitString(pref_value, kSeparator, &parts);
+  if (parts.size() != 2)
+    return false;
+
+  if (!base::StringToInt64(parts[1], service_worker_registration_id))
+    return false;
+
+  *origin = GURL(parts[0]);
+  return origin->is_valid();
+}
+
+}  // namespace
 
 // static
 void PushMessagingAppIdentifier::RegisterProfilePrefs(
@@ -34,7 +63,8 @@ PushMessagingAppIdentifier PushMessagingAppIdentifier::Generate(
 {
   std::string guid = base::GenerateGUID();
   CHECK(!guid.empty());
-  std::string app_id = kPushMessagingAppIdentifierPrefix + guid;
+  std::string app_id = kPushMessagingAppIdentifierPrefix + origin.spec()
+                     + kSeparator + guid;
 
   PushMessagingAppIdentifier app_identifier(app_id, origin,
                                             service_worker_registration_id);
@@ -43,55 +73,49 @@ PushMessagingAppIdentifier PushMessagingAppIdentifier::Generate(
 }
 
 // static
-PushMessagingAppIdentifier PushMessagingAppIdentifier::Get(
+PushMessagingAppIdentifier PushMessagingAppIdentifier::FindByAppId(
     Profile* profile, const std::string& app_id) {
-  // Workaround crbug.com/461867 in GCM where it converts subtypes to lowercase.
-  // TODO(johnme): Remove this when obsolete
-  const size_t prefix_len = strlen(kPushMessagingAppIdentifierPrefix);
-  if (app_id.size() < prefix_len)
-    return PushMessagingAppIdentifier();
-  std::string uppercase_app_id =
-      app_id.substr(0, prefix_len) +
-      StringToUpperASCII(app_id.substr(prefix_len, std::string::npos));
+  // Check case of app_id hasn't been mangled (crbug.com/461867).
+  DCHECK_GE(app_id.size(), kPrefixLength + kGuidLength);
+  DCHECK_EQ(kPushMessagingAppIdentifierPrefix, app_id.substr(0, kPrefixLength));
+  DCHECK_EQ(app_id.substr(app_id.size() - kGuidLength),
+            StringToUpperASCII(app_id.substr(app_id.size() - kGuidLength)));
 
   const base::DictionaryValue* map =
       profile->GetPrefs()->GetDictionary(prefs::kPushMessagingAppIdentifierMap);
 
-  std::string origin_and_sw_id;
-  if (!map->GetStringWithoutPathExpansion(uppercase_app_id, &origin_and_sw_id))
-    return PushMessagingAppIdentifier();
-
-  std::vector<std::string> parts;
-  base::SplitString(origin_and_sw_id, kSeparator, &parts);
-  if (parts.size() != 2)
+  std::string map_value;
+  if (!map->GetStringWithoutPathExpansion(app_id, &map_value))
     return PushMessagingAppIdentifier();
 
-  GURL origin = GURL(parts[0]);
-
+  GURL origin;
   int64_t service_worker_registration_id;
-  if (!base::StringToInt64(parts[1], &service_worker_registration_id))
+  if (!GetOriginAndSWRFromPrefValue(map_value, &origin,
+                                    &service_worker_registration_id)) {
+    NOTREACHED();
     return PushMessagingAppIdentifier();
+  }
 
-  PushMessagingAppIdentifier app_identifier(uppercase_app_id, origin,
+  PushMessagingAppIdentifier app_identifier(app_id, origin,
                                             service_worker_registration_id);
   app_identifier.DCheckValid();
   return app_identifier;
 }
 
 // static
-PushMessagingAppIdentifier PushMessagingAppIdentifier::Get(
+PushMessagingAppIdentifier PushMessagingAppIdentifier::FindByServiceWorker(
     Profile* profile, const GURL& origin,
     int64_t service_worker_registration_id)
 {
-  base::StringValue origin_and_sw_id = base::StringValue(origin.spec() +
-      kSeparator + base::Int64ToString(service_worker_registration_id));
+  const base::StringValue pref_value =
+      base::StringValue(MakePrefValue(origin, service_worker_registration_id));
 
   const base::DictionaryValue* map =
       profile->GetPrefs()->GetDictionary(prefs::kPushMessagingAppIdentifierMap);
   for (auto it = base::DictionaryValue::Iterator(*map); !it.IsAtEnd();
        it.Advance()) {
-    if (it.value().Equals(&origin_and_sw_id))
-      return Get(profile, it.key());
+    if (it.value().Equals(&pref_value))
+      return FindByAppId(profile, it.key());
   }
   return PushMessagingAppIdentifier();
 }
@@ -105,7 +129,7 @@ std::vector<PushMessagingAppIdentifier> PushMessagingAppIdentifier::GetAll(
       profile->GetPrefs()->GetDictionary(prefs::kPushMessagingAppIdentifierMap);
   for (auto it = base::DictionaryValue::Iterator(*map); !it.IsAtEnd();
        it.Advance()) {
-    result.push_back(Get(profile, it.key()));
+    result.push_back(FindByAppId(profile, it.key()));
   }
 
   return result;
@@ -137,14 +161,13 @@ void PushMessagingAppIdentifier::PersistToPrefs(Profile* profile) const {
 
   // Delete any stale entry with the same origin and Service Worker
   // registration id (hence we ensure there is a 1:1 not 1:many mapping).
-  PushMessagingAppIdentifier old = Get(profile, origin_,
-                                       service_worker_registration_id_);
+  PushMessagingAppIdentifier old = FindByServiceWorker(
+      profile, origin_, service_worker_registration_id_);
   if (!old.is_null())
-    map->RemoveWithoutPathExpansion(old.app_id_, nullptr);
+    map->RemoveWithoutPathExpansion(old.app_id_, nullptr /* out_value */);
 
-  std::string origin_and_sw_id = origin_.spec() + kSeparator +
-      base::Int64ToString(service_worker_registration_id_);
-  map->SetStringWithoutPathExpansion(app_id_, origin_and_sw_id);
+  map->SetStringWithoutPathExpansion(
+      app_id_, MakePrefValue(origin_, service_worker_registration_id_));
 }
 
 void PushMessagingAppIdentifier::DeleteFromPrefs(Profile* profile) const {
@@ -153,14 +176,27 @@ void PushMessagingAppIdentifier::DeleteFromPrefs(Profile* profile) const {
   DictionaryPrefUpdate update(profile->GetPrefs(),
                               prefs::kPushMessagingAppIdentifierMap);
   base::DictionaryValue* map = update.Get();
-  map->RemoveWithoutPathExpansion(app_id_, nullptr);
+  map->RemoveWithoutPathExpansion(app_id_, nullptr /* out_value */);
 }
 
 void PushMessagingAppIdentifier::DCheckValid() const {
-  const size_t prefix_len = strlen(kPushMessagingAppIdentifierPrefix);
   DCHECK_GE(service_worker_registration_id_, 0);
+
   DCHECK(origin_.is_valid());
   DCHECK_EQ(origin_.GetOrigin(), origin_);
-  DCHECK_EQ(app_id_.substr(0, prefix_len), kPushMessagingAppIdentifierPrefix);
-  DCHECK(base::IsValidGUID(app_id_.substr(prefix_len, std::string::npos)));
+
+  // "wp:"
+  DCHECK_EQ(kPushMessagingAppIdentifierPrefix,
+            app_id_.substr(0, kPrefixLength));
+  // Optional (origin.spec() + '#')
+  if (app_id_.size() != kPrefixLength + kGuidLength) {
+    const size_t suffix_length = 1 /* kSeparator */ + kGuidLength;
+    DCHECK(app_id_.size() > kPrefixLength + suffix_length);
+    DCHECK_EQ(origin_, GURL(app_id_.substr(
+        kPrefixLength, app_id_.size() - kPrefixLength - suffix_length)));
+    DCHECK_EQ(std::string(1, kSeparator),
+              app_id_.substr(app_id_.size() - suffix_length, 1));
+  }
+  // GUID
+  DCHECK(base::IsValidGUID(app_id_.substr(app_id_.size() - kGuidLength)));
 }