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

components/safe_browsing/password_protection/password_protection_service.h

 // Copyright 2017 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_SAFE_BROWSING_PASSWORD_PROTECTION_PASSWORD_PROTECTION_SERVICE_H_
 #define COMPONENTS_SAFE_BROWSING_PASSWORD_PROTECTION_PASSWORD_PROTECTION_SERVICE_H_
 
 #include <set>
 
 #include "base/callback.h"
@@ skipping 31 matching lines @@
 extern const base::Feature kPasswordProtectionInterstitial;
 extern const char kPasswordOnFocusRequestOutcomeHistogramName[];
 extern const char kPasswordEntryRequestOutcomeHistogramName[];
 
 // Manage password protection pings and verdicts. There is one instance of this
 // class per profile. Therefore, every PasswordProtectionService instance is
 // associated with a unique HistoryService instance and a unique
 // HostContentSettingsMap instance.
 class PasswordProtectionService : public history::HistoryServiceObserver {
  public:
+  using TriggerType = LoginReputationClientRequest::TriggerType;
   // The outcome of the request. These values are used for UMA.
   // DO NOT CHANGE THE ORDERING OF THESE VALUES.
   enum RequestOutcome {
     UNKNOWN = 0,
     SUCCEEDED = 1,
     CANCELED = 2,
     TIMEDOUT = 3,
     MATCHED_WHITELIST = 4,
     RESPONSE_ALREADY_CACHED = 5,
     DEPRECATED_NO_EXTENDED_REPORTING = 6,
@@ skipping 18 matching lines @@
 
   base::WeakPtr<PasswordProtectionService> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
 
   // Looks up |settings| to find the cached verdict response. If verdict is not
   // available or is expired, return VERDICT_TYPE_UNSPECIFIED. Can be called on
   // any thread.
   LoginReputationClientResponse::VerdictType GetCachedVerdict(
       const GURL& url,
+      TriggerType type,
       LoginReputationClientResponse* out_response);
 
-  // Stores |verdict| in |settings| based on |url|, |verdict| and
+  // Stores |verdict| in |settings| based on its |type|, |url|, |verdict| and
   // |receive_time|.
   virtual void CacheVerdict(const GURL& url,
+                            TriggerType type,
                             LoginReputationClientResponse* verdict,
                             const base::Time& receive_time);
 
   // Removes all the expired verdicts from cache.
   void CleanUpExpiredVerdicts();
 
   // Creates an instance of PasswordProtectionRequest and call Start() on that
   // instance. This function also insert this request object in |requests_| for
   // record keeping.
   void StartRequest(content::WebContents* web_contents,
                     const GURL& main_frame_url,
                     const GURL& password_form_action,
                     const GURL& password_form_frame_url,
                     const std::string& saved_domain,
-                    LoginReputationClientRequest::TriggerType type);
+                    TriggerType type);

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

nit: type -> trigger_type or verdict_type or request_type everywhere in this CL

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

Done.

 
   virtual void MaybeStartPasswordFieldOnFocusRequest(
       content::WebContents* web_contents,
       const GURL& main_frame_url,
       const GURL& password_form_action,
       const GURL& password_form_frame_url);
 
   virtual void MaybeStartProtectedPasswordEntryRequest(
       content::WebContents* web_contents,
       const GURL& main_frame_url,
@@ skipping 23 matching lines @@
   // itself from |requests_|.
   virtual void RequestFinished(
       PasswordProtectionRequest* request,
       bool already_cached,
       std::unique_ptr<LoginReputationClientResponse> response);
 
   // Cancels all requests in |requests_|, empties it, and releases references to
   // the requests.
   void CancelPendingRequests();
 
-  // Gets the total number of verdict (no matter expired or not) we cached for
-  // current active profile.
-  virtual int GetStoredVerdictCount();
+  // Gets the total number of verdicts of the specified |type| (no matter

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

nit: i find the wording a bit awkward, consider:

Gets the total number of verdicts of the specified |type| we cached for the
current active profile. This counts both expired and active verdicts.

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

Done.

+  // expired or not) we cached for current active profile.
+  virtual int GetStoredVerdictCount(TriggerType type);
 
   scoped_refptr<net::URLRequestContextGetter> request_context_getter() {
     return request_context_getter_;
   }
 
   // Returns the URL where PasswordProtectionRequest instances send requests.
   static GURL GetPasswordProtectionRequestUrl();
 
   // Gets the request timeout in milliseconds.
   static int GetRequestTimeoutInMS();
 
   // Obtains referrer chain of |event_url| and |event_tab_id| and adds this
   // info into |frame|.
   virtual void FillReferrerChain(
       const GURL& event_url,
       int event_tab_id,  // -1 if tab id is not available.
       LoginReputationClientRequest::Frame* frame) = 0;
 
-  void FillUserPopulation(
-      const LoginReputationClientRequest::TriggerType& request_type,
-      LoginReputationClientRequest* request_proto);
+  void FillUserPopulation(TriggerType request_type,
+                          LoginReputationClientRequest* request_proto);
 
   virtual bool IsExtendedReporting() = 0;
 
   virtual bool IsIncognito() = 0;
 
   virtual bool IsPingingEnabled(const base::Feature& feature,
                                 RequestOutcome* reason) = 0;
 
   virtual bool IsHistorySyncEnabled() = 0;
 
@@ skipping 27 matching lines @@
                      const std::set<GURL>& favicon_urls) override;
 
   void HistoryServiceBeingDeleted(
       history::HistoryService* history_service) override;
 
   // Posted to UI thread by OnURLsDeleted(..). This function cleans up password
   // protection content settings related to deleted URLs.
   void RemoveContentSettingsOnURLsDeleted(bool all_history,
                                           const history::URLRows& deleted_rows);
 
+  // Helper function called by RemoveContentSettingsOnURLsDeleted(..). It
+  // calculate the number of verdicts of |type| that associate with |url|.
+  int GetVerdictCountForURL(const GURL& url, TriggerType type);
+
+  // Remove verdict of |type| from |cache_dict|. Return false if no verdict
+  // removed, true otherwise.
+  bool RemoveExpiredVerdicts(TriggerType type,
+                             base::DictionaryValue* cache_dict);
+
   static bool ParseVerdictEntry(base::DictionaryValue* verdict_entry,
                                 int* out_verdict_received_time,
                                 LoginReputationClientResponse* out_verdict);
 
   static bool PathVariantsMatchCacheExpression(
       const std::vector<std::string>& generated_paths,
       const std::string& cache_expression_path);
 
   static bool IsCacheExpired(int cache_creation_time, int cache_duration);
 
   static void GeneratePathVariantsWithoutQuery(const GURL& url,
                                                std::vector<std::string>* paths);
 
   static std::string GetCacheExpressionPath(
       const std::string& cache_expression);
 
   static std::unique_ptr<base::DictionaryValue> CreateDictionaryFromVerdict(
       const LoginReputationClientResponse* verdict,
       const base::Time& receive_time);
 
   static void RecordNoPingingReason(const base::Feature& feature,
                                     RequestOutcome reason);
-  // Number of verdict stored for this profile.
-  int stored_verdict_count_;
+  // Number of verdict stored for this profile for password on focus pings.
+  int stored_verdict_count_password_on_focus_;
+
+  // Number of verdict stored for this profile for protected password entry
+  // pings.
+  int stored_verdict_count_password_entry_;

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

do you foresee having other verdict types that will need to be counted? If so,
perhaps store these in a map keyed by TriggerType?

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

Acknowledged.
No, these are the only two pings.

 
   scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
 
   // The context we use to issue network requests. This request_context_getter
   // is obtained from SafeBrowsingService so that we can use the Safe Browsing
   // cookie store.
   scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
 
   // Set of pending PasswordProtectionRequests.
   std::set<scoped_refptr<PasswordProtectionRequest>> requests_;
 
   ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
       history_service_observer_;
 
   // Content settings map associated with this instance.
   HostContentSettingsMap* content_settings_;
 
   // Weakptr can only cancel task if it is posted to the same thread. Therefore,
   // we need CancelableTaskTracker to cancel tasks posted to IO thread.
   base::CancelableTaskTracker tracker_;
 
   base::WeakPtrFactory<PasswordProtectionService> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(PasswordProtectionService);
 };
 
 }  // namespace safe_browsing
 
 #endif  // COMPONENTS_SAFE_BROWSING_PASSWORD_PROTECTION_PASSWORD_PROTECTION_SERVICE_H_