VLOG debug for password requirements

This CL adds some VLOG debug statements that help us identify problems in case
they arise during launch. They turned out to be useful while working on a finch
config. They won't replace debug data in chrome://password-manager-internals,
though. But that will be based on the serialization introduced here.

Bug: 846694
Change-Id: I20e8c1826cc8fa00860bfbeccaf832e93a0bb125
Reviewed-on: https://chromium-review.googlesource.com/1097086Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566547}

components/autofill/core/browser/BUILD.gn

@@ -118,6 +118,8 @@ static_library("browser") {
     "password_requirements_spec_fetcher.h",
     "password_requirements_spec_fetcher_impl.cc",
     "password_requirements_spec_fetcher_impl.h",
+    "password_requirements_spec_printer.cc",
+    "password_requirements_spec_printer.h",
     "payments/full_card_request.cc",
     "payments/full_card_request.h",
     "payments/payments_client.cc",

components/autofill/core/browser/password_requirements_spec_fetcher_impl.cc

@@ -9,6 +9,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
+#include "components/autofill/core/browser/password_requirements_spec_printer.h"
 #include "components/autofill/core/browser/proto/password_requirements.pb.h"
 #include "components/autofill/core/browser/proto/password_requirements_shard.pb.h"
 #include "net/base/load_flags.h"
@@ -92,6 +93,14 @@ void PasswordRequirementsSpecFetcherImpl::Fetch(
     GURL origin,
     FetchCallback callback) {
   DCHECK(origin.is_valid());
+  VLOG(1) << "Fetching password requirements spec for " << origin;
+
+  if (!url_loader_factory_) {
+    VLOG(1) << "No url_logger_factory_ available";
+    TriggerCallback(std::move(callback), ResultCode::kErrorNoUrlLoader,
+                    PasswordRequirementsSpec());
+    return;
+  }
 
   if (!url_loader_factory_) {
     TriggerCallback(std::move(callback), ResultCode::kErrorNoUrlLoader,
@@ -101,6 +110,7 @@ void PasswordRequirementsSpecFetcherImpl::Fetch(
 
   if (!origin.is_valid() || origin.HostIsIPAddress() ||
       !origin.SchemeIsHTTPOrHTTPS()) {
+    VLOG(1) << "No valid origin";
     TriggerCallback(std::move(callback), ResultCode::kErrorInvalidOrigin,
                     PasswordRequirementsSpec());
     return;
@@ -122,6 +132,7 @@ void PasswordRequirementsSpecFetcherImpl::Fetch(
   if (iter != lookups_in_flight_.end()) {
     iter->second->callbacks.push_back(
         std::make_pair(origin, std::move(callback)));
+    VLOG(1) << "Lookup already in flight";
     return;
   }
 
@@ -179,6 +190,8 @@ void PasswordRequirementsSpecFetcherImpl::OnFetchComplete(
   lookup->download_timer.Stop();
 
   if (!response_body || lookup->url_loader->NetError() != net::Error::OK) {
+    VLOG(1) << "Fetch for " << hash_prefix << ": failed to fetch "
+            << lookup->url_loader->NetError();
     TriggerCallbackToAll(&lookup->callbacks, ResultCode::kErrorFailedToFetch,
                          PasswordRequirementsSpec());
     return;
@@ -186,6 +199,7 @@ void PasswordRequirementsSpecFetcherImpl::OnFetchComplete(
 
   PasswordRequirementsShard shard;
   if (!shard.ParseFromString(*response_body)) {
+    VLOG(1) << "Fetch for " << hash_prefix << ": failed to parse response";
     TriggerCallbackToAll(&lookup->callbacks, ResultCode::kErrorFailedToParse,
                          PasswordRequirementsSpec());
     return;
@@ -203,6 +217,7 @@ void PasswordRequirementsSpecFetcherImpl::OnFetchComplete(
     auto host_iter = shard.specs().find(host);
     if (host_iter != shard.specs().end()) {
       const PasswordRequirementsSpec& spec = host_iter->second;
+      VLOG(1) << "Found for " << host << ": " << spec;
       TriggerCallback(std::move(callback_function), ResultCode::kFoundSpec,
                       spec);
       continue;
@@ -225,6 +240,7 @@ void PasswordRequirementsSpecFetcherImpl::OnFetchComplete(
       if (it != shard.specs().end()) {
         const PasswordRequirementsSpec& spec = it->second;
         found_entry = true;
+        VLOG(1) << "Found for " << host << ": " << spec;
         TriggerCallback(std::move(callback_function), ResultCode::kFoundSpec,
                         spec);
         break;
@@ -232,6 +248,7 @@ void PasswordRequirementsSpecFetcherImpl::OnFetchComplete(
     }
 
     if (!found_entry) {
+      VLOG(1) << "Found no entry for " << host;
       TriggerCallback(std::move(callback_function), ResultCode::kFoundNoSpec,
                       PasswordRequirementsSpec());
     }

components/autofill/core/browser/password_requirements_spec_printer.cc

+// Copyright 2018 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/autofill/core/browser/password_requirements_spec_printer.h"
+
+std::ostream& operator<<(
+    std::ostream& out,
+    const autofill::PasswordRequirementsSpec::CharacterClass& character_class) {
+  out << "{";
+  if (character_class.has_character_set())
+    out << "character_set: \"" << character_class.character_set() << "\", ";
+  if (character_class.has_min())
+    out << "min: " << character_class.min() << ", ";
+  if (character_class.has_max())
+    out << "max: " << character_class.max() << ", ";
+  out << "}";
+  return out;
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         const autofill::PasswordRequirementsSpec& spec) {
+  out << "{";
+  if (spec.has_priority())
+    out << "priority: " << spec.priority() << ", ";
+  if (spec.has_spec_version())
+    out << "spec_version: " << spec.spec_version() << ", ";
+  if (spec.has_min_length())
+    out << "min_length: " << spec.min_length() << ", ";
+  if (spec.has_max_length())
+    out << "max_length: " << spec.max_length() << ", ";
+  if (spec.has_lower_case())
+    out << "lower_case: " << spec.lower_case() << ", ";
+  if (spec.has_upper_case())
+    out << "upper_case: " << spec.upper_case() << ", ";
+  if (spec.has_alphabetic())
+    out << "alphabetic: " << spec.alphabetic() << ", ";
+  if (spec.has_numeric())
+    out << "numeric: " << spec.alphabetic() << ", ";
+  if (spec.has_symbols())
+    out << "symbols: " << spec.symbols() << ", ";
+  out << "}";
+  return out;
+}

components/autofill/core/browser/password_requirements_spec_printer.h

+// Copyright 2018 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_AUTOFILL_CORE_BROWSER_PASSWORD_REQUIREMENTS_SPEC_PRINTER_H_
+#define COMPONENTS_AUTOFILL_CORE_BROWSER_PASSWORD_REQUIREMENTS_SPEC_PRINTER_H_
+
+#include <ostream>
+
+#include "components/autofill/core/browser/proto/password_requirements.pb.h"
+
+std::ostream& operator<<(
+    std::ostream& out,
+    const autofill::PasswordRequirementsSpec::CharacterClass& character_class);
+
+std::ostream& operator<<(std::ostream& out,
+                         const autofill::PasswordRequirementsSpec& spec);
+
+#endif  // COMPONENTS_AUTOFILL_CORE_BROWSER_PASSWORD_REQUIREMENTS_SPEC_PRINTER_H_

components/password_manager/core/browser/password_requirements_service.cc

@@ -7,6 +7,8 @@
 #include "base/bind.h"
 #include "base/logging.h"
 
+#include "components/autofill/core/browser/password_requirements_spec_printer.h"
+
 namespace {
 constexpr size_t kCacheSizeForDomainKeyedSpecs = 200;
 constexpr size_t kCacheSizeForSignatureKeyedSpecs = 500;
@@ -53,10 +55,16 @@ autofill::PasswordRequirementsSpec PasswordRequirementsService::GetSpec(
     }
   }
 
+  VLOG(1) << "PasswordRequirementsService::GetSpec(" << main_frame_domain
+          << ", " << form_signature << ", " << field_signature
+          << ") = " << result;
+
   return result;
 }
 
 void PasswordRequirementsService::PrefetchSpec(const GURL& main_frame_domain) {
+  VLOG(1) << "PasswordRequirementsService::PrefetchSpec(" << main_frame_domain
+          << ")";
   // Using base::Unretained(this) is safe here because the
   // PasswordRequirementsService owns fetcher_. If |this| is deleted, so is
   // the |fetcher_|, and no callback can happen.
@@ -69,6 +77,8 @@ void PasswordRequirementsService::PrefetchSpec(const GURL& main_frame_domain) {
 void PasswordRequirementsService::OnFetchedRequirements(
     const GURL& main_frame_domain,
     const autofill::PasswordRequirementsSpec& spec) {
+  VLOG(1) << "PasswordRequirementsService::OnFetchedRequirements("
+          << main_frame_domain << ", " << spec << ")";
   specs_for_domains_.Put(main_frame_domain, spec);
 }
 
@@ -76,6 +86,8 @@ void PasswordRequirementsService::AddSpec(
     autofill::FormSignature form_signature,
     autofill::FieldSignature field_signature,
     const autofill::PasswordRequirementsSpec& spec) {
+  VLOG(1) << "PasswordRequirementsService::AddSpec(" << form_signature << ", "
+          << field_signature << ", " << spec << ")";
   specs_for_signatures_.Put(std::make_pair(form_signature, field_signature),
                             spec);
 }