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}

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,

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,

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

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

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
+    if (!network_delegate_ ||
+        !network_delegate_->CancelURLRequestWithPolicyViolatingReferrerHeader(
+            *this, url(), GURL(referrer_))) {
       referrer_.clear();
-    base::RecordAction(
-        base::UserMetricsAction("Net.URLRequest_StartJob_InvalidReferrer"));
+    } 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().

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");
 

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

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;