Reland: Cache protected password entry and password on focus ping separately.

components/safe_browsing/password_protection/password_protection_service.cc

Index: components/safe_browsing/password_protection/password_protection_service.cc
diff --git a/components/safe_browsing/password_protection/password_protection_service.cc b/components/safe_browsing/password_protection/password_protection_service.cc
index a16ef3afd719de6a893d9adbc83b0685b1ec5030..054cb50713002eb18626728d0cdb22e5492fbf40 100644
--- a/components/safe_browsing/password_protection/password_protection_service.cc
+++ b/components/safe_browsing/password_protection/password_protection_service.cc
@@ -38,6 +38,7 @@ const char kVerdictProto[] = "verdict_proto";
 const int kRequestTimeoutMs = 10000;
 const char kPasswordProtectionRequestUrl[] =
     "https://sb-ssl.google.com/safebrowsing/clientreport/login";
+const char kPasswordOnFocusCacheKey[] = "password_on_focus_cache_key";
 
 // Helper function to determine if the given origin matches content settings
 // map's patterns.
@@ -87,7 +88,8 @@ PasswordProtectionService::PasswordProtectionService(
     scoped_refptr<net::URLRequestContextGetter> request_context_getter,
     HistoryService* history_service,
     HostContentSettingsMap* host_content_settings_map)
-    : stored_verdict_count_(-1),
+    : stored_verdict_count_password_on_focus_(-1),
+      stored_verdict_count_password_entry_(-1),
       database_manager_(database_manager),
       request_context_getter_(request_context_getter),
       history_service_observer_(this),
@@ -125,21 +127,37 @@ bool PasswordProtectionService::CanGetReputationOfURL(const GURL& url) {
 LoginReputationClientResponse::VerdictType
 PasswordProtectionService::GetCachedVerdict(
     const GURL& url,
+    TriggerType type,
     LoginReputationClientResponse* out_response) {
+  DCHECK(content_settings_);
+  DCHECK(type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE ||
+         type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
+
   if (!url.is_valid())
     return LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED;
 
-  DCHECK(content_settings_);
-
   GURL hostname = GetHostNameWithHTTPScheme(url);
-  std::unique_ptr<base::DictionaryValue> verdict_dictionary =
+  std::unique_ptr<base::DictionaryValue> cache_dictionary =
       base::DictionaryValue::From(content_settings_->GetWebsiteSetting(
           hostname, GURL(), CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION,
           std::string(), nullptr));
-  // Return early if there is no verdict cached for this origin.
-  if (!verdict_dictionary.get() || verdict_dictionary->empty())
+
+  if (!cache_dictionary.get() || cache_dictionary->empty())
     return LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED;
 
+  base::DictionaryValue* verdict_dictionary = nullptr;
+  if (type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE) {
+    // All UNFAMILIAR_LOGIN_PAGE verdicts (a.k.a password on focus ping)
+    // are cached under |kPasswordOnFocusCacheKey|.
+    if (!cache_dictionary->HasKey(kPasswordOnFocusCacheKey)) {
+      return LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED;
+    }
+    DCHECK(cache_dictionary->GetDictionaryWithoutPathExpansion(
+        kPasswordOnFocusCacheKey, &verdict_dictionary));
+  } else {
+    verdict_dictionary = cache_dictionary.get();
+  }
+
   std::vector<std::string> paths;
   GeneratePathVariantsWithoutQuery(url, &paths);
   int max_path_depth = -1;
@@ -148,8 +166,12 @@ PasswordProtectionService::GetCachedVerdict(
   // For all the verdicts of the same origin, we key them by |cache_expression|.
   // Its corresponding value is a DictionaryValue contains its creation time and
   // the serialized verdict proto.
-  for (base::DictionaryValue::Iterator it(*verdict_dictionary.get());
-       !it.IsAtEnd(); it.Advance()) {
+  for (base::DictionaryValue::Iterator it(*verdict_dictionary); !it.IsAtEnd();
+       it.Advance()) {
+    if (type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT &&
+        it.key() == kPasswordOnFocusCacheKey) {
+      continue;
+    }
     base::DictionaryValue* verdict_entry = nullptr;
     CHECK(verdict_dictionary->GetDictionaryWithoutPathExpansion(
         it.key() /* cache_expression */, &verdict_entry));
@@ -180,39 +202,64 @@ PasswordProtectionService::GetCachedVerdict(
 
 void PasswordProtectionService::CacheVerdict(
     const GURL& url,
+    TriggerType type,
     LoginReputationClientResponse* verdict,
     const base::Time& receive_time) {
   DCHECK(verdict);
   DCHECK(content_settings_);
+  DCHECK(type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE ||
+         type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
 
   GURL hostname = GetHostNameWithHTTPScheme(url);
-  std::unique_ptr<base::DictionaryValue> verdict_dictionary =
+  int* stored_verdict_count =
+      type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE
+          ? &stored_verdict_count_password_on_focus_
+          : &stored_verdict_count_password_entry_;
+  std::unique_ptr<base::DictionaryValue> cache_dictionary =
       base::DictionaryValue::From(content_settings_->GetWebsiteSetting(
           hostname, GURL(), CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION,
           std::string(), nullptr));
 
-  if (!verdict_dictionary.get())
-    verdict_dictionary = base::MakeUnique<base::DictionaryValue>();
+  if (!cache_dictionary.get())
+    cache_dictionary = base::MakeUnique<base::DictionaryValue>();
 
   std::unique_ptr<base::DictionaryValue> verdict_entry =
       CreateDictionaryFromVerdict(verdict, receive_time);
 
+  base::DictionaryValue* verdict_dictionary = nullptr;
+  if (type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE) {
+    // All UNFAMILIAR_LOGIN_PAGE verdicts (a.k.a password on focus ping)
+    // are cached under |kPasswordOnFocusCacheKey|.
+    if (!cache_dictionary->HasKey(kPasswordOnFocusCacheKey)) {
+      cache_dictionary->SetDictionaryWithoutPathExpansion(
+          kPasswordOnFocusCacheKey, base::MakeUnique<base::DictionaryValue>());
+    }
+    DCHECK(cache_dictionary->GetDictionaryWithoutPathExpansion(
+        kPasswordOnFocusCacheKey, &verdict_dictionary));
+  } else {
+    verdict_dictionary = cache_dictionary.get();
+  }
+
   // Increases stored verdict count if we haven't seen this cache expression
   // before.
   if (!verdict_dictionary->HasKey(verdict->cache_expression()))
-    stored_verdict_count_ = GetStoredVerdictCount() + 1;
+    *stored_verdict_count = GetStoredVerdictCount(type) + 1;
   // If same cache_expression is already in this verdict_dictionary, we simply
   // override it.
   verdict_dictionary->SetWithoutPathExpansion(verdict->cache_expression(),
                                               std::move(verdict_entry));
   content_settings_->SetWebsiteSettingDefaultScope(
       hostname, GURL(), CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION,
-      std::string(), std::move(verdict_dictionary));
+      std::string(), std::move(cache_dictionary));
 }
 
 void PasswordProtectionService::CleanUpExpiredVerdicts() {
   DCHECK(content_settings_);
-  if (GetStoredVerdictCount() <= 0)
+
+  if (GetStoredVerdictCount(
+          LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE) <= 0 &&
+      GetStoredVerdictCount(
+          LoginReputationClientRequest::PASSWORD_REUSE_EVENT) <= 0)
     return;
 
   ContentSettingsForOneType password_protection_settings;
@@ -224,44 +271,29 @@ void PasswordProtectionService::CleanUpExpiredVerdicts() {
        password_protection_settings) {
     GURL primary_pattern_url = GURL(source.primary_pattern.ToString());
     // Find all verdicts associated with this origin.
-    std::unique_ptr<base::DictionaryValue> verdict_dictionary =
+    std::unique_ptr<base::DictionaryValue> cache_dictionary =
         base::DictionaryValue::From(content_settings_->GetWebsiteSetting(
             primary_pattern_url, GURL(),
             CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, std::string(), nullptr));
-    std::vector<std::string> expired_keys;
-    for (base::DictionaryValue::Iterator it(*verdict_dictionary.get());
-         !it.IsAtEnd(); it.Advance()) {
-      base::DictionaryValue* verdict_entry = nullptr;
-      CHECK(verdict_dictionary->GetDictionaryWithoutPathExpansion(
-          it.key(), &verdict_entry));
-      int verdict_received_time;
-      LoginReputationClientResponse verdict;
-      CHECK(ParseVerdictEntry(verdict_entry, &verdict_received_time, &verdict));
-
-      if (IsCacheExpired(verdict_received_time, verdict.cache_duration_sec())) {
-        // Since DictionaryValue::Iterator cannot be used to modify the
-        // dictionary, we record the keys of expired verdicts in |expired_keys|
-        // and remove them in the next for-loop.
-        expired_keys.push_back(it.key());
-      }
-    }
-
-    for (const std::string& key : expired_keys) {
-      verdict_dictionary->RemoveWithoutPathExpansion(key, nullptr);
-      stored_verdict_count_--;
-    }
-
-    if (verdict_dictionary->size() == 0u) {
+    bool has_expired_password_on_focus_entry = RemoveExpiredVerdicts(
+        LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE,
+        cache_dictionary.get());
+    bool has_expired_password_reuse_entry = RemoveExpiredVerdicts(
+        LoginReputationClientRequest::PASSWORD_REUSE_EVENT,
+        cache_dictionary.get());
+
+    if (cache_dictionary->size() == 0u) {
       content_settings_->ClearSettingsForOneTypeWithPredicate(
           CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, base::Time(),
           base::Bind(&OriginMatchPrimaryPattern, primary_pattern_url));
-    } else if (expired_keys.size() > 0u) {
+    } else if (has_expired_password_on_focus_entry ||
+               has_expired_password_reuse_entry) {
       // Set the website setting of this origin with the updated
       // |verdict_diectionary|.
       content_settings_->SetWebsiteSettingDefaultScope(
           primary_pattern_url, GURL(),
           CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, std::string(),
-          std::move(verdict_dictionary));
+          std::move(cache_dictionary));
     }
   }
 }
@@ -272,7 +304,7 @@ void PasswordProtectionService::StartRequest(
     const GURL& password_form_action,
     const GURL& password_form_frame_url,
     const std::string& saved_domain,
-    LoginReputationClientRequest::TriggerType type) {
+    TriggerType type) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   scoped_refptr<PasswordProtectionRequest> request(
       new PasswordProtectionRequest(web_contents, main_frame_url,
@@ -329,8 +361,8 @@ void PasswordProtectionService::RequestFinished(
 
   if (response) {
     if (!already_cached) {
-      CacheVerdict(request->main_frame_url(), response.get(),
-                   base::Time::Now());
+      CacheVerdict(request->main_frame_url(), request->request_type(),
+                   response.get(), base::Time::Now());
     }
 
     if (request->request_type() ==
@@ -376,17 +408,24 @@ GURL PasswordProtectionService::GetPasswordProtectionRequestUrl() {
   return url.Resolve("?key=" + net::EscapeQueryParamValue(api_key, true));
 }
 
-int PasswordProtectionService::GetStoredVerdictCount() {
+int PasswordProtectionService::GetStoredVerdictCount(TriggerType type) {
   DCHECK(content_settings_);
+  DCHECK(type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE ||
+         type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
+  int* stored_verdict_count =
+      type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE
+          ? &stored_verdict_count_password_on_focus_
+          : &stored_verdict_count_password_entry_;
   // If we have already computed this, return its value.
-  if (stored_verdict_count_ >= 0)
-    return stored_verdict_count_;
+  if (*stored_verdict_count >= 0)
+    return *stored_verdict_count;
 
   ContentSettingsForOneType password_protection_settings;
   content_settings_->GetSettingsForOneType(
       CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, std::string(),
       &password_protection_settings);
-  stored_verdict_count_ = 0;
+  stored_verdict_count_password_on_focus_ = 0;
+  stored_verdict_count_password_entry_ = 0;
   if (password_protection_settings.empty())
     return 0;
 
@@ -396,10 +435,20 @@ int PasswordProtectionService::GetStoredVerdictCount() {
         base::DictionaryValue::From(content_settings_->GetWebsiteSetting(
             GURL(source.primary_pattern.ToString()), GURL(),
             CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, std::string(), nullptr));
-    if (verdict_dictionary.get() && !verdict_dictionary->empty())
-      stored_verdict_count_ += static_cast<int>(verdict_dictionary->size());
+    if (verdict_dictionary.get() && !verdict_dictionary->empty()) {

[lpz, 2017/06/08 15:18:33]

I think this block could use a couple comments (ie: the outer dict size contains
both types of verdicts and we want to split them apart...is that right?)

[Jialiu Lin, 2017/06/08 20:47:30]

The outer dict size is # of PASSWORD_REUSE_EVENT verdict +1. The "+1" maps to
the lower layer dictionary (keyed on |kPasswordOnFocusCacheKey|) holds all
UMFAMILIAR_LOGIN_PAGE verdicts. 

Added one comment to make it clearer. 

+      stored_verdict_count_password_entry_ +=
+          static_cast<int>(verdict_dictionary->size());
+      if (verdict_dictionary->HasKey(kPasswordOnFocusCacheKey)) {
+        stored_verdict_count_password_entry_ -= 1;
+        base::DictionaryValue* password_on_focus_dict = nullptr;
+        verdict_dictionary->GetDictionaryWithoutPathExpansion(
+            kPasswordOnFocusCacheKey, &password_on_focus_dict);
+        stored_verdict_count_password_on_focus_ +=
+            static_cast<int>(password_on_focus_dict->size());
+      }
+    }
   }
-  return stored_verdict_count_;
+  return *stored_verdict_count;
 }
 
 int PasswordProtectionService::GetRequestTimeoutInMS() {
@@ -407,7 +456,7 @@ int PasswordProtectionService::GetRequestTimeoutInMS() {
 }
 
 void PasswordProtectionService::FillUserPopulation(
-    const LoginReputationClientRequest::TriggerType& request_type,
+    TriggerType request_type,
     LoginReputationClientRequest* request_proto) {
   ChromeUserPopulation* user_population = request_proto->mutable_population();
   user_population->set_user_population(
@@ -437,8 +486,10 @@ void PasswordProtectionService::OnURLsDeleted(
     bool expired,
     const history::URLRows& deleted_rows,
     const std::set<GURL>& favicon_urls) {
-  if (stored_verdict_count_ <= 0)
+  if (stored_verdict_count_password_on_focus_ <= 0 &&
+      stored_verdict_count_password_entry_ <= 0) {
     return;
+  }
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
@@ -460,7 +511,8 @@ void PasswordProtectionService::RemoveContentSettingsOnURLsDeleted(
   if (all_history) {
     content_settings_->ClearSettingsForOneType(
         CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION);
-    stored_verdict_count_ = 0;
+    stored_verdict_count_password_on_focus_ = 0;
+    stored_verdict_count_password_entry_ = 0;
     return;
   }
 
@@ -472,23 +524,99 @@ void PasswordProtectionService::RemoveContentSettingsOnURLsDeleted(
     if (!row.url().SchemeIsHTTPOrHTTPS())
       continue;
     GURL url_key = GetHostNameWithHTTPScheme(row.url());
-    std::unique_ptr<base::DictionaryValue> verdict_dictionary =
-        base::DictionaryValue::From(content_settings_->GetWebsiteSetting(
-            url_key, GURL(), CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION,
-            std::string(), nullptr));
-
-    // Move on if we have no cached verdict for this deleted history row.
-    if (!verdict_dictionary.get() || verdict_dictionary->empty())
-      continue;
-
-    int verdict_count = static_cast<int>(verdict_dictionary->size());
-    stored_verdict_count_ = GetStoredVerdictCount() - verdict_count;
+    stored_verdict_count_password_on_focus_ =
+        GetStoredVerdictCount(
+            LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE) -
+        GetVerdictCountForURL(
+            url_key, LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE);
+    stored_verdict_count_password_entry_ =
+        GetStoredVerdictCount(
+            LoginReputationClientRequest::PASSWORD_REUSE_EVENT) -
+        GetVerdictCountForURL(
+            url_key, LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
     content_settings_->ClearSettingsForOneTypeWithPredicate(
         CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, base::Time(),
         base::Bind(&OriginMatchPrimaryPattern, url_key));
   }
 }
 
+int PasswordProtectionService::GetVerdictCountForURL(const GURL& url,
+                                                     TriggerType type) {
+  DCHECK(type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE ||
+         type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
+  std::unique_ptr<base::DictionaryValue> verdict_dictionary =
+      base::DictionaryValue::From(content_settings_->GetWebsiteSetting(
+          url, GURL(), CONTENT_SETTINGS_TYPE_PASSWORD_PROTECTION, std::string(),
+          nullptr));
+  if (!verdict_dictionary.get() || verdict_dictionary->empty())
+    return 0;
+
+  if (type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE) {
+    base::DictionaryValue* password_on_focus_dict = nullptr;
+    verdict_dictionary->GetDictionaryWithoutPathExpansion(
+        kPasswordOnFocusCacheKey, &password_on_focus_dict);
+    return password_on_focus_dict ? password_on_focus_dict->size() : 0;
+  } else {
+    return verdict_dictionary->HasKey(kPasswordOnFocusCacheKey)
+               ? verdict_dictionary->size() - 1
+               : verdict_dictionary->size();
+  }
+}
+
+bool PasswordProtectionService::RemoveExpiredVerdicts(
+    TriggerType type,
+    base::DictionaryValue* cache_dict) {
+  DCHECK(type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE ||
+         type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
+  base::DictionaryValue* verdict_dictionary = nullptr;
+  if (type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT) {
+    verdict_dictionary = cache_dict;
+  } else {
+    if (!cache_dict->GetDictionaryWithoutPathExpansion(kPasswordOnFocusCacheKey,
+                                                       &verdict_dictionary)) {
+      return false;
+    }
+  }
+
+  if (!verdict_dictionary || verdict_dictionary->empty())
+    return false;

[lpz, 2017/06/08 15:18:33]

nit: +1 newline after

[Jialiu Lin, 2017/06/08 20:47:30]

Done.

+  std::vector<std::string> expired_keys;
+  for (base::DictionaryValue::Iterator it(*verdict_dictionary); !it.IsAtEnd();
+       it.Advance()) {
+    if (type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT &&
+        it.key() == std::string(kPasswordOnFocusCacheKey))
+      continue;

[lpz, 2017/06/08 15:18:33]

nit: +1 newline after

[Jialiu Lin, 2017/06/08 20:47:30]

Done.

+    base::DictionaryValue* verdict_entry = nullptr;
+    CHECK(verdict_dictionary->GetDictionaryWithoutPathExpansion(
+        it.key(), &verdict_entry));
+    int verdict_received_time;
+    LoginReputationClientResponse verdict;
+    CHECK(ParseVerdictEntry(verdict_entry, &verdict_received_time, &verdict));
+
+    if (IsCacheExpired(verdict_received_time, verdict.cache_duration_sec())) {
+      // Since DictionaryValue::Iterator cannot be used to modify the
+      // dictionary, we record the keys of expired verdicts in |expired_keys|
+      // and remove them in the next for-loop.
+      expired_keys.push_back(it.key());
+    }
+  }
+
+  for (const std::string& key : expired_keys) {
+    verdict_dictionary->RemoveWithoutPathExpansion(key, nullptr);
+    if (type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT)
+      stored_verdict_count_password_entry_--;
+    else
+      stored_verdict_count_password_on_focus_--;
+  }
+
+  if (type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE &&
+      verdict_dictionary->size() == 0U) {
+    cache_dict->RemoveWithoutPathExpansion(kPasswordOnFocusCacheKey, nullptr);
+  }
+
+  return expired_keys.size() > 0U;
+}
+
 // static
 bool PasswordProtectionService::ParseVerdictEntry(
     base::DictionaryValue* verdict_entry,