Introducing the OffDomainInclusionDetector to analyze resource requests for suspicious activity.

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

Index: chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
diff --git a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
index fd8b196f376443176b1b5d0926e14f2624a62292..70d7aa466a1539962d5958d4ab9471d6e365f403 100644
--- a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
+++ b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
@@ -307,6 +307,9 @@ void ChromeResourceDispatcherHostDelegate::RequestBeginning(
     content::AppCacheService* appcache_service,
     ResourceType resource_type,
     ScopedVector<content::ResourceThrottle>* throttles) {
+  if (safe_browsing_)
+    safe_browsing_->OnResourceRequest(request);

[Nico, 2014/11/14 19:52:58]

nit: if someone helpfully deletes this in the future, I don't think any tests
would fail…

[gab, 2014/11/18 17:23:19]

Right, but this class itself is highly undertested already (it only has 3 smalls
tests in chrome_resource_dispatcher_host_delegate_browsertest.cc which appear to
be way too little to provide significant coverage of the logic in this class).

The simple addition in this CL could be unit tested easily if the dependencies
were injected (rather than obtained from g_browser_process) and could be mocked.
But it seems out-of-scope for this CL to modify dependency injection and
introduce unit tests for this class just for this.

I could also add a browser test which is a bit simpler (doesn't need a full
class overhaul as the real dependencies can be used), and simply hook a
TestSafeBrowsingService (just like the current TestDispatcherHostDelegate in
ChromeResourceDispatcherHostDelegateBrowserTest).
But a browser test seems overkill to test this...

In short, I don't think there is a good testing solution for this code as-is and
I think many many things in this class could use tests before this code is the
most important to test here.

Once the OffDomainInclusionDetector is fully hooked to provide incident reports,
I will add integration tests which will exercise this code (and before then I
will definitely notice if this line goes away as I'll be using this code daily).

+
   const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request);
   bool is_prerendering =
       info->GetVisibilityState() == blink::WebPageVisibilityStatePrerender;