Make Web Push use InstanceID tokens instead of GCM registrations

chrome/browser/push_messaging/push_messaging_app_identifier.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/push_messaging/push_messaging_app_identifier.h"
 
 #include <string.h>
 
 #include "base/guid.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 
 const char kPushMessagingAppIdentifierPrefix[] = "wp:";
+const char kInstanceIDGuidSuffix[] = "-V2";
 
 namespace {
 
 // sizeof is strlen + 1 since it's null-terminated.
 const size_t kPrefixLength = sizeof(kPushMessagingAppIdentifierPrefix) - 1;
+const size_t kGuidSuffixLength = sizeof(kInstanceIDGuidSuffix) - 1;
 
 const char kSeparator = '#';    // Ok as only the origin of the url is used.
 const size_t kGuidLength = 36;  // "%08X-%04X-%04X-%04X-%012llX"
 
 std::string MakePrefValue(const GURL& origin,
                           int64_t service_worker_registration_id) {
   return origin.spec() + kSeparator +
          base::Int64ToString(service_worker_registration_id);
 }
 
@@ skipping 19 matching lines @@
 void PushMessagingAppIdentifier::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   // TODO(johnme): If push becomes enabled in incognito, be careful that this
   // pref is read from the right profile, as prefs defined in a regular profile
   // are visible in the corresponding incognito profile unless overridden.
   // TODO(johnme): Make sure this pref doesn't get out of sync after crashes.
   registry->RegisterDictionaryPref(prefs::kPushMessagingAppIdentifierMap);
 }
 
 // static
+bool PushMessagingAppIdentifier::UseInstanceID(const std::string& app_id) {
+  return app_id.length() > kPrefixLength + kGuidLength &&
+         kInstanceIDGuidSuffix == app_id.substr(app_id.size() - 3);
+}
+
+// static
 PushMessagingAppIdentifier PushMessagingAppIdentifier::Generate(
     const GURL& origin,
     int64_t service_worker_registration_id) {
   // Use uppercase GUID for consistency with GUIDs Push has already sent to GCM.
   // Also allows detecting case mangling; see code commented "crbug.com/461867".
   std::string guid = base::ToUpperASCII(base::GenerateGUID());
+  // All new push subscriptions are Instance ID tokens.
+  guid.replace(guid.size() - kGuidSuffixLength, kGuidSuffixLength,
+               kInstanceIDGuidSuffix);
   CHECK(!guid.empty());
   std::string app_id =
       kPushMessagingAppIdentifierPrefix + origin.spec() + kSeparator + guid;
 
   PushMessagingAppIdentifier app_identifier(app_id, origin,
                                             service_worker_registration_id);
   app_identifier.DCheckValid();
   return app_identifier;
 }
 
@@ skipping 115 matching lines @@
 
 void PushMessagingAppIdentifier::DCheckValid() const {
   DCHECK_GE(service_worker_registration_id_, 0);
 
   DCHECK(origin_.is_valid());
   DCHECK_EQ(origin_.GetOrigin(), origin_);
 
   // "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);

[Peter Beverloo, 2016/06/02 15:53:17]

nit: DCHECK_GT

[johnme, 2016/06/07 14:16:42]

Done.

     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)));
+
+  // GUID, optionally with last few chars replaced with kInstanceIDGuidSuffix.
+  DCHECK(base::IsValidGUID(
+      app_id_.substr(app_id_.size() - kGuidLength,
+                     kGuidLength - kGuidSuffixLength) +
+      // It's ok for it to end with kInstanceIDGuidSuffix; if so, pass
+      // arbitrary hex of the same length so that IsValidGUID passes.
+      (kInstanceIDGuidSuffix ==
+               app_id_.substr(app_id_.size() - kGuidSuffixLength)
+           ? std::string(kGuidSuffixLength, 'C')  // arbitrary hex
+           : app_id_.substr(app_id_.size() - kGuidSuffixLength)  // real suffix
+       )));

[Peter Beverloo, 2016/06/02 15:53:17]

This is a ten line DCHECK containing significant logic, the complication of this
is way too high.

Let's instead separate this out in a series of variables living on the stack.
How about the following?

===============
  // App identifiers created for InstanceID-based subscriptions have a suffix
  // allowing them to be identified as such, whilst breaking the GUID syntax.
  std::string guid = app_id_.substr(app_id_.size() - kGuidLength);
  bool is_instance_id =
      base::EndsWith(guid, kInstanceIDGuidSuffix, base::CompareCase::SENSITIVE);

  if (is_instance_id) {
    DCHECK(!base::IsValidGUID(guid));

    // Replace the suffix with valid GUID characters in order to be able to
    // validate the rest of the string.
    guid = guid.replace(guid.size() - kInstanceIDGuidSuffix,
                        kInstanceIDGuidSuffix, kInstanceIDGuidSuffix, 'C');
  }

  DCHECK(base::IsValidGUID(guid));
===============

That's incredibly much easier to read.

If you're worried about compiling that code in non-debug builds, let's stick the
entire body of this method in an "#if DCHECK_IS_ON()" block.

[johnme, 2016/06/07 14:16:42]

Done.

 }