Announcement Notification: Use base::Time::FromUTCString().

Currently a server time string is parsed by base::Time::FromString
in announcement notification system. When not specify the time zone,
then Chrome uses local time zone.

We should use base::Time::FromUTCString that assumes UTC time when
there is no time zone in the server time string.

Bug: 1048437
Change-Id: Iba84f557fdcea19a5635f00c2f7ebc3c41279d7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037013Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740402}

chrome/browser/updates/announcement_notification/announcement_notification_service.cc

@@ -70,7 +70,7 @@ class AnnouncementNotificationServiceImpl
     remote_url_ = base::GetFieldTrialParamValueByFeature(
         kAnnouncementNotification, kAnnouncementUrl);
 
-    bool success = base::Time::FromString(
+    bool success = base::Time::FromUTCString(
         base::GetFieldTrialParamValueByFeature(kAnnouncementNotification,
                                                kSkipFirstRunAfterTime)
             .c_str(),

chrome/browser/updates/announcement_notification/announcement_notification_service.h

@@ -32,8 +32,8 @@ constexpr char kSkipFirstRun[] = "skip_first_run";
 
 // The Finch parameter name for a string value that represents a time.
 // If first run happens after this time, notification will not show.
-// The string defined in Finch config should specify UTC time, and will be
-// parsed by base::Time::FromString().
+// The string defined in Finch config should specify the time zone.
+// e.g. 02 Feb 2020 13:00:00 GMT.
 constexpr char kSkipFirstRunAfterTime[] = "skip_first_run_after_time";
 
 // The Finch parameter name for a boolean value that whether to show

chrome/browser/updates/announcement_notification/announcement_notification_service_unittest.cc

@@ -73,9 +73,11 @@ class AnnouncementNotificationServiceTest : public testing::Test {
     return time;
   }
 
+  // Sets the current time used in test. Assume UTC time when time zone is not
+  // specified.
   base::Time SetNow(const char* now_str) {
     base::Time now;
-    EXPECT_TRUE(base::Time::FromString(now_str, &now));
+    EXPECT_TRUE(base::Time::FromUTCString(now_str, &now));
     clock()->SetNow(now);
     EXPECT_FALSE(now.is_null());
     return now;
@@ -196,14 +198,14 @@ TEST_F(AnnouncementNotificationServiceTest, SaveFirstRunTimeNotFirstRun) {
 // Not to show notification if first run timestamp happens after Finch parameter
 // timestamp.
 TEST_F(AnnouncementNotificationServiceTest, SkipFirstRunAfterTimeNotShow) {
-  SetNow("10 Feb 2020 13:00:00");
+  SetNow("10 Feb 2020 13:00:00 GMT");
   std::map<std::string, std::string> parameters = {
       {kSkipFirstRun, "false"},
       {kVersion, "2"},
       {kSkipNewProfile, "false"},
-      {kSkipFirstRunAfterTime, "10 Feb 2020 12:15:00"}};
+      {kSkipFirstRunAfterTime, "10 Feb 2020 12:15:00 GMT"}};
   Init(parameters, true, false, 1, false);
-  auto first_run_time = SetFirstRunTimePref("10 Feb 2020 12:30:00");
+  auto first_run_time = SetFirstRunTimePref("10 Feb 2020 12:30:00 GMT");
   ON_CALL(*delegate(), IsFirstRun()).WillByDefault(Return(false));
   EXPECT_CALL(*delegate(), ShowNotification()).Times(0);
   service()->MaybeShowNotification();
@@ -214,14 +216,14 @@ TEST_F(AnnouncementNotificationServiceTest, SkipFirstRunAfterTimeNotShow) {
 // Show notification if first run timestamp happens before Finch parameter
 // timestamp.
 TEST_F(AnnouncementNotificationServiceTest, SkipFirstRunAfterTimeShow) {
-  SetNow("10 Feb 2020 13:00:00");
+  SetNow("10 Feb 2020 13:00:00 GMT");
   std::map<std::string, std::string> parameters = {
       {kSkipFirstRun, "false"},
       {kVersion, "2"},
       {kSkipNewProfile, "false"},
-      {kSkipFirstRunAfterTime, "10 Feb 2020 12:15:00"}};
+      {kSkipFirstRunAfterTime, "10 Feb 2020 12:15:00 GMT"}};
   Init(parameters, true, false, 1, false);
-  SetFirstRunTimePref("10 Feb 2020 12:10:00");
+  SetFirstRunTimePref("10 Feb 2020 12:10:00 GMT");
   ON_CALL(*delegate(), IsFirstRun()).WillByDefault(Return(false));
   EXPECT_CALL(*delegate(), ShowNotification());
   service()->MaybeShowNotification();
@@ -231,12 +233,12 @@ TEST_F(AnnouncementNotificationServiceTest, SkipFirstRunAfterTimeShow) {
 // Show notification if there is no first run timestamp but Finch has
 // "skip_first_run_after_time" parameter.
 TEST_F(AnnouncementNotificationServiceTest, SkipFirstRunAfterNoFirstRunPref) {
-  SetNow("10 Feb 2020 13:00:00");
+  SetNow("10 Feb 2020 13:00:00 GMT");
   std::map<std::string, std::string> parameters = {
       {kSkipFirstRun, "false"},
       {kVersion, "2"},
       {kSkipNewProfile, "false"},
-      {kSkipFirstRunAfterTime, "10 Feb 2020 12:15:00"}};
+      {kSkipFirstRunAfterTime, "10 Feb 2020 12:15:00 GMT"}};
   Init(parameters, true, false, 1, false);
   EXPECT_EQ(FirstRunTimePref(), base::Time());
   ON_CALL(*delegate(), IsFirstRun()).WillByDefault(Return(false));
@@ -246,6 +248,23 @@ TEST_F(AnnouncementNotificationServiceTest, SkipFirstRunAfterNoFirstRunPref) {
   EXPECT_EQ(FirstRunTimePref(), base::Time());
 }
 
+// "skip_first_run_after_time" parameter should assume UTC when time zone is not
+// specified.
+TEST_F(AnnouncementNotificationServiceTest, SkipFirstRunAfterAssumeUTCTime) {
+  SetNow("10 Feb 2020 13:00:00 GMT");
+  std::map<std::string, std::string> parameters = {
+      {kSkipFirstRun, "false"},
+      {kVersion, "2"},
+      {kSkipNewProfile, "false"},
+      // No time zone specified. The time string should assume UTC.
+      {kSkipFirstRunAfterTime, "10 Feb 2020 12:59:59"}};
+  Init(parameters, true, false, 1, false);
+  EXPECT_EQ(FirstRunTimePref(), base::Time());
+  ON_CALL(*delegate(), IsFirstRun()).WillByDefault(Return(false));
+  EXPECT_CALL(*delegate(), ShowNotification());
+  service()->MaybeShowNotification();
+}
+
 struct VersionTestParam {
   bool enable_feature;
   bool skip_first_run;