Add Lookup Logic for ChangePasswordUrlService

Add lookup logic for ChangePasswordUrlService, implement Callback for the gstatic fetch and test the url lookup logic.

Bug: 1086141
Change-Id: I6992cc891d2e43f9e0091f82c53b624f8f277ec2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2285125
Commit-Queue: Jonathan Mengedoht <mengedoht@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791758}

components/password_manager/core/browser/change_password_url_service_impl.cc

@@ -6,6 +6,8 @@
 
 #include "base/callback.h"
 #include "base/containers/flat_map.h"
+#include "base/json/json_reader.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "url/gurl.h"
 #include "url/origin.h"
@@ -30,15 +32,32 @@ void ChangePasswordUrlServiceImpl::Initialize() {
 void ChangePasswordUrlServiceImpl::GetChangePasswordUrl(
     const url::Origin& origin,
     UrlCallback callback) {
-  // TODO(crbug.com/1086141): call callback if response available, otherwise
-  // save callback.
-  url_callbacks_.emplace_back(std::make_pair(origin, std::move(callback)));
+  DCHECK(started_fetching_) << "Call Initialize() before.";
+  if (fetch_complete_) {
+    std::move(callback).Run(ChangePasswordUrlFor(origin));
+  } else {
+    url_callbacks_.emplace_back(origin, std::move(callback));
+  }
 }
 
 void ChangePasswordUrlServiceImpl::OnFetchComplete(
     std::unique_ptr<std::string> response_body) {
-  // TODO(crbug.com/1086141): handle response and convert JSON.
-  change_password_url_map_ = {};
+  fetch_complete_ = true;
+  // TODO(crbug.com/1086141): Log error codes in histograms.
+  if (response_body) {
+    base::Optional<base::Value> data = base::JSONReader::Read(*response_body);
+    if (data && data->is_dict()) {
+      for (auto&& url_pair : data->DictItems()) {
+        if (url_pair.second.is_string()) {
+          GURL url = GURL(url_pair.second.GetString());
+          if (url.is_valid()) {
+            change_password_url_map_.try_emplace(change_password_url_map_.end(),
+                                                 url_pair.first, url);
+          }
+        }
+      }
+    }
+  }
 
   for (auto& url_callback : std::exchange(url_callbacks_, {})) {
     GURL url = ChangePasswordUrlFor(url_callback.first);
@@ -48,9 +67,14 @@ void ChangePasswordUrlServiceImpl::OnFetchComplete(
 
 GURL ChangePasswordUrlServiceImpl::ChangePasswordUrlFor(
     const url::Origin& origin) {
-  // TODO(crbug.com/1086141): lookup url override from map.
-
-  // Fallback if no change-password url available or request failed
+  std::string domain_and_registry =
+      net::registry_controlled_domains::GetDomainAndRegistry(
+          origin, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
+  auto it = change_password_url_map_.find(domain_and_registry);
+  if (it != change_password_url_map_.end()) {
+    return it->second;
+  }
+  // Fallback if no valid change-password url available or request failed.
   return origin.GetURL();
 }
 

components/password_manager/core/browser/change_password_url_service_impl.h

@@ -43,13 +43,14 @@ class ChangePasswordUrlServiceImpl
  private:
   // Callback for the the request to gstatic.
   void OnFetchComplete(std::unique_ptr<std::string> response_body);
-  // Retrieves the url override from the |change_password_url_map| for a given
-  // origin. It uses eTLD+1 for the lookup but also checks if overrides for
-  // eTLD+1+N exist.
+  // Retrieves the url override from the |change_password_url_map_| for a given
+  // origin using eTLD+1. The origin is returned when no override is available.
   GURL ChangePasswordUrlFor(const url::Origin& origin);
 
   // Stores if the request is already started to only fetch once.
   bool started_fetching_ = false;
+  // True when the gstatic response arrived.
+  bool fetch_complete_ = false;
   // Stores the JSON result for the url overrides.
   base::flat_map<std::string, GURL> change_password_url_map_;
   // Stores the callbacks that are waiting for the request to finish.

components/password_manager/core/browser/change_password_url_service_impl_unittest.cc

@@ -4,6 +4,8 @@
 #include "components/password_manager/core/browser/change_password_url_service_impl.h"
 
 #include "base/logging.h"
+#include "base/test/mock_callback.h"
+#include "base/test/task_environment.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/test/test_shared_url_loader_factory.h"
 #include "services/network/test/test_url_loader_factory.h"
@@ -12,11 +14,36 @@
 namespace password_manager {
 
 class ChangePasswordUrlServiceTest : public testing::Test {
+ public:
+  ChangePasswordUrlServiceTest() = default;
+
+  // Test the bevahiour for a given |url| and compares the result to the given
+  // |expected_url| in the callback.
+  void TestOverride(GURL url, GURL expected_url);
+
  private:
   network::TestURLLoaderFactory test_url_loader_factory_;
   scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_ =
       base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
           &test_url_loader_factory_);
+  ChangePasswordUrlServiceImpl change_password_url_service_{
+      test_shared_loader_factory_};
 };
 
+void ChangePasswordUrlServiceTest::TestOverride(GURL url, GURL expected_url) {
+  change_password_url_service_.Initialize();
+
+  base::MockCallback<password_manager::ChangePasswordUrlService::UrlCallback>
+      callback;
+  EXPECT_CALL(callback, Run(expected_url));
+  change_password_url_service_.GetChangePasswordUrl(url::Origin::Create(url),
+                                                    callback.Get());
+}
+
+TEST_F(ChangePasswordUrlServiceTest, eTLDLookup) {
+  // TODO(crbug.com/1086141): If possible mock eTLD registry to ensure sites are
+  // listed.
+  TestOverride(GURL("https://google.com/foo"), GURL("https://google.com/"));
+}
+
 }  // namespace password_manager