Add SafeBrowsingNavigationObserver to listen to navigation events

chrome/browser/safe_browsing/safe_browsing_navigation_observer.cc

Index: chrome/browser/safe_browsing/safe_browsing_navigation_observer.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_navigation_observer.cc b/chrome/browser/safe_browsing/safe_browsing_navigation_observer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ee49fb42707ea28612316583fe88d5e4c006608e
--- /dev/null
+++ b/chrome/browser/safe_browsing/safe_browsing_navigation_observer.cc
@@ -0,0 +1,231 @@
+// Copyright 2016 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 "chrome/browser/safe_browsing/safe_browsing_navigation_observer.h"
+
+#include "base/memory/ptr_util.h"
+#include "base/time/time.h"
+#include "chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h"
+#include "chrome/browser/sessions/session_tab_helper.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/resource_request_details.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/common/resource_type.h"
+
+using content::WebContents;
+
+namespace {
+const char kWebContentsUserDataKey[] =
+    "web_contents_safe_browsing_navigation_observer";
+}  // namespace
+
+namespace safe_browsing {
+
+// SafeBrowsingNavigationObserver::NavigationEvent-----------------------------
+NavigationEvent::NavigationEvent()
+    : source_url(),

[Charlie Reis, 2016/10/17 23:54:03]

nit: These empty initializations aren't needed, right?  Most code I've seen
omits them.

[Jialiu Lin, 2016/10/18 19:06:04]

Acknowledged. Compiler complains if I omit this default constructor. It is used
by safe_browsing_navigation_observer_manager.cc, and in tests.

[Charlie Reis, 2016/10/19 17:10:45]

Acknowledged.

+      source_tab_id(-1),
+      target_url(),
+      target_tab_id(-1),
+      frame_id(-1),
+      main_frame_url(),
+      timestamp(base::Time::Now()),
+      is_user_initiated(false),
+      has_committed(false),
+      has_server_redirect(false),
+      server_redirect_url() {}
+
+NavigationEvent::NavigationEvent(NavigationEvent&& nav_event)
+    : source_url(std::move(nav_event.source_url)),
+      source_tab_id(std::move(nav_event.source_tab_id)),
+      target_url(std::move(nav_event.target_url)),
+      target_tab_id(std::move(nav_event.target_tab_id)),
+      frame_id(nav_event.frame_id),
+      main_frame_url(std::move(nav_event.main_frame_url)),
+      timestamp(nav_event.timestamp),
+      is_user_initiated(nav_event.is_user_initiated),
+      has_committed(nav_event.has_committed),
+      has_server_redirect(nav_event.has_server_redirect),
+      server_redirect_url(std::move(nav_event.server_redirect_url)) {}
+
+NavigationEvent& NavigationEvent::operator=(NavigationEvent&& nav_event) {
+  source_url = std::move(nav_event.source_url);
+  source_tab_id = std::move(nav_event.source_tab_id);
+  target_url = std::move(nav_event.target_url);
+  target_tab_id = std::move(nav_event.target_tab_id);
+  frame_id = nav_event.frame_id;
+  main_frame_url = std::move(nav_event.main_frame_url);
+  timestamp = nav_event.timestamp;
+  is_user_initiated = nav_event.is_user_initiated;
+  has_committed = nav_event.has_committed;
+  has_server_redirect = nav_event.has_server_redirect;
+  server_redirect_url = std::move(nav_event.server_redirect_url);
+  return *this;
+}
+
+NavigationEvent::~NavigationEvent() {}
+
+// SafeBrowsingNavigationObserver --------------------------------------------
+
+// static
+void SafeBrowsingNavigationObserver::MaybeCreateForWebContents(
+    content::WebContents* web_contents) {
+  if (FromWebContents(web_contents))
+    return;
+  NOTIMPLEMENTED();
+  // TODO(jialiul): This method will be called by TabHelpers::AttachTabHelpers.
+  // Complete this method when the entire class is ready.
+}
+
+// static
+SafeBrowsingNavigationObserver* SafeBrowsingNavigationObserver::FromWebContents(
+    content::WebContents* web_contents) {
+  return static_cast<SafeBrowsingNavigationObserver*>(
+      web_contents->GetUserData(kWebContentsUserDataKey));
+}
+
+SafeBrowsingNavigationObserver::SafeBrowsingNavigationObserver(
+    content::WebContents* contents,
+    const scoped_refptr<SafeBrowsingNavigationObserverManager>& manager)
+    : content::WebContentsObserver(contents),
+      manager_(manager),
+      has_user_gesture_(false),
+      last_user_gesture_timestamp_(base::Time()) {}
+
+SafeBrowsingNavigationObserver::~SafeBrowsingNavigationObserver() {}
+
+// Called when a navigation started in the WebContents. |navigation_handle| in
+// parameter is unique to this navigation, which will appear in the following
+// DidRedirectNavigation, and DidFinishNavigation too.
+void SafeBrowsingNavigationObserver::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  // If we already seen this navigation_handle before, no need to do anything.

[Charlie Reis, 2016/10/17 23:54:03]

I'm not sure whether it's a good idea to ignore it.  It might be possible to get
two DidStarts if navigation does a cross-process transfer, for example, in which
case you might want to update the data structure.  (Even if none of the current
properties can change during a transfer, I'd worry about bugs if we added a
property later, like which RFH it's in.)

[Jialiu Lin, 2016/10/18 19:06:04]

Acknowledged. I added a TODO here, since this 2 DidStarts scenario may also
requires changes in the why how I handle user gestures.
If navifation does a cross-process transfer, will it generates 2 DidStarts all
the time or only occasionally? Since Safe Browsing backend will aggregate these
referrer chains, we don't need to get every NavigationEvent correct in all
corner cases.  

[Charlie Reis, 2016/10/19 17:10:45]

This happens on every transfer, not just occasionally.  You should be able to
test it by doing a server redirect to the Chrome Web Store (e.g., fire up
testserver.py and go to
http://127.0.0.1:8080/server-redirect?https://chrome.google.com/webstore).  (I'm
away from a debugger right now so I haven't verified it manually, but I'm pretty
sure that's how it works.)

Because of this and your user gesture concern, we may want to consider fixing it
before landing the CL, or at least characterize exactly what might be broken in
those cases.

[Jialiu Lin, 2016/10/21 02:38:38]

Agree.  refactored this function to address this concern. 

+  if (navigation_handle_map_.find(navigation_handle) !=
+      navigation_handle_map_.end())
+    return;
+
+  // Construct a NavigationEvent based on available information in
+  // navigation_handle.
+  NavigationEvent nav_event;
+  content::RenderFrameHost* host =
+      navigation_handle->GetWebContents()->FindFrameByFrameTreeNodeId(
+          navigation_handle->GetFrameTreeNodeId());

[Charlie Reis, 2016/10/17 23:54:03]

This doesn't look safe, at least for general uses of |host| as the RFH being
navigated.  The navigation could be started from a pending RenderFrameHost, not
the current RenderFrameHost for the node.  (If you explicitly want the
|current_frame_host|, then this would be ok.)

[Jialiu Lin, 2016/10/18 19:06:04]

Let's keep it for now. Since |host| here is only used to figure out the
source_url. After we can get the true initiating url from NavigationHandle, we
don't need this RHF anymore. 

[Charlie Reis, 2016/10/19 17:10:45]

I see.  We should name it current_frame_host to be clear if we keep it.

[Jialiu Lin, 2016/10/21 02:38:38]

Done.

+
+  // If there was URL previously committed in this render frame host,
+  // set it as the source url of this navigation. Otherwise, this is the
+  // first url going to commit in this frame. We set navigation_handle's URL as
+  // the source url.
+  if (host && host->GetLastCommittedURL().is_valid()) {

[Charlie Reis, 2016/10/17 23:54:03]

The last committed URL in the frame is not the source URL for a navigation. 
Another frame could have targeted this frame after something was already loaded
in it.

(That's why we want to add an initiating URL/origin to the NavigationHandle,
right?  If this is just a stopgap until that's added, we should add a big scary
TODO that the information in source_url is wrong for the time being.)  :)

[Jialiu Lin, 2016/10/18 19:06:04]

Oh, yes, forgot to add this TODO. 

+    nav_event.source_url =
+        SafeBrowsingNavigationObserverManager::ClearEmptyURLRef(
+            host->GetLastCommittedURL());
+  } else {
+    nav_event.source_url =
+        SafeBrowsingNavigationObserverManager::ClearEmptyURLRef(
+            navigation_handle->GetURL());
+  }
+  nav_event.target_url =
+      SafeBrowsingNavigationObserverManager::ClearEmptyURLRef(
+          navigation_handle->GetURL());
+
+  nav_event.source_tab_id =
+      SessionTabHelper::IdForTab(navigation_handle->GetWebContents());
+  nav_event.timestamp = base::Time::Now();
+  nav_event.frame_id = navigation_handle->GetFrameTreeNodeId();
+
+  if (navigation_handle->IsInMainFrame()) {
+    nav_event.main_frame_url = nav_event.source_url;
+  } else {
+    nav_event.main_frame_url =
+        SafeBrowsingNavigationObserverManager::ClearEmptyURLRef(
+            navigation_handle->GetWebContents()->GetLastCommittedURL());
+  }
+  if ((has_user_gesture_ &&
+       !SafeBrowsingNavigationObserverManager::IsUserGestureExpired(
+           last_user_gesture_timestamp_)) ||
+      !navigation_handle->IsRendererInitiated()) {
+    nav_event.is_user_initiated = has_user_gesture_;
+    manager_->OnUserGestureConsumed(web_contents(),
+                                    last_user_gesture_timestamp_);
+  }
+  has_user_gesture_ = false;
+  navigation_handle_map_.insert(
+      std::make_pair(navigation_handle, std::move(nav_event)));
+}
+
+void SafeBrowsingNavigationObserver::DidRedirectNavigation(
+    content::NavigationHandle* navigation_handle) {
+  // We should have already seen this navigation_handle in DidStartNavigation.
+  if (navigation_handle_map_.find(navigation_handle) ==
+      navigation_handle_map_.end())
+    return;

[Charlie Reis, 2016/10/17 23:54:03]

Maybe add a NOTREACHED to aid with debugging and tests?

[Jialiu Lin, 2016/10/18 19:06:04]

Good idea. Done.

+
+  NavigationEvent* nav_event = &navigation_handle_map_[navigation_handle];
+  nav_event->has_server_redirect = true;
+  nav_event->server_redirect_url =
+      SafeBrowsingNavigationObserverManager::ClearEmptyURLRef(
+          navigation_handle->GetURL());
+  nav_event->timestamp = base::Time::Now();
+}
+
+void SafeBrowsingNavigationObserver::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (navigation_handle_map_.find(navigation_handle) ==
+      navigation_handle_map_.end())
+    return;

[Charlie Reis, 2016/10/17 23:54:03]

NOTREACHED?

[Jialiu Lin, 2016/10/18 19:06:04]

Done.

+
+  // If it is an error page, we ignore this navigation.
+  if (navigation_handle->IsErrorPage()) {
+    navigation_handle_map_.erase(navigation_handle);
+    return;
+  }
+  NavigationEvent* nav_event = &navigation_handle_map_[navigation_handle];
+
+  nav_event->has_committed = navigation_handle->HasCommitted();
+  nav_event->target_tab_id =
+      SessionTabHelper::IdForTab(navigation_handle->GetWebContents());
+  nav_event->timestamp = base::Time::Now();
+
+  GURL nav_event_key =

[Charlie Reis, 2016/10/17 23:54:03]

I'm not sure I understand this complex clause.  Would
navigation_handle->GetURL() give us the same thing (possibly after passing
through ClearEmptyURLRef)?  If I'm missing something, then this could use a
comment explaining it.

[Jialiu Lin, 2016/10/18 19:06:04]

Ah, don't need this complex thing anymore. 

+      navigation_handle->HasCommitted()
+          ? SafeBrowsingNavigationObserverManager::ClearEmptyURLRef(
+                navigation_handle->GetRenderFrameHost()->GetLastCommittedURL())
+          : nav_event->has_server_redirect ? nav_event->server_redirect_url
+                                           : nav_event->target_url;
+  manager_->RecordNavigationEvent(nav_event_key, nav_event);
+  navigation_handle_map_.erase(navigation_handle);
+}
+
+void SafeBrowsingNavigationObserver::DidGetResourceResponseStart(
+    const content::ResourceRequestDetails& details) {
+  // We only care about main frame and sub frame.
+  if (details.resource_type != content::RESOURCE_TYPE_MAIN_FRAME &&
+      details.resource_type != content::RESOURCE_TYPE_SUB_FRAME) {
+    return;
+  }
+  if (!details.url.is_valid() || details.socket_address.IsEmpty())
+    return;
+
+  manager_->RecordHostToIpMapping(details.url.host(),
+                                  details.socket_address.host());
+}
+
+void SafeBrowsingNavigationObserver::DidGetUserInteraction(
+    const blink::WebInputEvent::Type type) {
+  last_user_gesture_timestamp_ = base::Time::Now();
+  has_user_gesture_ = true;
+  // TODO (jialiul): Refine user gesture logic when DidOpenRequestedURL
+  // covers all retargetting cases.
+  manager_->RecordUserGestureForWebContents(web_contents(),
+                                            last_user_gesture_timestamp_);
+}
+
+void SafeBrowsingNavigationObserver::WebContentsDestroyed() {
+  manager_->OnWebContentDestroyed(web_contents());
+  web_contents()->RemoveUserData(kWebContentsUserDataKey);
+  // web_contents is null after this function.
+}
+
+}  // namespace safe_browsing