Make a network request from BulkLeakCheckImpl.

Bug: 1049185
Change-Id: Ia34aad31bf40f5ba5dfbfa52c41982666b3335d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066791
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744053}

components/password_manager/core/browser/leak_detection/BUILD.gn

@@ -67,6 +67,8 @@ jumbo_source_set("test_support") {
     "mock_leak_detection_check_factory.h",
     "mock_leak_detection_delegate.cc",
     "mock_leak_detection_delegate.h",
+    "mock_leak_detection_request_factory.cc",
+    "mock_leak_detection_request_factory.h",
   ]
   deps = [
     ":leak_detection",

components/password_manager/core/browser/leak_detection/authenticated_leak_check_unittest.cc

@@ -13,6 +13,7 @@
 #include "components/password_manager/core/browser/leak_detection/leak_detection_request_factory.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_request_utils.h"
 #include "components/password_manager/core/browser/leak_detection/mock_leak_detection_delegate.h"
+#include "components/password_manager/core/browser/leak_detection/mock_leak_detection_request_factory.h"
 #include "components/password_manager/core/browser/leak_detection/single_lookup_response.h"
 #include "components/signin/public/identity_manager/identity_test_environment.h"
 #include "crypto/sha2.h"
@@ -39,16 +40,6 @@ constexpr char kExampleCom[] = "https://example.com";
 const int64_t kMockElapsedTime =
     base::ScopedMockElapsedTimersForTest::kMockElapsedTime.InMilliseconds();
 
-class MockLeakDetectionRequest : public LeakDetectionRequestInterface {
- public:
-  // LeakDetectionRequestInterface:
-  MOCK_METHOD(void, LookupSingleLeak,
-              (network::mojom::URLLoaderFactory*,
-               const std::string&,
-               LookupSingleLeakPayload,
-               LookupSingleLeakCallback), (override));
-};
-
 struct TestLeakDetectionRequest : public LeakDetectionRequestInterface {
   ~TestLeakDetectionRequest() override = default;
   // LeakDetectionRequestInterface:
@@ -64,13 +55,6 @@ struct TestLeakDetectionRequest : public LeakDetectionRequestInterface {
   LookupSingleLeakCallback callback_;
 };
 
-class MockLeakDetectionRequestFactory : public LeakDetectionRequestFactory {
- public:
-  // LeakDetectionRequestFactory:
-  MOCK_CONST_METHOD0(CreateNetworkRequest,
-                     std::unique_ptr<LeakDetectionRequestInterface>());
-};
-
 // Helper struct for making a fake network request.
 struct PayloadAndCallback {
   std::string payload;

components/password_manager/core/browser/leak_detection/bulk_leak_check_impl.cc

@@ -12,6 +12,7 @@
 #include "components/password_manager/core/browser/leak_detection/encryption_utils.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_delegate_interface.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_request_utils.h"
+#include "components/password_manager/core/browser/leak_detection/single_lookup_response.h"
 #include "components/signin/public/identity_manager/access_token_fetcher.h"
 #include "components/signin/public/identity_manager/access_token_info.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
@@ -27,7 +28,7 @@ std::unique_ptr<BulkLeakCheckImpl::CredentialHolder> RemoveFromQueue(
                          [weak_holder](const auto& element) {
                            return element.get() == weak_holder;
                          });
-  DCHECK(it != queue->end());
+  CHECK(it != queue->end());
   std::unique_ptr<BulkLeakCheckImpl::CredentialHolder> holder = std::move(*it);
   queue->erase(it);
   return holder;
@@ -51,6 +52,9 @@ struct BulkLeakCheckImpl::CredentialHolder {
 
   // Request for the needed access token.
   std::unique_ptr<signin::AccessTokenFetcher> token_fetcher;
+
+  // Network request for the API call.
+  std::unique_ptr<LeakDetectionRequestInterface> network_request_;
 };
 
 LeakCheckCredential::LeakCheckCredential(base::string16 username,
@@ -138,7 +142,25 @@ void BulkLeakCheckImpl::OnTokenReady(
     // |this| can be destroyed here.
     return;
   }
-  // TODO(crbug.com/1049185): make a network request.
+
+  holder->token_fetcher.reset();
+  holder->network_request_ = network_request_factory_->CreateNetworkRequest();
+  holder->network_request_->LookupSingleLeak(
+      url_loader_factory_.get(), access_token_info.token,
+      std::move(holder->payload),
+      base::BindOnce(&BulkLeakCheckImpl::OnLookupLeakResponse,
+                     weak_ptr_factory_.GetWeakPtr(), holder.get()));
+  waiting_response_.push_back(std::move(holder));
+}
+
+void BulkLeakCheckImpl::OnLookupLeakResponse(
+    CredentialHolder* weak_holder,
+    std::unique_ptr<SingleLookupResponse> response) {
+  std::unique_ptr<CredentialHolder> holder =
+      RemoveFromQueue(weak_holder, &waiting_response_);
+
+  holder->network_request_.reset();
+  // TODO(crbug.com/1049185): decrypt the stuff.
 }
 
 }  // namespace password_manager
\ No newline at end of file

components/password_manager/core/browser/leak_detection/bulk_leak_check_impl.h

@@ -12,6 +12,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
+#include "components/password_manager/core/browser/leak_detection/leak_detection_request_factory.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_request_utils.h"
 
 namespace network {
@@ -25,6 +26,7 @@ class IdentityManager;
 namespace password_manager {
 
 class BulkLeakCheckDelegateInterface;
+struct SingleLookupResponse;
 
 // Implementation of the bulk leak check.
 // Every credential in the list is processed consequitively:
@@ -48,6 +50,13 @@ class BulkLeakCheckImpl : public BulkLeakCheck {
   void CheckCredentials(std::vector<LeakCheckCredential> credentials) override;
   size_t GetPendingChecksCount() const override;
 
+#if defined(UNIT_TEST)
+  void set_network_factory(
+      std::unique_ptr<LeakDetectionRequestFactory> factory) {
+    network_request_factory_ = std::move(factory);
+  }
+#endif  // defined(UNIT_TEST)
+
  private:
   // Called when a payload for one credential is ready.
   void OnPayloadReady(CredentialHolder* weak_holder,
@@ -58,6 +67,10 @@ class BulkLeakCheckImpl : public BulkLeakCheck {
                     GoogleServiceAuthError error,
                     signin::AccessTokenInfo access_token_info);
 
+  // Called when the server replied with something.
+  void OnLookupLeakResponse(CredentialHolder* weak_holder,
+                            std::unique_ptr<SingleLookupResponse> response);
+
   // Delegate for the instance. Should outlive |this|.
   BulkLeakCheckDelegateInterface* const delegate_;
 
@@ -68,6 +81,10 @@ class BulkLeakCheckImpl : public BulkLeakCheck {
   // endpoint.
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
+  // A factory for creating network requests.
+  std::unique_ptr<LeakDetectionRequestFactory> network_request_factory_ =
+      std::make_unique<LeakDetectionRequestFactory>();
+
   // The key used to encrypt/decrypt all the payloads for all the credentials.
   // Creating it once saves CPU time.
   const std::string encryption_key_;
@@ -81,6 +98,9 @@ class BulkLeakCheckImpl : public BulkLeakCheck {
   // the background thread.
   base::circular_deque<std::unique_ptr<CredentialHolder>> waiting_token_;
 
+  // The queue of the requests waiting for the server reply.
+  base::circular_deque<std::unique_ptr<CredentialHolder>> waiting_response_;
+
   // Task runner for preparing the payload. It takes a lot of memory. Therefore,
   // those tasks aren't parallelized.
   scoped_refptr<base::SequencedTaskRunner> payload_task_runner_;

components/password_manager/core/browser/leak_detection/bulk_leak_check_impl_unittest.cc

@@ -8,6 +8,7 @@
 #include "base/test/task_environment.h"
 #include "components/password_manager/core/browser/leak_detection/leak_detection_delegate_interface.h"
 #include "components/password_manager/core/browser/leak_detection/mock_leak_detection_delegate.h"
+#include "components/password_manager/core/browser/leak_detection/mock_leak_detection_request_factory.h"
 #include "components/signin/public/identity_manager/identity_test_environment.h"
 #include "services/network/test/test_shared_url_loader_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -16,6 +17,14 @@
 namespace password_manager {
 namespace {
 
+using ::testing::_;
+using ::testing::AllOf;
+using ::testing::ByMove;
+using ::testing::ElementsAre;
+using ::testing::Field;
+using ::testing::Return;
+
+constexpr char kAccessToken[] = "access_token";
 constexpr char kTestEmail[] = "user@gmail.com";
 
 LeakCheckCredential TestCredential(base::StringPiece username) {
@@ -29,7 +38,12 @@ class BulkLeakCheckTest : public testing::Test {
       : bulk_check_(
             &delegate_,
             identity_test_env_.identity_manager(),
-            base::MakeRefCounted<network::TestSharedURLLoaderFactory>()) {}
+            base::MakeRefCounted<network::TestSharedURLLoaderFactory>()) {
+    auto mock_request_factory = std::make_unique<
+        ::testing::StrictMock<MockLeakDetectionRequestFactory>>();
+    request_factory_ = mock_request_factory.get();
+    bulk_check_.set_network_factory(std::move(mock_request_factory));
+  }
 
   void RunUntilIdle() { task_env_.RunUntilIdle(); }
 
@@ -37,12 +51,16 @@ class BulkLeakCheckTest : public testing::Test {
     return identity_test_env_;
   }
   MockBulkLeakCheckDelegateInterface& delegate() { return delegate_; }
+  MockLeakDetectionRequestFactory* request_factory() {
+    return request_factory_;
+  }
   BulkLeakCheckImpl& bulk_check() { return bulk_check_; }
 
  private:
   base::test::TaskEnvironment task_env_;
   signin::IdentityTestEnvironment identity_test_env_;
   ::testing::StrictMock<MockBulkLeakCheckDelegateInterface> delegate_;
+  MockLeakDetectionRequestFactory* request_factory_;
   BulkLeakCheckImpl bulk_check_;
 };
 
@@ -81,7 +99,6 @@ TEST_F(BulkLeakCheckTest, CheckCredentialsAccessTokenAuthError) {
   identity_test_env().SetCookieAccounts({{info.email, info.gaia}});
   identity_test_env().SetRefreshTokenForAccount(info.account_id);
 
-  EXPECT_CALL(delegate(), OnFinishedCredential).Times(0);
   EXPECT_CALL(delegate(), OnError(LeakDetectionError::kTokenRequestFailure));
 
   std::vector<LeakCheckCredential> credentials;
@@ -98,7 +115,6 @@ TEST_F(BulkLeakCheckTest, CheckCredentialsAccessTokenNetError) {
   identity_test_env().SetCookieAccounts({{info.email, info.gaia}});
   identity_test_env().SetRefreshTokenForAccount(info.account_id);
 
-  EXPECT_CALL(delegate(), OnFinishedCredential).Times(0);
   EXPECT_CALL(delegate(), OnError(LeakDetectionError::kNetworkError));
 
   std::vector<LeakCheckCredential> credentials;
@@ -108,5 +124,52 @@ TEST_F(BulkLeakCheckTest, CheckCredentialsAccessTokenNetError) {
       GoogleServiceAuthError::FromConnectionError(net::ERR_TIMED_OUT));
 }
 
+TEST_F(BulkLeakCheckTest, CheckCredentialsAccessDoesNetworkRequest) {
+  AccountInfo info = identity_test_env().MakeAccountAvailable(kTestEmail);
+  identity_test_env().SetCookieAccounts({{info.email, info.gaia}});
+  identity_test_env().SetRefreshTokenForAccount(info.account_id);
+
+  std::vector<LeakCheckCredential> credentials;
+  credentials.push_back(TestCredential("USERNAME@gmail.com"));
+  bulk_check().CheckCredentials(std::move(credentials));
+
+  auto network_request = std::make_unique<MockLeakDetectionRequest>();
+  EXPECT_CALL(*network_request,
+              LookupSingleLeak(
+                  _, kAccessToken,
+                  AllOf(Field(&LookupSingleLeakPayload::username_hash_prefix,
+                              ElementsAre(-67, 116, -87)),
+                        Field(&LookupSingleLeakPayload::encrypted_payload,
+                              testing::Ne(""))),
+                  _));
+  EXPECT_CALL(*request_factory(), CreateNetworkRequest)
+      .WillOnce(Return(ByMove(std::move(network_request))));
+  identity_test_env().WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
+      kAccessToken, base::Time::Max());
+}
+
+TEST_F(BulkLeakCheckTest, CheckCredentialsMultipleNetworkRequests) {
+  AccountInfo info = identity_test_env().MakeAccountAvailable(kTestEmail);
+  identity_test_env().SetCookieAccounts({{info.email, info.gaia}});
+  identity_test_env().SetRefreshTokenForAccount(info.account_id);
+
+  std::vector<LeakCheckCredential> credentials;
+  credentials.push_back(TestCredential("user1"));
+  credentials.push_back(TestCredential("user2"));
+  bulk_check().CheckCredentials(std::move(credentials));
+
+  auto network_request1 = std::make_unique<MockLeakDetectionRequest>();
+  auto network_request2 = std::make_unique<MockLeakDetectionRequest>();
+  EXPECT_CALL(*network_request1, LookupSingleLeak(_, kAccessToken, _, _));
+  EXPECT_CALL(*network_request2, LookupSingleLeak(_, kAccessToken, _, _));
+  EXPECT_CALL(*request_factory(), CreateNetworkRequest)
+      .WillOnce(Return(ByMove(std::move(network_request1))))
+      .WillOnce(Return(ByMove(std::move(network_request2))));
+  identity_test_env().WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
+      kAccessToken, base::Time::Max());
+  identity_test_env().WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
+      kAccessToken, base::Time::Max());
+}
+
 }  // namespace
 }  // namespace password_manager

components/password_manager/core/browser/leak_detection/mock_leak_detection_request_factory.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/password_manager/core/browser/leak_detection/mock_leak_detection_request_factory.h"
+
+namespace password_manager {
+
+MockLeakDetectionRequest::MockLeakDetectionRequest() = default;
+MockLeakDetectionRequest::~MockLeakDetectionRequest() = default;
+
+MockLeakDetectionRequestFactory::MockLeakDetectionRequestFactory() = default;
+MockLeakDetectionRequestFactory::~MockLeakDetectionRequestFactory() = default;
+
+}  // namespace password_manager

components/password_manager/core/browser/leak_detection/mock_leak_detection_request_factory.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LEAK_DETECTION_MOCK_LEAK_DETECTION_REQUEST_FACTORY_H_
+#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LEAK_DETECTION_MOCK_LEAK_DETECTION_REQUEST_FACTORY_H_
+
+#include "components/password_manager/core/browser/leak_detection/leak_detection_request_factory.h"
+#include "components/password_manager/core/browser/leak_detection/leak_detection_request_utils.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace password_manager {
+
+class MockLeakDetectionRequest : public LeakDetectionRequestInterface {
+ public:
+  MockLeakDetectionRequest();
+  ~MockLeakDetectionRequest() override;
+
+  // LeakDetectionRequestInterface:
+  MOCK_METHOD(void,
+              LookupSingleLeak,
+              (network::mojom::URLLoaderFactory*,
+               const std::string&,
+               LookupSingleLeakPayload,
+               LookupSingleLeakCallback),
+              (override));
+};
+
+class MockLeakDetectionRequestFactory : public LeakDetectionRequestFactory {
+ public:
+  MockLeakDetectionRequestFactory();
+  ~MockLeakDetectionRequestFactory() override;
+
+  // LeakDetectionRequestFactory:
+  MOCK_METHOD(std::unique_ptr<LeakDetectionRequestInterface>,
+              CreateNetworkRequest,
+              (),
+              (const override));
+};
+
+}  // namespace password_manager
+
+#endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LEAK_DETECTION_MOCK_LEAK_DETECTION_REQUEST_FACTORY_H_