Split header modification out of willSendRequest

third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp

Index: third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
diff --git a/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp b/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
index f6fef44a73e50c266bc77bb9c52e6373c7e5a500..5d5b32d68feb721336cd1e0a0ccfc89e0138b785 100644
--- a/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
+++ b/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
@@ -494,7 +494,7 @@ bool InspectorNetworkAgent::shouldBlockRequest(const ResourceRequest& request)
 void InspectorNetworkAgent::didBlockRequest(LocalFrame* frame, const ResourceRequest& request, DocumentLoader* loader, const FetchInitiatorInfo& initiatorInfo, ResourceRequestBlockedReason reason)
 {
     unsigned long identifier = createUniqueIdentifier();
-    willSendRequestInternal(frame, identifier, loader, request, ResourceResponse(), initiatorInfo);
+    willSendRequest(frame, identifier, loader, request, ResourceResponse(), initiatorInfo);
 
     String requestId = IdentifiersFactory::requestId(identifier);
     String protocolReason = buildBlockedReason(reason);
@@ -507,8 +507,15 @@ void InspectorNetworkAgent::didChangeResourcePriority(unsigned long identifier,
     frontend()->resourceChangedPriority(requestId, resourcePriorityJSON(loadPriority), monotonicallyIncreasingTime());
 }
 
-void InspectorNetworkAgent::willSendRequestInternal(LocalFrame* frame, unsigned long identifier, DocumentLoader* loader, const ResourceRequest& request, const ResourceResponse& redirectResponse, const FetchInitiatorInfo& initiatorInfo)
+void InspectorNetworkAgent::willSendRequest(LocalFrame* frame, unsigned long identifier, DocumentLoader* loader, const ResourceRequest& request, const ResourceResponse& redirectResponse, const FetchInitiatorInfo& initiatorInfo)
 {
+    // Ignore the request initiated internally.
+    if (initiatorInfo.name == FetchInitiatorTypeNames::internal)
+        return;
+
+    if (initiatorInfo.name == FetchInitiatorTypeNames::document && loader->substituteData().isValid())
+        return;
+
     String requestId = IdentifiersFactory::requestId(identifier);
     String loaderId = IdentifiersFactory::loaderId(loader);
     m_resourcesData->resourceCreated(requestId, loaderId, request.url());
@@ -540,15 +547,8 @@ void InspectorNetworkAgent::willSendRequestInternal(LocalFrame* frame, unsigned
         frontend()->flush();
 }
 
-void InspectorNetworkAgent::willSendRequest(LocalFrame* frame, unsigned long identifier, DocumentLoader* loader, ResourceRequest& request, const ResourceResponse& redirectResponse, const FetchInitiatorInfo& initiatorInfo)
+void InspectorNetworkAgent::prepareRequest(DocumentLoader* loader, ResourceRequest& request)
 {
-    // Ignore the request initiated internally.

[hiroshige, 2016/08/04 09:51:14]

Don't we need these |if| statements in InspectorNetworkAgent::prepareRequest()?

[Nate Chapin, 2016/08/04 21:21:55]

Dropping the subsituteData() check is benign, since this code just sets HTTP
headers that will be ignored for a substituteData() load.

I'm not sure about the internal check.

-    if (initiatorInfo.name == FetchInitiatorTypeNames::internal)
-        return;
-
-    if (initiatorInfo.name == FetchInitiatorTypeNames::document && loader->substituteData().isValid())
-        return;
-
     protocol::DictionaryValue* headers = m_state->getObject(NetworkAgentState::extraRequestHeaders);
     if (headers) {
         for (size_t i = 0; i < headers->size(); ++i) {
@@ -568,8 +568,6 @@ void InspectorNetworkAgent::willSendRequest(LocalFrame* frame, unsigned long ide
     if (m_state->booleanProperty(NetworkAgentState::bypassServiceWorker, false))
         request.setSkipServiceWorker(WebURLRequest::SkipServiceWorker::All);
 
-    willSendRequestInternal(frame, identifier, loader, request, redirectResponse, initiatorInfo);
-
     if (!m_hostId.isEmpty())
         request.addHTTPHeaderField(HTTPNames::X_DevTools_Emulate_Network_Conditions_Client_Id, AtomicString(m_hostId));

[pfeldman, 2016/08/04 21:39:59]

Do you mean that these are now getting reported over the protocol? We should
move it into the willSendRequest intead after reporting to the front-end (to the
line 548).

[Nate Chapin, 2016/08/04 21:46:49]

I think that's what this would mean, yeah. Will move.

[Nate Chapin, 2016/08/04 22:51:31]

So, my theory had been that willSendRequest would take a const ResourceRequest,
because this is exactly the request that blink is sending to the browser
process. Would it suffice to filter this header out when constructing the
protocol::Network::Headers in buildObjectForHeaders(), instead of delaying
setting the header until after requestWillBeSent()?

I've updated with the filter, let me know what you think.

 }