Move handling of invalid referrer to the network delegate

We now always abort invalid loads. According to UMA, they don't happen
anymore, so this should only affect future bugs.

Also, move the UMA reporting to the right thread.

BUG=none
R=mmenke@chromium.org,mef@chromium.org,nasko@chromium.org

Review URL: https://codereview.chromium.org/572273002

Cr-Commit-Position: refs/heads/master@{#295107}

8 files changed, 99 insertions, 8 deletions

diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc
index 77abac0..285a2e3 100644
--- a/chrome/browser/net/chrome_network_delegate.cc
+++ b/chrome/browser/net/chrome_network_delegate.cc
@@ -12,6 +12,7 @@
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/user_metrics.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_member.h"
 #include "base/prefs/pref_service.h"
@@ -233,6 +234,12 @@ void RecordIOThreadToRequestStartOnUIThread(
 }
 #endif  // defined(OS_ANDROID)
 
+void ReportInvalidReferrerSend(const GURL& target_url,
+                               const GURL& referrer_url) {
+  base::RecordAction(
+      base::UserMetricsAction("Net.URLRequest_StartJob_InvalidReferrer"));
+}
+
 }  // namespace
 
 ChromeNetworkDelegate::ChromeNetworkDelegate(
@@ -811,6 +818,15 @@ int ChromeNetworkDelegate::OnBeforeSocketStreamConnect(
   return net::OK;
 }
 
+bool ChromeNetworkDelegate::OnCancelURLRequestWithPolicyViolatingReferrerHeader(
+    const net::URLRequest& request,
+    const GURL& target_url,
+    const GURL& referrer_url) const {
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+      base::Bind(&ReportInvalidReferrerSend, target_url, referrer_url));
+  return true;
+}
+
 void ChromeNetworkDelegate::AccumulateContentLength(
     int64 received_content_length,
     int64 original_content_length,
diff --git a/chrome/browser/net/chrome_network_delegate.h b/chrome/browser/net/chrome_network_delegate.h
index 0431a66..bfe5de7 100644
--- a/chrome/browser/net/chrome_network_delegate.h
+++ b/chrome/browser/net/chrome_network_delegate.h
@@ -261,6 +261,10 @@ class ChromeNetworkDelegate : public net::NetworkDelegate {
   virtual int OnBeforeSocketStreamConnect(
       net::SocketStream* stream,
       const net::CompletionCallback& callback) OVERRIDE;
+  virtual bool OnCancelURLRequestWithPolicyViolatingReferrerHeader(
+      const net::URLRequest& request,
+      const GURL& target_url,
+      const GURL& referrer_url) const OVERRIDE;
 
   void AccumulateContentLength(
       int64 received_payload_byte_count,
diff --git a/net/base/network_delegate.cc b/net/base/network_delegate.cc
index 76d94ab..1c315ec 100644
--- a/net/base/network_delegate.cc
+++ b/net/base/network_delegate.cc
@@ -166,6 +166,15 @@ bool NetworkDelegate::CanEnablePrivacyMode(
   return OnCanEnablePrivacyMode(url, first_party_for_cookies);
 }
 
+bool NetworkDelegate::CancelURLRequestWithPolicyViolatingReferrerHeader(
+    const URLRequest& request,
+    const GURL& target_url,
+    const GURL& referrer_url) const {
+  DCHECK(CalledOnValidThread());
+  return OnCancelURLRequestWithPolicyViolatingReferrerHeader(
+      request, target_url, referrer_url);
+}
+
 int NetworkDelegate::OnBeforeURLRequest(URLRequest* request,
                                         const CompletionCallback& callback,
                                         GURL* new_url) {
@@ -269,4 +278,11 @@ int NetworkDelegate::OnBeforeSocketStreamConnect(
   return OK;
 }
 
+bool NetworkDelegate::OnCancelURLRequestWithPolicyViolatingReferrerHeader(
+    const URLRequest& request,
+    const GURL& target_url,
+    const GURL& referrer_url) const {
+  return false;
+}
+
 }  // namespace net
diff --git a/net/base/network_delegate.h b/net/base/network_delegate.h
index a290d16..299989c 100644
--- a/net/base/network_delegate.h
+++ b/net/base/network_delegate.h
@@ -108,6 +108,11 @@ class NET_EXPORT NetworkDelegate : public base::NonThreadSafe {
   int NotifyBeforeSocketStreamConnect(SocketStream* socket,
                                       const CompletionCallback& callback);
 
+  bool CancelURLRequestWithPolicyViolatingReferrerHeader(
+      const URLRequest& request,
+      const GURL& target_url,
+      const GURL& referrer_url) const;
+
  private:
   // This is the interface for subclasses of NetworkDelegate to implement. These
   // member functions will be called by the respective public notification
@@ -270,6 +275,16 @@ class NET_EXPORT NetworkDelegate : public base::NonThreadSafe {
   // See OnBeforeURLRequest for return value description. Returns OK by default.
   virtual int OnBeforeSocketStreamConnect(
       SocketStream* socket, const CompletionCallback& callback);
+
+  // Called when the |referrer_url| for requesting |target_url| during handling
+  // of the |request| is does not comply with the referrer policy (e.g. a
+  // secure referrer for an insecure initial target).
+  // Returns true if the request should be cancelled. Otherwise, the referrer
+  // header is stripped from the request.
+  virtual bool OnCancelURLRequestWithPolicyViolatingReferrerHeader(
+      const URLRequest& request,
+      const GURL& target_url,
+      const GURL& referrer_url) const;
 };
 
 }  // namespace net
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 15b9ab7..3764488 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -13,7 +13,6 @@
 #include "base/memory/singleton.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/stats_counters.h"
-#include "base/metrics/user_metrics.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
@@ -691,12 +690,21 @@ void URLRequest::StartJob(URLRequestJob* job) {
   if (referrer_policy_ ==
           CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE &&
       GURL(referrer_).SchemeIsSecure() && !url().SchemeIsSecure()) {
-#if !defined(OFFICIAL_BUILD)
-    LOG(FATAL) << "Trying to send secure referrer for insecure load";
-#endif
-    referrer_.clear();
-    base::RecordAction(
-        base::UserMetricsAction("Net.URLRequest_StartJob_InvalidReferrer"));
+    if (!network_delegate_ ||
+        !network_delegate_->CancelURLRequestWithPolicyViolatingReferrerHeader(
+            *this, url(), GURL(referrer_))) {
+      referrer_.clear();
+    } else {
+      // We need to clear the referrer anyway to avoid an infinite recursion
+      // when starting the error job.
+      referrer_.clear();
+      std::string source("delegate");
+      net_log_.AddEvent(NetLog::TYPE_CANCELLED,
+                        NetLog::StringCallback("source", &source));
+      RestartWithJob(new URLRequestErrorJob(
+          this, network_delegate_, ERR_BLOCKED_BY_CLIENT));
+      return;
+    }
   }
 
   // Don't allow errors to be sent from within Start().
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index 2d3f3aa..82d8579 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -320,7 +320,8 @@ TestNetworkDelegate::TestNetworkDelegate()
       has_load_timing_info_before_redirect_(false),
       has_load_timing_info_before_auth_(false),
       can_access_files_(true),
-      can_throttle_requests_(true) {
+      can_throttle_requests_(true),
+      cancel_request_with_policy_violating_referrer_(false) {
 }
 
 TestNetworkDelegate::~TestNetworkDelegate() {
@@ -604,6 +605,13 @@ int TestNetworkDelegate::OnBeforeSocketStreamConnect(
   return OK;
 }
 
+bool TestNetworkDelegate::OnCancelURLRequestWithPolicyViolatingReferrerHeader(
+    const URLRequest& request,
+    const GURL& target_url,
+    const GURL& referrer_url) const {
+  return cancel_request_with_policy_violating_referrer_;
+}
+
 // static
 std::string ScopedCustomUrlRequestTestHttpHost::value_("127.0.0.1");
 
diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h
index 0c342f4..cad472e 100644
--- a/net/url_request/url_request_test_util.h
+++ b/net/url_request/url_request_test_util.h
@@ -269,6 +269,10 @@ class TestNetworkDelegate : public NetworkDelegate {
   void set_can_throttle_requests(bool val) { can_throttle_requests_ = val; }
   bool can_throttle_requests() const { return can_throttle_requests_; }
 
+  void set_cancel_request_with_policy_violating_referrer(bool val) {
+    cancel_request_with_policy_violating_referrer_ = val;
+  }
+
   int observed_before_proxy_headers_sent_callbacks() const {
     return observed_before_proxy_headers_sent_callbacks_;
   }
@@ -324,6 +328,10 @@ class TestNetworkDelegate : public NetworkDelegate {
   virtual int OnBeforeSocketStreamConnect(
       SocketStream* stream,
       const CompletionCallback& callback) OVERRIDE;
+  virtual bool OnCancelURLRequestWithPolicyViolatingReferrerHeader(
+      const URLRequest& request,
+      const GURL& target_url,
+      const GURL& referrer_url) const OVERRIDE;
 
   void InitRequestStatesIfNew(int request_id);
 
@@ -363,6 +371,7 @@ class TestNetworkDelegate : public NetworkDelegate {
 
   bool can_access_files_;  // true by default
   bool can_throttle_requests_;  // true by default
+  bool cancel_request_with_policy_violating_referrer_;  // false by default
 };
 
 // Overrides the host used by the LocalHttpTestServer in
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 4efba54..06e05b9 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -1021,6 +1021,21 @@ TEST_F(URLRequestTest, InvalidUrlTest) {
   }
 }
 
+TEST_F(URLRequestTest, InvalidReferrerTest) {
+  TestURLRequestContext context;
+  TestNetworkDelegate network_delegate;
+  network_delegate.set_cancel_request_with_policy_violating_referrer(true);
+  context.set_network_delegate(&network_delegate);
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(context.CreateRequest(
+      GURL("http://localhost/"), DEFAULT_PRIORITY, &d, NULL));
+  req->SetReferrer("https://somewhere.com/");
+
+  req->Start();
+  base::RunLoop().Run();
+  EXPECT_TRUE(d.request_failed());
+}
+
 #if defined(OS_WIN)
 TEST_F(URLRequestTest, ResolveShortcutTest) {
   base::FilePath app_path;