Don't commit the blocked URL when a frame is blocked by XFrameOptions.

third_party/WebKit/Source/core/loader/DocumentLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 346 matching lines @@
 }
 
 void DocumentLoader::cancelLoadAfterXFrameOptionsOrCSPDenied(const ResourceResponse& response)
 {
     InspectorInstrumentation::continueAfterXFrameOptionsDenied(m_frame, this, mainResourceIdentifier(), response, m_mainResource.get());
 
     setWasBlockedAfterXFrameOptionsOrCSP();
 
     // Pretend that this was an empty HTTP 200 response.
     clearMainResourceHandle();
-    m_response = ResourceResponse(blankURL(), "text/html", 0, nullAtom, String());
+    updateForSameDocumentNavigation(SecurityOrigin::urlWithUniqueSecurityOrigin(), SameDocumentNavigationDefault);

[Charlie Reis, 2016/06/23 05:56:45]

Why same document?  Aren't we leaving the current document and replacing it with
an empty document?

[Mike West, 2016/06/23 06:26:40]

We haven't created the document yet for this response, as this method is called
from `DocumentLoader::receivedResponse`. The committed document is still the
pre-navigation document.

Looking at `updateForSameDocumentNavigation`, it's not actually clear what in
there you need; it seems like it would be enough to replace `m_originalRequest`
and `m_request`'s URLs with the unique URL. Do you need the redirect bits as
well?

[alexmos, 2016/06/23 14:09:45]

I think I do need the redirect bits.  I tried this without them first, modifying
just the m_request url, but that hit a DCHECK in history_service.cc:
https://cs.chromium.org/chromium/src/components/history/core/browser/history_...
So I think there's an expectation that the last thing in the redirects array
will match the committed URL.

I used updateForSameDocumentNavigation since it updates both.  It actually
clears out the redirects array first - not sure whether that's ok (if we arrive
to the blocked page after a bunch of redirects), or whether I should just modify
the URL of the last m_redirectChain entry.

[Mike West, 2016/06/23 15:35:10]

Hrm. Ok. I think I'd prefer for you to just extract the bits that set the URL
and adds it to the redirect list, and add a `TODO(mkwst): Remove this once XFO
moves to the browser. https://crbug.com/555418`. Calling into the other method
seems confusing, given its name.\

[alexmos, 2016/06/23 17:37:33]

Done.  Removed the function call in favor of explicitly updating the URL, and
also changed to update just the last entry in the redirect chain, rather than
clearing the whole chain, which I think is more correct.

+    m_response = ResourceResponse(SecurityOrigin::urlWithUniqueSecurityOrigin(), "text/html", 0, nullAtom, String());
     finishedLoading(monotonicallyIncreasingTime());
 
     return;
 }
 
 void DocumentLoader::responseReceived(Resource* resource, const ResourceResponse& response, PassOwnPtr<WebDataConsumerHandle> handle)
 {
     ASSERT_UNUSED(resource, m_mainResource == resource);
     ASSERT_UNUSED(handle, !handle);
     ASSERT(frame());
@@ skipping 324 matching lines @@
 {
     m_writer = createWriterFor(init, mimeType(), m_writer ? m_writer->encoding() : emptyAtom, true, ForceSynchronousParsing);
     if (!source.isNull())
         m_writer->appendReplacingData(source);
     endWriting(m_writer.get());
 }
 
 DEFINE_WEAK_IDENTIFIER_MAP(DocumentLoader);
 
 } // namespace blink