[Push] Add expiration time property in app identifier

This change allows to save and retrieve an optional expiration time
property in the PushMessagingAppIdentifier and specifically in the
profile preferences.

It is saved in the same preference map as the (origin, SWR id) and will
be converted in a string as seconds from Windows epoch.

BUG: 1104215
Change-Id: I37d7f7cbc7d4160c049d72935b7d6d9bb308170e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2315682
Commit-Queue: Viviane Yang <viviy@google.com>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791731}

chrome/browser/push_messaging/push_messaging_app_identifier.cc

@@ -29,29 +29,60 @@ constexpr size_t kPrefixLength = sizeof(kPushMessagingAppIdentifierPrefix) - 1;
 constexpr size_t kGuidSuffixLength = sizeof(kInstanceIDGuidSuffix) - 1;
 
 // Ok to use '#' as separator since only the origin of the url is used.
-constexpr char kOriginSWRIdSeparator = '#';
+constexpr char kPrefValueSeparator = '#';
 constexpr size_t kGuidLength = 36;  // "%08X-%04X-%04X-%04X-%012llX"
 
-std::string MakePrefValue(const GURL& origin,
-                          int64_t service_worker_registration_id) {
-  return origin.spec() + kOriginSWRIdSeparator +
-         base::NumberToString(service_worker_registration_id);
+std::string FromTimeToString(base::Time time) {
+  DCHECK(!time.is_null());
+  return base::NumberToString(time.ToDeltaSinceWindowsEpoch().InSeconds());
 }
 
-bool GetOriginAndSWRFromPrefValue(const std::string& pref_value,
-                                  GURL* origin,
-                                  int64_t* service_worker_registration_id) {
+bool FromStringToTime(const std::string& time_string,
+                      base::Optional<base::Time>* time) {
+  DCHECK(!time_string.empty());
+  int64_t seconds;
+  if (base::StringToInt64(time_string, &seconds) && seconds > 0) {
+    *time = base::make_optional(base::Time::FromDeltaSinceWindowsEpoch(
+        base::TimeDelta::FromSeconds(seconds)));
+    return true;
+  }
+  return false;
+}
+
+std::string MakePrefValue(
+    const GURL& origin,
+    int64_t service_worker_registration_id,
+    const base::Optional<base::Time>& expiration_time = base::nullopt) {
+  std::string result = origin.spec() + kPrefValueSeparator +
+                       base::NumberToString(service_worker_registration_id);
+  if (expiration_time)
+    result += kPrefValueSeparator + FromTimeToString(*expiration_time);
+  return result;
+}
+
+bool DisassemblePrefValue(const std::string& pref_value,
+                          GURL* origin,
+                          int64_t* service_worker_registration_id,
+                          base::Optional<base::Time>* expiration_time) {
   std::vector<std::string> parts =
-      base::SplitString(pref_value, std::string(1, kOriginSWRIdSeparator),
+      base::SplitString(pref_value, std::string(1, kPrefValueSeparator),
                         base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
-  if (parts.size() != 2)
+
+  if (parts.size() < 2 || parts.size() > 3)
     return false;
 
   if (!base::StringToInt64(parts[1], service_worker_registration_id))
     return false;
 
   *origin = GURL(parts[0]);
-  return origin->is_valid();
+  if (!origin->is_valid()) {
+    return false;
+  }
+
+  if (parts.size() == 3)
+    return FromStringToTime(parts[2], expiration_time);
+
+  return true;
 }
 
 }  // namespace
@@ -75,25 +106,28 @@ bool PushMessagingAppIdentifier::UseInstanceID(const std::string& app_id) {
 // static
 PushMessagingAppIdentifier PushMessagingAppIdentifier::Generate(
     const GURL& origin,
-    int64_t service_worker_registration_id) {
+    int64_t service_worker_registration_id,
+    const base::Optional<base::Time>& expiration_time) {
   // All new push subscriptions use Instance ID tokens.
   return GenerateInternal(origin, service_worker_registration_id,
-                          true /* use_instance_id */);
+                          true /* use_instance_id */, expiration_time);
 }
 
 // static
 PushMessagingAppIdentifier PushMessagingAppIdentifier::LegacyGenerateForTesting(
     const GURL& origin,
-    int64_t service_worker_registration_id) {
+    int64_t service_worker_registration_id,
+    const base::Optional<base::Time>& expiration_time) {
   return GenerateInternal(origin, service_worker_registration_id,
-                          false /* use_instance_id */);
+                          false /* use_instance_id */, expiration_time);
 }
 
 // static
 PushMessagingAppIdentifier PushMessagingAppIdentifier::GenerateInternal(
     const GURL& origin,
     int64_t service_worker_registration_id,
-    bool use_instance_id) {
+    bool use_instance_id,
+    const base::Optional<base::Time>& expiration_time) {
   // 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());
@@ -103,10 +137,10 @@ PushMessagingAppIdentifier PushMessagingAppIdentifier::GenerateInternal(
   }
   CHECK(!guid.empty());
   std::string app_id = kPushMessagingAppIdentifierPrefix + origin.spec() +
-                       kOriginSWRIdSeparator + guid;
+                       kPrefValueSeparator + guid;
 
-  PushMessagingAppIdentifier app_identifier(app_id, origin,
-                                            service_worker_registration_id);
+  PushMessagingAppIdentifier app_identifier(
+      app_id, origin, service_worker_registration_id, expiration_time);
   app_identifier.DCheckValid();
   return app_identifier;
 }
@@ -136,14 +170,18 @@ PushMessagingAppIdentifier PushMessagingAppIdentifier::FindByAppId(
 
   GURL origin;
   int64_t service_worker_registration_id;
-  if (!GetOriginAndSWRFromPrefValue(*map_value, &origin,
-                                    &service_worker_registration_id)) {
+  base::Optional<base::Time> expiration_time;
+  // Try disassemble the pref value, return an invalid app identifier if the
+  // pref value is corrupted
+  if (!DisassemblePrefValue(*map_value, &origin,
+                            &service_worker_registration_id,
+                            &expiration_time)) {
     NOTREACHED();
     return PushMessagingAppIdentifier();
   }
 
-  PushMessagingAppIdentifier app_identifier(app_id, origin,
-                                            service_worker_registration_id);
+  PushMessagingAppIdentifier app_identifier(
+      app_id, origin, service_worker_registration_id, expiration_time);
   app_identifier.DCheckValid();
   return app_identifier;
 }
@@ -153,14 +191,15 @@ PushMessagingAppIdentifier PushMessagingAppIdentifier::FindByServiceWorker(
     Profile* profile,
     const GURL& origin,
     int64_t service_worker_registration_id) {
-  const base::Value pref_value =
-      base::Value(MakePrefValue(origin, service_worker_registration_id));
+  const std::string base_pref_value =
+      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(&pref_value))
+    if (base::StartsWith(it.value().GetString(), base_pref_value,
+                         base::CompareCase::SENSITIVE))
       return FindByAppId(profile, it.key());
   }
   return PushMessagingAppIdentifier();
@@ -196,16 +235,21 @@ size_t PushMessagingAppIdentifier::GetCount(Profile* profile) {
       ->size();
 }
 
+PushMessagingAppIdentifier::PushMessagingAppIdentifier(
+    const PushMessagingAppIdentifier& other) = default;
+
 PushMessagingAppIdentifier::PushMessagingAppIdentifier()
     : origin_(GURL::EmptyGURL()), service_worker_registration_id_(-1) {}
 
 PushMessagingAppIdentifier::PushMessagingAppIdentifier(
     const std::string& app_id,
     const GURL& origin,
-    int64_t service_worker_registration_id)
+    int64_t service_worker_registration_id,
+    const base::Optional<base::Time>& expiration_time)
     : app_id_(app_id),
       origin_(origin),
-      service_worker_registration_id_(service_worker_registration_id) {}
+      service_worker_registration_id_(service_worker_registration_id),
+      expiration_time_(expiration_time) {}
 
 PushMessagingAppIdentifier::~PushMessagingAppIdentifier() {}
 
@@ -223,8 +267,9 @@ void PushMessagingAppIdentifier::PersistToPrefs(Profile* profile) const {
   if (!old.is_null())
     map->RemoveKey(old.app_id_);
 
-  map->SetKey(app_id_, base::Value(MakePrefValue(
-                           origin_, service_worker_registration_id_)));
+  map->SetKey(app_id_,
+              base::Value(MakePrefValue(
+                  origin_, service_worker_registration_id_, expiration_time_)));
 }
 
 void PushMessagingAppIdentifier::DeleteFromPrefs(Profile* profile) const {
@@ -249,12 +294,12 @@ void PushMessagingAppIdentifier::DCheckValid() const {
 
   // Optional (origin.spec() + '#')
   if (app_id_.size() != kPrefixLength + kGuidLength) {
-    constexpr size_t suffix_length = 1 /* kOriginSWRIdSeparator */ + kGuidLength;
+    constexpr size_t suffix_length = 1 /* kPrefValueSeparator */ + kGuidLength;
     DCHECK_GT(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, kOriginSWRIdSeparator),
+    DCHECK_EQ(std::string(1, kPrefValueSeparator),
               app_id_.substr(app_id_.size() - suffix_length, 1));
   }
 

chrome/browser/push_messaging/push_messaging_app_identifier.h

@@ -12,6 +12,8 @@
 
 #include "base/check.h"
 #include "base/gtest_prod_util.h"
+#include "base/optional.h"
+#include "base/time/time.h"
 #include "url/gurl.h"
 
 class Profile;
@@ -41,7 +43,8 @@ class PushMessagingAppIdentifier {
   // Generates a new app identifier, with partially random app_id.
   static PushMessagingAppIdentifier Generate(
       const GURL& origin,
-      int64_t service_worker_registration_id);
+      int64_t service_worker_registration_id,
+      const base::Optional<base::Time>& expiration_time = base::nullopt);
 
   // Looks up an app identifier by app_id. If not found, is_null() will be true.
   static PushMessagingAppIdentifier FindByAppId(Profile* profile,
@@ -97,6 +100,14 @@ class PushMessagingAppIdentifier {
     return service_worker_registration_id_;
   }
 
+  base::Optional<base::Time> expiration_time() const {
+    DCHECK(!is_null());
+    return expiration_time_;
+  }
+
+  // Copy constructor
+  PushMessagingAppIdentifier(const PushMessagingAppIdentifier& other);
+
  private:
   friend class PushMessagingAppIdentifierTest;
   friend class PushMessagingBrowserTest;
@@ -105,19 +116,23 @@ class PushMessagingAppIdentifier {
   // Generates a new app identifier for legacy GCM (not modern InstanceID).
   static PushMessagingAppIdentifier LegacyGenerateForTesting(
       const GURL& origin,
-      int64_t service_worker_registration_id);
+      int64_t service_worker_registration_id,
+      const base::Optional<base::Time>& expiration_time = base::nullopt);
 
   static PushMessagingAppIdentifier GenerateInternal(
       const GURL& origin,
       int64_t service_worker_registration_id,
-      bool use_instance_id);
+      bool use_instance_id,
+      const base::Optional<base::Time>& expiration_time = base::nullopt);
 
   // Constructs an invalid app identifier.
   PushMessagingAppIdentifier();
   // Constructs a valid app identifier.
-  PushMessagingAppIdentifier(const std::string& app_id,
-                             const GURL& origin,
-                             int64_t service_worker_registration_id);
+  PushMessagingAppIdentifier(
+      const std::string& app_id,
+      const GURL& origin,
+      int64_t service_worker_registration_id,
+      const base::Optional<base::Time>& expiration_time = base::nullopt);
 
   // Validates that all the fields contain valid values.
   void DCheckValid() const;
@@ -125,6 +140,7 @@ class PushMessagingAppIdentifier {
   std::string app_id_;
   GURL origin_;
   int64_t service_worker_registration_id_;
+  base::Optional<base::Time> expiration_time_;
 };
 
 #endif  // CHROME_BROWSER_PUSH_MESSAGING_PUSH_MESSAGING_APP_IDENTIFIER_H_

chrome/browser/push_messaging/push_messaging_app_identifier_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <stdint.h>
 
+#include "base/time/time.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -18,8 +19,12 @@ void ExpectAppIdentifiersEqual(const PushMessagingAppIdentifier& a,
   EXPECT_EQ(a.origin(), b.origin());
   EXPECT_EQ(a.service_worker_registration_id(),
             b.service_worker_registration_id());
+  EXPECT_EQ(a.expiration_time(), b.expiration_time());
 }
 
+base::Time kExpirationTime =
+    base::Time::FromDeltaSinceWindowsEpoch(base::TimeDelta::FromSeconds(1));
+
 }  // namespace
 
 class PushMessagingAppIdentifierTest : public testing::Test {
@@ -45,6 +50,11 @@ class PushMessagingAppIdentifierTest : public testing::Test {
         GURL("https://foobar.example.com/"), 1);
     different_sw_ = PushMessagingAppIdentifier::Generate(
         GURL("https://www.example.com/"), 42);
+    with_et_ = PushMessagingAppIdentifier::Generate(
+        GURL("https://www.example.com/"), 1, kExpirationTime);
+    different_et_ = PushMessagingAppIdentifier::Generate(
+        GURL("https://www.example.com/"), 1,
+        kExpirationTime + base::TimeDelta::FromSeconds(100));
   }
 
   Profile* profile() { return &profile_; }
@@ -53,6 +63,8 @@ class PushMessagingAppIdentifierTest : public testing::Test {
   PushMessagingAppIdentifier same_origin_and_sw_;
   PushMessagingAppIdentifier different_origin_;
   PushMessagingAppIdentifier different_sw_;
+  PushMessagingAppIdentifier different_et_;
+  PushMessagingAppIdentifier with_et_;
 
  private:
   content::BrowserTaskEnvironment task_environment_;
@@ -254,3 +266,45 @@ TEST_F(PushMessagingAppIdentifierTest, GetAll) {
   EXPECT_TRUE(contained_different_origin);
   EXPECT_TRUE(contained_different_sw);
 }
+
+TEST_F(PushMessagingAppIdentifierTest, PersistWithExpirationTime) {
+  ASSERT_TRUE(with_et_.expiration_time());
+  ASSERT_TRUE(different_et_.expiration_time());
+  ASSERT_EQ(with_et_.origin(), different_et_.origin());
+  ASSERT_EQ(with_et_.service_worker_registration_id(),
+            different_et_.service_worker_registration_id());
+  ASSERT_FALSE(kExpirationTime.is_null());
+
+  different_et_.PersistToPrefs(profile());
+
+  // Test PersistToPrefs and FindByAppId, whether expiration time is saved
+  // properly
+  std::vector<PushMessagingAppIdentifier> all_app_identifiers =
+      PushMessagingAppIdentifier::GetAll(profile());
+  EXPECT_EQ(1u, all_app_identifiers.size());
+  {
+    PushMessagingAppIdentifier found_by_app_id =
+        PushMessagingAppIdentifier::FindByAppId(profile(),
+                                                different_et_.app_id());
+    // Check whether expiration time was saved
+    ExpectAppIdentifiersEqual(found_by_app_id, different_et_);
+  }
+  with_et_.PersistToPrefs(profile());
+  {
+    all_app_identifiers = PushMessagingAppIdentifier::GetAll(profile());
+    EXPECT_EQ(1u, all_app_identifiers.size());
+  }
+  {
+    PushMessagingAppIdentifier found_by_with_et_app_id =
+        PushMessagingAppIdentifier::FindByAppId(profile(), with_et_.app_id());
+    EXPECT_FALSE(found_by_with_et_app_id.is_null());
+    EXPECT_EQ(found_by_with_et_app_id.expiration_time(), kExpirationTime);
+    ExpectAppIdentifiersEqual(found_by_with_et_app_id, with_et_);
+  }
+  {
+    PushMessagingAppIdentifier found_by_different_et_app_id =
+        PushMessagingAppIdentifier::FindByAppId(profile(),
+                                                different_et_.app_id());
+    EXPECT_TRUE(found_by_different_et_app_id.is_null());
+  }
+}