Add management related code to SafeBrowsingNavigationObserverManager

chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h

Index: chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h
diff --git a/chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h b/chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h
index f66172fe4eaf510de94361f484f9a1640cf77428..19fd9ac835fb4a163b825d91a86cd4015a52e66a 100644
--- a/chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h
+++ b/chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_NAVIGATION_OBSERVER_MANAGER_H_
 #define CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_NAVIGATION_OBSERVER_MANAGER_H_
 
+#include "chrome/common/safe_browsing/csd.pb.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -17,7 +18,7 @@ struct NavigationEvent;
 struct ResolvedIPAddress;
 
 // Manager class for SafeBrowsingNavigationObserver, which is in charge of
-// cleaning up stale navigation events, and identifing landing page/landing
+// cleaning up stale navigation events, and identifying landing page/landing
 // referrer for a specific download.
 // TODO(jialiul): For now, SafeBrowsingNavigationObserverManager also listens to
 // NOTIFICATION_RETARGETING as a way to detect cross frame/tab navigation.
@@ -27,6 +28,18 @@ class SafeBrowsingNavigationObserverManager
     : public content::NotificationObserver,
       public base::RefCountedThreadSafe<SafeBrowsingNavigationObserverManager> {
  public:
+  // For UMA histogram counting. Do NOT change order.
+  enum AttributionResult {
+    SUCCESS = 1,                   // Identified referrer chain is not empty.
+    SUCCESS_LANDING_PAGE = 2,      // Successfully identified landing page.
+    SUCCESS_LANDING_REFERRER = 3,  // Successfully identified landing referrer.
+    INVALID_URL = 4,
+    NAVIGATION_EVENT_NOT_FOUND = 5,
+
+    // Always at the end

[Charlie Reis, 2016/12/09 22:00:25]

nit: End with period.

[Jialiu Lin, 2016/12/12 23:43:38]

Done.

+    ATTRIBUTION_FAILURE_TYPE_MAX
+  };
+
   // Helper function to check if user gesture is older than
   // kUserGestureTTLInSecond.
   static bool IsUserGestureExpired(const base::Time& timestamp);
@@ -49,8 +62,22 @@ class SafeBrowsingNavigationObserverManager
   void RecordHostToIpMapping(const std::string& host, const std::string& ip);
   // Clean-ups need to be done when a WebContents gets destroyed.
   void OnWebContentDestroyed(content::WebContents* web_contents);
-
-  // TODO(jialiul): more functions are coming for managing navigation_map_.
+  // Remove all the observed NavigationEvents, user gestures, and resolved IP

[Charlie Reis, 2016/12/09 22:00:25]

nit: These will be easier to read with blank lines between the related blocks
(e.g., before lines 63, 65, 68, 74).

[Jialiu Lin, 2016/12/12 23:43:38]

Done.

+  // addresses that are older than kNavigationFootprintTTLInSecond.
+  void CleanUpStaleNavigationFootprints();
+  // Identify referrer chain within user_gesture_count_limit user gestures.

[Charlie Reis, 2016/12/09 22:00:25]

Can you elaborate on this comment?  This is kind of the heart of the analysis,
right?  It would help to describe that this looks through connected navigation
events that have been recorded to find which page was responsible.  Also worth
mentioning that it includes only user initiated events.

Please also document that |referrer_chain| is an out parameter.

[Jialiu Lin, 2016/12/12 23:43:38]

Done.

[Charlie Reis, 2016/12/14 07:43:57]

Thanks!  That helps a lot.

+  AttributionResult IdentifyReferrerChain(
+      const GURL& target_url,
+      int target_tab_id,  // -1 if tab id is not valid
+      int user_gesture_count_limit,
+      std::vector<ReferrerChainEntry>* referrer_chain);
+  // Identify and add referrer chain to ClientDownloadRequest proto.

[Charlie Reis, 2016/12/09 22:00:25]

Is |request| an output parameter?  Where is the proto?  Let's elaborate in the
comment.

Also mention that this records UMA stats on the results.

[Jialiu Lin, 2016/12/12 23:43:38]

ClientDownloadRequest is located at chrome/common/safe_browsing/csd.proto, this
is the proto going to be send to SB backend for download classification.
Added more details to this comment. 

+  // TODO(jialiul): This function will be moved to DownloadProtectionService
+  // class shortly.
+  void AddReferrerChainToClientDownloadRequest(
+      const GURL& download_url,
+      content::WebContents* source_contents,
+      ClientDownloadRequest* request);
 
  private:
   friend class base::RefCountedThreadSafe<
@@ -58,6 +85,12 @@ class SafeBrowsingNavigationObserverManager
   friend class TestNavigationObserverManager;
   friend class SBNavigationObserverBrowserTest;
   friend class SBNavigationObserverTest;
+  FRIEND_TEST_ALL_PREFIXES(SBNavigationObserverTest,

[Charlie Reis, 2016/12/09 22:00:25]

This is already listed as a friend class above.  Is there a reason these tests
need to be listed here as well?

[Jialiu Lin, 2016/12/12 23:43:38]

Done. 
These tests called SafeBrowsingNavigationObserverManager's private functions,
though it actually makes more sense to add wrappers in the
SBNavigationObserverTest class. I'll remove these FRIEND_TEST_ALL_PREFIXES. 

+                           TestCleanUpStaleNavigationEvents);
+  FRIEND_TEST_ALL_PREFIXES(SBNavigationObserverTest,
+                           TestCleanUpStaleUserGestures);
+  FRIEND_TEST_ALL_PREFIXES(SBNavigationObserverTest,
+                           TestCleanUpStaleIPAddresses);
 
   struct GurlHash {
     std::size_t operator()(const GURL& url) const {
@@ -84,6 +117,30 @@ class SafeBrowsingNavigationObserverManager
 
   HostToIpMap* host_to_ip_map() { return &host_to_ip_map_; }
 
+  // Remove stale entries from navigation_map_ if they are older than
+  // kNavigationFootprintTTLInSecond (2 minutes).
+  void CleanUpNavigationEvents();
+
+  // Remove stale entries from user_gesture_map_ if they are older than
+  // kUserGestureTTLInSecond (1 sec).
+  void CleanUpUserGestures();
+
+  // Remove stale entries from host_to_ip_map_ if they are older than
+  // kNavigationFootprintTTLInSecond (2 minutes).
+  void CleanUpIpAddresses();
+
+  bool IsCleanUpScheduled() const;
+
+  void ScheduleNextCleanUpAfterInterval(base::TimeDelta interval);
+
+  NavigationEvent* FindNavigationEvent(const GURL& target_url,
+                                       int target_tab_id);
+  void AddToReferrerChain(
+      std::vector<ReferrerChainEntry>*
+          referrer_chain,
+      NavigationEvent* nav_event,
+      ReferrerChainEntry::URLType type);
+
   // navigation_map_ keeps track of all the observed navigations. This map is
   // keyed on the resolved request url. In other words, in case of server
   // redirects, its key is the last server redirect url, otherwise, it is the
@@ -108,6 +165,8 @@ class SafeBrowsingNavigationObserverManager
 
   content::NotificationRegistrar registrar_;
 
+  base::OneShotTimer cleanup_timer_;
+
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingNavigationObserverManager);
 };
 }  // namespace safe_browsing