[Passwords] Use OneShotTimer to wait for the result of Affiliation Service...

[Passwords] Use OneShotTimer to wait for the result of Affiliation Service prefetch before the redirect action.

This change implements OneShotTimer that waits for the result of Affiliation Service prefetch.
The result or timeout is followed by redirection to change password url or origin.
The facet to be requested in Affiliation Service is now created from SchemeHostPort type which removes the relative path.

Bug: 1108279
Change-Id: I2c47dc3b0b70bc9768b967441762b80a965386cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401165
Commit-Queue: Nela Kaczmarek <nelakaczmarek@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810520}

components/password_manager/core/browser/site_affiliation/affiliation_service_impl.cc

@@ -143,8 +143,9 @@ std::vector<FacetURI> AffiliationServiceImpl::ConvertMissingURLsToFacets(
     if (url.is_valid()) {
       url::SchemeHostPort scheme_host_port(url);
       if (!base::Contains(change_password_urls_, scheme_host_port)) {
+        facets.push_back(
+            FacetURI::FromCanonicalSpec(scheme_host_port.Serialize()));
         requested_tuple_origins_.push_back(std::move(scheme_host_port));
-        facets.push_back(FacetURI::FromCanonicalSpec(url.spec()));
       }
     }
   }

components/password_manager/core/browser/site_affiliation/affiliation_service_impl_unittest.cc

@@ -42,7 +42,8 @@ constexpr char k5ExampleURL[] = "https://5.example.com";
 std::vector<FacetURI> ToFacetsURIs(const std::vector<GURL>& urls) {
   std::vector<FacetURI> facet_URIs;
   for (const auto& url : urls) {
-    facet_URIs.push_back(FacetURI::FromCanonicalSpec(url.spec()));
+    facet_URIs.push_back(
+        FacetURI::FromCanonicalSpec(url::SchemeHostPort(url).Serialize()));
   }
   return facet_URIs;
 }

components/password_manager/core/browser/well_known_change_password_state.cc

@@ -9,6 +9,7 @@
 #include "base/optional.h"
 #include "components/password_manager/core/browser/site_affiliation/affiliation_service.h"
 #include "components/password_manager/core/browser/well_known_change_password_util.h"
+#include "components/password_manager/core/common/password_manager_features.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
@@ -69,6 +70,8 @@ CreateResourceRequestToWellKnownNonExistingResourceFor(
 }
 }  // namespace
 
+constexpr base::TimeDelta WellKnownChangePasswordState::kPrefetchTimeout;
+
 WellKnownChangePasswordState::WellKnownChangePasswordState(
     WellKnownChangePasswordStateDelegate* delegate)
     : delegate_(delegate) {}
@@ -95,6 +98,8 @@ void WellKnownChangePasswordState::FetchNonExistingResource(
 void WellKnownChangePasswordState::PrefetchChangePasswordURLs(
     AffiliationService* affiliation_service,
     const std::vector<GURL>& urls) {
+  prefetch_timer_.Start(FROM_HERE, kPrefetchTimeout, this,
+                        &WellKnownChangePasswordState::ContinueProcessing);
   affiliation_service->PrefetchChangePasswordURLs(
       urls,
       base::BindOnce(
@@ -115,15 +120,21 @@ void WellKnownChangePasswordState::FetchNonExistingResourceCallback(
   ContinueProcessing();
 }
 
-void WellKnownChangePasswordState::PrefetchChangePasswordURLsCallback() {}
+void WellKnownChangePasswordState::PrefetchChangePasswordURLsCallback() {
+  if (prefetch_timer_.IsRunning()) {
+    prefetch_timer_.Stop();
+    ContinueProcessing();
+  }
+}
 
 void WellKnownChangePasswordState::ContinueProcessing() {
-  // TODO: Implement timer and insert condition (prefetch_completed_ ||
-  // prefetch_tiemout_) if ChangePasswordAffiliationInfo flag is enabled.
-  if (!BothRequestsFinished()) {
-    return;
+  if (BothRequestsFinished()) {
+    bool is_well_known_supported = SupportsWellKnownChangePasswordUrl();
+    // Don't wait for change password URL from Affiliation Service if
+    // .well-known/change-password is supported.
+    if (is_well_known_supported || !prefetch_timer_.IsRunning())
+      delegate_->OnProcessingFinished(is_well_known_supported);
   }
-  delegate_->OnProcessingFinished(SupportsChangePasswordUrl());
 }
 
 bool WellKnownChangePasswordState::BothRequestsFinished() const {
@@ -131,7 +142,7 @@ bool WellKnownChangePasswordState::BothRequestsFinished() const {
          change_password_response_code_ != 0;
 }
 
-bool WellKnownChangePasswordState::SupportsChangePasswordUrl() const {
+bool WellKnownChangePasswordState::SupportsWellKnownChangePasswordUrl() const {
   DCHECK(BothRequestsFinished());
   return 200 <= change_password_response_code_ &&
          change_password_response_code_ < 300 &&

components/password_manager/core/browser/well_known_change_password_state.h

@@ -10,6 +10,7 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/timer/timer.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "url/gurl.h"
@@ -33,6 +34,12 @@ class WellKnownChangePasswordStateDelegate {
 // Processes if .well-known/change-password is supported by a site.
 class WellKnownChangePasswordState {
  public:
+  // Time to wait for the callback from AffiliationService before finishing
+  // processing. A callback signals the prefetch action was completed regardless
+  // if the response arrived or not.
+  static constexpr base::TimeDelta kPrefetchTimeout =
+      base::TimeDelta::FromSeconds(2);
+
   explicit WellKnownChangePasswordState(
       password_manager::WellKnownChangePasswordStateDelegate* delegate);
   ~WellKnownChangePasswordState();
@@ -60,17 +67,19 @@ class WellKnownChangePasswordState {
   // Callback for the request to the Affiliation Service prefetch.
   void PrefetchChangePasswordURLsCallback();
   // Function is called when both requests are finished. Decides to continue or
-  // redirect to homepage.
+  // redirect to change password URL or homepage.
   void ContinueProcessing();
   // Checks if both requests are finished.
   bool BothRequestsFinished() const;
-  // Checks the status codes and returns if change password is supported.
-  bool SupportsChangePasswordUrl() const;
+  // Checks the status codes and returns if .well-known/change-password is
+  // supported.
+  bool SupportsWellKnownChangePasswordUrl() const;
 
   WellKnownChangePasswordStateDelegate* delegate_ = nullptr;
   int non_existing_resource_response_code_ = 0;
   int change_password_response_code_ = 0;
   std::unique_ptr<network::SimpleURLLoader> url_loader_;
+  base::OneShotTimer prefetch_timer_;
   base::WeakPtrFactory<WellKnownChangePasswordState> weak_factory_{this};
 };
 

components/password_manager/core/browser/well_known_change_password_state_unittest.cc

@@ -6,7 +6,10 @@
 
 #include "base/task/post_task.h"
 #include "base/test/task_environment.h"
+#include "base/timer/mock_timer.h"
+#include "components/password_manager/core/browser/site_affiliation/affiliation_service_impl.h"
 #include "components/password_manager/core/browser/well_known_change_password_util.h"
+#include "components/sync/driver/test_sync_service.h"
 #include "net/base/isolation_info.h"
 #include "net/base/load_flags.h"
 #include "services/network/public/cpp/resource_request.h"
@@ -38,6 +41,19 @@ class MockWellKnownChangePasswordStateDelegate
   MOCK_METHOD(void, OnProcessingFinished, (bool), (override));
 };
 
+class MockAffiliationService : public AffiliationService {
+ public:
+  MockAffiliationService() = default;
+  ~MockAffiliationService() override = default;
+
+  MOCK_METHOD(void,
+              PrefetchChangePasswordURLs,
+              (const std::vector<GURL>&, base::OnceClosure),
+              (override));
+  MOCK_METHOD(void, Clear, (), (override));
+  MOCK_METHOD(GURL, GetChangePasswordURL, (const GURL&), (override, const));
+};
+
 class WellKnownChangePasswordStateTest
     : public testing::Test,
       public testing::WithParamInterface<ResponseDelayParams> {
@@ -59,16 +75,25 @@ class WellKnownChangePasswordStateTest
   void RespondeToChangePasswordRequest(net::HttpStatusCode status, int delay);
 
   MockWellKnownChangePasswordStateDelegate* delegate() { return &delegate_; }
+  WellKnownChangePasswordState* state() { return &state_; }
+  network::SharedURLLoaderFactory* test_shared_loader_factory() {
+    return test_shared_loader_factory_.get();
+  }
 
   // Wait until all PostTasks are processed.
   void FastForwardPostTasks() {
     task_environment_.FastForwardUntilNoTasksRemain();
   }
 
+  // Fast forward by certain amount of time.
+  void FastForwardBy(base::TimeDelta delta) {
+    task_environment_.FastForwardBy(delta);
+  }
+
  private:
   base::test::SingleThreadTaskEnvironment task_environment_{
       base::test::TaskEnvironment::TimeSource::MOCK_TIME};
-  MockWellKnownChangePasswordStateDelegate delegate_;
+  testing::StrictMock<MockWellKnownChangePasswordStateDelegate> delegate_;
   WellKnownChangePasswordState state_{&delegate_};
   network::ResourceRequest::TrustedParams trusted_params_;
   network::TestURLLoaderFactory test_url_loader_factory_;
@@ -167,6 +192,62 @@ TEST_P(WellKnownChangePasswordStateTest, NoSupport_Redirect) {
   FastForwardPostTasks();
 }
 
+TEST_P(WellKnownChangePasswordStateTest,
+       NoAwaitForPrefetchResultIfWellKnownChangePasswordSupported) {
+  MockAffiliationService mock_affiliation_service;
+  EXPECT_CALL(mock_affiliation_service, PrefetchChangePasswordURLs);
+  state()->PrefetchChangePasswordURLs(&mock_affiliation_service, {});
+
+  EXPECT_CALL(*delegate(), OnProcessingFinished(true));
+
+  ResponseDelayParams params = GetParam();
+  RespondeToChangePasswordRequest(net::HTTP_OK, params.change_password_delay);
+  RespondeToNonExistingRequest(net::HTTP_NOT_FOUND, params.not_exist_delay);
+
+  // FastForwardBy makes sure the prefech timeout is not reached.
+  const int64_t ms_to_forward =
+      std::max(params.change_password_delay, params.not_exist_delay) + 1;
+  FastForwardBy(base::TimeDelta::FromMilliseconds(ms_to_forward));
+}
+
+TEST_P(WellKnownChangePasswordStateTest, TimeoutTriggersOnProcessingFinished) {
+  MockAffiliationService mock_affiliation_service;
+  EXPECT_CALL(mock_affiliation_service, PrefetchChangePasswordURLs);
+  state()->PrefetchChangePasswordURLs(&mock_affiliation_service, {});
+
+  ResponseDelayParams params = GetParam();
+  RespondeToChangePasswordRequest(net::HTTP_NOT_FOUND,
+                                  params.change_password_delay);
+  RespondeToNonExistingRequest(net::HTTP_NOT_FOUND, params.not_exist_delay);
+  const int64_t ms_to_forward =
+      std::max(params.change_password_delay, params.not_exist_delay) + 1;
+  FastForwardBy(base::TimeDelta::FromMilliseconds(ms_to_forward));
+
+  EXPECT_CALL(*delegate(), OnProcessingFinished(false));
+  FastForwardBy(WellKnownChangePasswordState::kPrefetchTimeout);
+}
+
+TEST_P(WellKnownChangePasswordStateTest,
+       PrefetchCallbackTriggersOnProcessingFinished) {
+  syncer::TestSyncService test_sync_service;
+  AffiliationServiceImpl affiliation_service(&test_sync_service,
+                                             test_shared_loader_factory());
+  state()->PrefetchChangePasswordURLs(&affiliation_service, {});
+
+  ResponseDelayParams params = GetParam();
+  RespondeToChangePasswordRequest(net::HTTP_NOT_FOUND,
+                                  params.change_password_delay);
+  RespondeToNonExistingRequest(net::HTTP_NOT_FOUND, params.not_exist_delay);
+  const int64_t ms_to_forward =
+      std::max(params.change_password_delay, params.not_exist_delay) + 1;
+  FastForwardBy(base::TimeDelta::FromMilliseconds(ms_to_forward));
+
+  EXPECT_CALL(*delegate(), OnProcessingFinished(false));
+  static_cast<AffiliationFetcherDelegate*>(&affiliation_service)
+      ->OnFetchSucceeded(
+          std::make_unique<AffiliationFetcherDelegate::Result>());
+}
+
 constexpr ResponseDelayParams kDelayParams[] = {{0, 1}, {1, 0}};
 
 INSTANTIATE_TEST_SUITE_P(All,