Safety check: move timestamp when safety check ran to backend

* Allows reusing time-based logic for when safety check ran (e.g.
  string construction) to be reused across platforms.

Bug: 1015841
Change-Id: I6db5be39619d510f1a6c95d16767275146477c84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144090
Commit-Queue: Rainhard Findling <rainhard@chromium.org>
Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760007}

chrome/browser/resources/settings/safety_check_page/safety_check_browser_proxy.js

@@ -114,10 +114,9 @@ cr.define('settings', function() {
     /**
      * Get the display string for the safety check parent, showing how long ago
      * safety check last ran.`
-     * @param {number} timestamp The timestamp safety check last ran.
      * @return {!Promise<string>}
      */
-    getParentRanDisplayString(timestamp) {}
+    getParentRanDisplayString() {}
   }
 
   /** @implements {settings.SafetyCheckBrowserProxy} */
@@ -128,8 +127,8 @@ cr.define('settings', function() {
     }
 
     /** @override */
-    getParentRanDisplayString(timestamp) {
-      return cr.sendWithPromise('getSafetyCheckRanDisplayString', timestamp);
+    getParentRanDisplayString() {
+      return cr.sendWithPromise('getSafetyCheckRanDisplayString');
     }
   }
 

chrome/browser/resources/settings/safety_check_page/safety_check_page.js

@@ -218,11 +218,9 @@ Polymer({
     this.parentDisplayString_ = event.displayString;
     if (this.parentStatus_ === settings.SafetyCheckParentStatus.AFTER) {
       // Start periodic safety check parent ran string updates.
-      const timestamp = Date.now();
       const update = async () => {
         this.parentDisplayString_ =
-            await this.safetyCheckBrowserProxy_.getParentRanDisplayString(
-                timestamp);
+            await this.safetyCheckBrowserProxy_.getParentRanDisplayString();
       };
       clearInterval(this.updateTimerId_);
       this.updateTimerId_ = setInterval(update, 60000);

chrome/browser/ui/webui/settings/safety_check_handler.cc

@@ -164,12 +164,11 @@ void SafetyCheckHandler::HandlePerformSafetyCheck(const base::ListValue* args) {
 void SafetyCheckHandler::HandleGetParentRanDisplayString(
     const base::ListValue* args) {
   const base::Value* callback_id;
-  double timestampRanDouble;
   CHECK(args->Get(0, &callback_id));
-  CHECK(args->GetDouble(1, &timestampRanDouble));
 
   ResolveJavascriptCallback(
-      *callback_id, base::Value(GetStringForParentRan(timestampRanDouble)));
+      *callback_id,
+      base::Value(GetStringForParentRan(safety_check_completion_time_)));
 }
 
 void SafetyCheckHandler::CheckUpdates() {
@@ -492,47 +491,48 @@ base::string16 SafetyCheckHandler::GetStringForExtensions(
   }
 }
 
-base::string16 SafetyCheckHandler::GetStringForParentRan(double timestamp_ran) {
-  return SafetyCheckHandler::GetStringForParentRan(timestamp_ran,
+base::string16 SafetyCheckHandler::GetStringForParentRan(
+    base::Time safety_check_completion_time) {
+  return SafetyCheckHandler::GetStringForParentRan(safety_check_completion_time,
                                                    base::Time::Now());
 }
 
 base::string16 SafetyCheckHandler::GetStringForParentRan(
-    double timestamp_ran,
+    base::Time safety_check_completion_time,
     base::Time system_time) {
-  const base::Time timeRan = base::Time::FromJsTime(timestamp_ran);
-  base::Time::Exploded timeRanExploded;
-  timeRan.LocalExplode(&timeRanExploded);
+  base::Time::Exploded completion_time_exploded;
+  safety_check_completion_time.LocalExplode(&completion_time_exploded);
 
-  base::Time::Exploded systemTimeExploded;
-  system_time.LocalExplode(&systemTimeExploded);
+  base::Time::Exploded system_time_exploded;
+  system_time.LocalExplode(&system_time_exploded);
 
-  const base::Time timeYesterday = system_time - base::TimeDelta::FromDays(1);
-  base::Time::Exploded timeYesterdayExploded;
-  timeYesterday.LocalExplode(&timeYesterdayExploded);
+  const base::Time time_yesterday = system_time - base::TimeDelta::FromDays(1);
+  base::Time::Exploded time_yesterday_exploded;
+  time_yesterday.LocalExplode(&time_yesterday_exploded);
 
-  const auto timeDiff = system_time - timeRan;
-  if (timeRanExploded.year == systemTimeExploded.year &&
-      timeRanExploded.month == systemTimeExploded.month &&
-      timeRanExploded.day_of_month == systemTimeExploded.day_of_month) {
+  const auto time_diff = system_time - safety_check_completion_time;
+  if (completion_time_exploded.year == system_time_exploded.year &&
+      completion_time_exploded.month == system_time_exploded.month &&
+      completion_time_exploded.day_of_month ==
+          system_time_exploded.day_of_month) {
     // Safety check ran today.
-    const int timeDiffInMinutes = timeDiff.InMinutes();
-    if (timeDiffInMinutes == 0) {
+    const int time_diff_in_mins = time_diff.InMinutes();
+    if (time_diff_in_mins == 0) {
       return l10n_util::GetStringUTF16(
           IDS_SETTINGS_SAFETY_CHECK_PARENT_PRIMARY_LABEL_AFTER);
-    } else if (timeDiffInMinutes < 60) {
+    } else if (time_diff_in_mins < 60) {
       return l10n_util::GetPluralStringFUTF16(
           IDS_SETTINGS_SAFETY_CHECK_PARENT_PRIMARY_LABEL_AFTER_MINS,
-          timeDiffInMinutes);
+          time_diff_in_mins);
     } else {
       return l10n_util::GetPluralStringFUTF16(
           IDS_SETTINGS_SAFETY_CHECK_PARENT_PRIMARY_LABEL_AFTER_HOURS,
-          timeDiffInMinutes / 60);
+          time_diff_in_mins / 60);
     }
-  } else if (timeRanExploded.year == timeYesterdayExploded.year &&
-             timeRanExploded.month == timeYesterdayExploded.month &&
-             timeRanExploded.day_of_month ==
-                 timeYesterdayExploded.day_of_month) {
+  } else if (completion_time_exploded.year == time_yesterday_exploded.year &&
+             completion_time_exploded.month == time_yesterday_exploded.month &&
+             completion_time_exploded.day_of_month ==
+                 time_yesterday_exploded.day_of_month) {
     // Safety check ran yesterday.
     return l10n_util::GetStringUTF16(
         IDS_SETTINGS_SAFETY_CHECK_PARENT_PRIMARY_LABEL_AFTER_YESTERDAY);
@@ -541,10 +541,10 @@ base::string16 SafetyCheckHandler::GetStringForParentRan(
     // TODO(crbug.com/1015841): While a minor issue, this is not be the ideal
     // way to calculate the days passed since safety check ran. For example,
     // <48 h might still be 2 days ago.
-    const int timeDiffInDays = timeDiff.InDays();
+    const int time_diff_in_days = time_diff.InDays();
     return l10n_util::GetPluralStringFUTF16(
         IDS_SETTINGS_SAFETY_CHECK_PARENT_PRIMARY_LABEL_AFTER_DAYS,
-        timeDiffInDays);
+        time_diff_in_days);
   }
 }
 
@@ -651,11 +651,10 @@ void SafetyCheckHandler::CompleteParentIfChildrenCompleted() {
       passwords_status_ != PasswordsStatus::kChecking &&
       safe_browsing_status_ != SafeBrowsingStatus::kChecking &&
       extensions_status_ != ExtensionsStatus::kChecking) {
-    // TODO(crbug.com/1015841): Minor improvement: also store the timestamp when
-    // safety check completed in the backend instead of in the frontend. This
-    // allows computing the safety check parent ran string on all platforms
-    // without the frontend handling the timestamp.
     parent_status_ = ParentStatus::kAfter;
+    // Remember when safety check completed.
+    safety_check_completion_time_ = base::Time::Now();
+    // Update UI.
     FireBasicSafetyCheckWebUiListener(kParentEvent,
                                       static_cast<int>(parent_status_),
                                       GetStringForParent(parent_status_));

chrome/browser/ui/webui/settings/safety_check_handler.h

@@ -103,9 +103,9 @@ class SafetyCheckHandler
 
   // Constructs the 'safety check ran' display string by how long ago safety
   // check ran.
-  base::string16 GetStringForParentRan(double timestamp_ran);
-  base::string16 GetStringForParentRan(double timestamp_ran,
-                                       base::Time systemTime);
+  base::string16 GetStringForParentRan(base::Time safety_check_completion_time);
+  base::string16 GetStringForParentRan(base::Time safety_check_completion_time,
+                                       base::Time system_time);
 
  protected:
   SafetyCheckHandler(std::unique_ptr<VersionUpdater> version_updater,
@@ -222,6 +222,9 @@ class SafetyCheckHandler
   SafeBrowsingStatus safe_browsing_status_ = SafeBrowsingStatus::kChecking;
   ExtensionsStatus extensions_status_ = ExtensionsStatus::kChecking;
 
+  // System time when safety check completed.
+  base::Time safety_check_completion_time_;
+
   std::unique_ptr<VersionUpdater> version_updater_;
   password_manager::BulkLeakCheckService* leak_service_ = nullptr;
   extensions::PasswordsPrivateDelegate* passwords_delegate_ = nullptr;

chrome/browser/ui/webui/settings/safety_check_handler_unittest.cc

@@ -1034,7 +1034,7 @@ TEST_F(SafetyCheckHandlerTest, CheckParentRanDisplayString) {
   // 1 second before midnight Dec 31st 2020, so that -(24h-1s) is still on the
   // same day. This test time is hard coded to prevent DST flakiness, see
   // crbug.com/1066576.
-  const base::Time systemTime =
+  const base::Time system_time =
       base::Time::FromDoubleT(1609459199).LocalMidnight() -
       base::TimeDelta::FromSeconds(1);
   // Display strings for given time deltas in seconds.
@@ -1056,9 +1056,9 @@ TEST_F(SafetyCheckHandlerTest, CheckParentRanDisplayString) {
   // Test that above time deltas produce the corresponding display strings.
   for (auto tuple : tuples) {
     const base::Time time =
-        systemTime - base::TimeDelta::FromSeconds(std::get<1>(tuple));
-    const base::string16 displayString = safety_check_->GetStringForParentRan(
-        time.ToJsTimeIgnoringNull(), systemTime);
-    EXPECT_EQ(base::UTF8ToUTF16(std::get<0>(tuple)), displayString);
+        system_time - base::TimeDelta::FromSeconds(std::get<1>(tuple));
+    const base::string16 display_string =
+        safety_check_->GetStringForParentRan(time, system_time);
+    EXPECT_EQ(base::UTF8ToUTF16(std::get<0>(tuple)), display_string);
   }
 }

chrome/test/data/webui/settings/safety_check_page_test.js

@@ -177,10 +177,7 @@ suite('SafetyCheckUiTests', function() {
     assertTrue(page.$$('#safetyCheckCollapse').opened);
 
     // Ensure the automatic browser proxy calls are started.
-    const timestamp =
-        await safetyCheckBrowserProxy.whenCalled('getParentRanDisplayString');
-    assertTrue(Number.isInteger(timestamp));
-    assertTrue(timestamp >= 0);
+    return safetyCheckBrowserProxy.whenCalled('getParentRanDisplayString');
   });
 
   test('HappinessTrackingSurveysTest', function() {