[PwdCheckAndroid] Save timestamp of last successful check

Bug: 1102025,1092444
Change-Id: Ic703c3b3cb730d7ce1d27507f9d1fb2232a9e60e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353257
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798618}

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckBridge.java

@@ -106,9 +106,8 @@ class PasswordCheckBridge {
     /**
      * @return The timestamp of the last completed check.
      */
-    long getCheckTimestamp() {
-        // TODO(crbug.com/1102025): Add method to retrieve the timestamp.
-        return 0L;
+    long getLastCheckTimestamp() {
+        return PasswordCheckBridgeJni.get().getLastCheckTimestamp(mNativePasswordCheckBridge);
     }
 
     /**
@@ -164,6 +163,7 @@ class PasswordCheckBridge {
         long create(PasswordCheckBridge passwordCheckBridge);
         void startCheck(long nativePasswordCheckBridge);
         void stopCheck(long nativePasswordCheckBridge);
+        long getLastCheckTimestamp(long nativePasswordCheckBridge);
         int getCompromisedCredentialsCount(long nativePasswordCheckBridge);
         int getSavedPasswordsCount(long nativePasswordCheckBridge);
         void getCompromisedCredentials(

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckImpl.java

@@ -100,8 +100,8 @@ class PasswordCheckImpl implements PasswordCheck, PasswordCheckObserver {
     }
 
     @Override
-    public long getCheckTimestamp() {
-        return mPasswordCheckBridge.getCheckTimestamp();
+    public long getLastCheckTimestamp() {
+        return mPasswordCheckBridge.getLastCheckTimestamp();
     }
 
     @Override

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckMediator.java

@@ -108,7 +108,7 @@ class PasswordCheckMediator
         Integer compromisedCredentialCount = null;
         if (status == PasswordCheckUIStatus.IDLE) {
             compromisedCredentialCount = getPasswordCheck().getCompromisedCredentialsCount();
-            checkTimestamp = getPasswordCheck().getCheckTimestamp();
+            checkTimestamp = getPasswordCheck().getLastCheckTimestamp();
         }
         header.set(CHECK_TIMESTAMP, checkTimestamp);
         header.set(COMPROMISED_CREDENTIALS_COUNT, compromisedCredentialCount);

chrome/browser/password_check/android/java/src/org/chromium/chrome/browser/password_check/PasswordCheck.java

@@ -69,7 +69,7 @@ public interface PasswordCheck extends PasswordCheckComponentUi.Delegate {
     /**
      * @return The timestamp of the last completed check.
      */
-    long getCheckTimestamp();
+    long getLastCheckTimestamp();
 
     /**
      * @return The last known status of the check.

chrome/browser/password_check/android/password_check_bridge.cc

@@ -51,6 +51,10 @@ void PasswordCheckBridge::StopCheck(JNIEnv* env) {
   check_manager_.StopCheck();
 }
 
+int64_t PasswordCheckBridge::GetLastCheckTimestamp(JNIEnv* env) {
+  return check_manager_.GetLastCheckTimestamp().ToJavaTime();
+}
+
 jint PasswordCheckBridge::GetCompromisedCredentialsCount(JNIEnv* env) {
   return check_manager_.GetCompromisedCredentialsCount();
 }

chrome/browser/password_check/android/password_check_bridge.h

@@ -28,6 +28,9 @@ class PasswordCheckBridge : public PasswordCheckManager::Observer {
   // Called by Java to stop the password check.
   void StopCheck(JNIEnv* env);
 
+  // Called by Java to retrieve the time when the last check finished.
+  int64_t GetLastCheckTimestamp(JNIEnv* env);
+
   // Called by Java to get the number of compromised credentials.
   jint GetCompromisedCredentialsCount(JNIEnv* env);
 

chrome/browser/password_check/android/password_check_manager.cc

@@ -16,6 +16,8 @@
 #include "components/password_manager/core/browser/password_manager_util.h"
 #include "components/password_manager/core/browser/ui/compromised_credentials_manager.h"
 #include "components/password_manager/core/common/password_manager_features.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/prefs/pref_service.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/sync/driver/profile_sync_service.h"
 #include "components/url_formatter/url_formatter.h"
@@ -92,6 +94,7 @@ void PasswordCheckManager::StartCheck() {
 
   // The request is being handled, so reset the boolean.
   was_start_requested_ = false;
+  is_check_running_ = true;
   bulk_leak_check_service_adapter_.StartBulkLeakCheck();
 }
 
@@ -99,6 +102,11 @@ void PasswordCheckManager::StopCheck() {
   bulk_leak_check_service_adapter_.StopBulkLeakCheck();
 }
 
+base::Time PasswordCheckManager::GetLastCheckTimestamp() {
+  return base::Time::FromDoubleT(profile_->GetPrefs()->GetDouble(
+      password_manager::prefs::kLastTimePasswordCheckCompleted));
+}
+
 int PasswordCheckManager::GetCompromisedCredentialsCount() const {
   return compromised_credentials_manager_.GetCompromisedCredentials().size();
 }
@@ -157,6 +165,17 @@ void PasswordCheckManager::OnCompromisedCredentialsChanged(
 }
 
 void PasswordCheckManager::OnStateChanged(State state) {
+  if (state == State::kIdle && is_check_running_) {
+    // Save the time at which the last successful check finished.
+    profile_->GetPrefs()->SetDouble(
+        password_manager::prefs::kLastTimePasswordCheckCompleted,
+        base::Time::Now().ToDoubleT());
+  }
+
+  if (state != State::kRunning) {
+    is_check_running_ = false;
+  }
+
   observer_->OnPasswordCheckStatusChanged(GetUIStatus(state));
 }
 

chrome/browser/password_check/android/password_check_manager.h

@@ -60,6 +60,9 @@ class PasswordCheckManager
   // Stops a running check.
   void StopCheck();
 
+  // Called by java to retireve the timestamp of the last password check.
+  base::Time GetLastCheckTimestamp();
+
   // Called by java to retrieve the number of compromised credentials. If the
   // credentials haven't been fetched yet, this will return 0.
   int GetCompromisedCredentialsCount() const;
@@ -156,6 +159,9 @@ class PasswordCheckManager
   // Whether the check start was requested.
   bool was_start_requested_ = false;
 
+  // Whether a check is currently running.
+  bool is_check_running_ = false;
+
   // A scoped observer for `saved_passwords_presenter_`.
   ScopedObserver<password_manager::SavedPasswordsPresenter,
                  password_manager::SavedPasswordsPresenter::Observer>

chrome/browser/password_check/android/password_check_manager_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind_test_util.h"
+#include "base/time/time.h"
 #include "chrome/browser/password_check/android/password_check_ui_status.h"
 #include "chrome/browser/password_manager/bulk_leak_check_service_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
@@ -19,8 +20,11 @@
 #include "components/password_manager/core/browser/bulk_leak_check_service.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/test_password_store.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/prefs/testing_pref_service.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/signin/public/identity_manager/identity_test_environment.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/test/browser_task_environment.h"
 #include "services/network/test/test_shared_url_loader_factory.h"
@@ -33,6 +37,7 @@ using password_manager::CompromisedCredentials;
 using password_manager::CompromiseType;
 using password_manager::PasswordCheckUIStatus;
 using password_manager::TestPasswordStore;
+using password_manager::prefs::kLastTimePasswordCheckCompleted;
 using testing::_;
 using testing::ElementsAre;
 using testing::Field;
@@ -41,6 +46,7 @@ using testing::NiceMock;
 
 using CompromisedCredentialForUI =
     PasswordCheckManager::CompromisedCredentialForUI;
+using State = password_manager::BulkLeakCheckService::State;
 
 namespace {
 
@@ -169,6 +175,7 @@ class PasswordCheckManagerTest : public testing::Test {
 
   void RunUntilIdle() { task_env_.RunUntilIdle(); }
 
+  BulkLeakCheckService* service() { return service_; }
   TestPasswordStore& store() { return *store_; }
 
  protected:
@@ -176,11 +183,10 @@ class PasswordCheckManagerTest : public testing::Test {
   std::unique_ptr<PasswordCheckManager> manager_;
 
  private:
-  content::BrowserTaskEnvironment task_env_{
-      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+  content::BrowserTaskEnvironment task_env_;
   signin::IdentityTestEnvironment identity_test_env_;
   TestingProfile profile_;
-  BulkLeakCheckService* bulk_leak_check_service_ =
+  BulkLeakCheckService* service_ =
       CreateAndUseBulkLeakCheckService(identity_test_env_.identity_manager(),
                                        &profile_);
   scoped_refptr<TestPasswordStore> store_ =
@@ -264,3 +270,29 @@ TEST_F(PasswordCheckManagerTest, CorrectlyCreatesUIStructForAppCredentials) {
               "com.example.app", base::nullopt, /*is_android_credential=*/true,
               /*has_script=*/false)));
 }
+
+TEST_F(PasswordCheckManagerTest, SetsTimestampOnSuccessfulCheck) {
+  InitializeManager();
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
+  RunUntilIdle();
+
+  // Pretend to start the check so that the manager thinks a check is running.
+  manager_->StartCheck();
+
+  // Change the state to idle to simulate a successful check finish.
+  service()->set_state_and_notify(State::kIdle);
+  EXPECT_NE(0.0, manager_->GetLastCheckTimestamp().ToDoubleT());
+}
+
+TEST_F(PasswordCheckManagerTest, DoesntRecordTimestampOfUnsuccessfulCheck) {
+  InitializeManager();
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
+  RunUntilIdle();
+
+  // Pretend to start the check so that the manager thinks a check is running.
+  manager_->StartCheck();
+
+  // Change the state to an error state to simulate a unsuccessful check finish.
+  service()->set_state_and_notify(State::kSignedOut);
+  EXPECT_EQ(0.0, manager_->GetLastCheckTimestamp().ToDoubleT());
+}