Split header modification out of willSendRequest

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

 /*
  * 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 476 matching lines @@
         auto entry = blockedURLs->at(i);
         if (matches(url, entry.first))
             return true;
     }
     return false;
 }
 
 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);
     frontend()->loadingFailed(requestId, monotonicallyIncreasingTime(), InspectorPageAgent::resourceTypeJson(m_resourcesData->resourceType(requestId)), String(), false, protocolReason);
 }
 
 void InspectorNetworkAgent::didChangeResourcePriority(unsigned long identifier, ResourceLoadPriority loadPriority)
 {
     String requestId = IdentifiersFactory::requestId(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());
 
     InspectorPageAgent::ResourceType type = InspectorPageAgent::OtherResource;
     if (initiatorInfo.name == FetchInitiatorTypeNames::xmlhttprequest) {
         type = InspectorPageAgent::XHRResource;
         m_resourcesData->setResourceType(requestId, type);
     } else if (initiatorInfo.name == FetchInitiatorTypeNames::document) {
         type = InspectorPageAgent::DocumentResource;
@@ skipping 11 matching lines @@
     std::unique_ptr<protocol::Network::Request> requestInfo(buildObjectForResourceRequest(request));
 
     requestInfo->setMixedContentType(mixedContentTypeForContextType(MixedContentChecker::contextTypeForInspector(frame, request)));
 
     String resourceType = InspectorPageAgent::resourceTypeJson(type);
     frontend()->requestWillBeSent(requestId, frameId, loaderId, urlWithoutFragment(loader->url()).getString(), std::move(requestInfo), monotonicallyIncreasingTime(), currentTime(), std::move(initiatorObject), buildObjectForResourceResponse(redirectResponse), resourceType);
     if (m_pendingXHRReplayData && !m_pendingXHRReplayData->async())
         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) {
             auto header = headers->at(i);
             String16 value;
             if (header.second->asString(&value))
                 request.setHTTPHeaderField(AtomicString(header.first), AtomicString(String(value)));
         }
     }
 
     request.setReportRawHeaders(true);
 
     if (m_state->booleanProperty(NetworkAgentState::cacheDisabled, false)) {
         request.setCachePolicy(WebCachePolicy::BypassingCache);
         request.setShouldResetAppCache(true);
     }
     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.

 }
 
 void InspectorNetworkAgent::markResourceAsCached(unsigned long identifier)
 {
     frontend()->requestServedFromCache(IdentifiersFactory::requestId(identifier));
 }
 
 void InspectorNetworkAgent::didReceiveResourceResponse(LocalFrame* frame, unsigned long identifier, DocumentLoader* loader, const ResourceResponse& response, Resource* cachedResource)
 {
     String requestId = IdentifiersFactory::requestId(identifier);
@@ skipping 606 matching lines @@
     , m_removeFinishedReplayXHRTimer(this, &InspectorNetworkAgent::removeFinishedReplayXHRFired)
 {
 }
 
 bool InspectorNetworkAgent::shouldForceCORSPreflight()
 {
     return m_state->booleanProperty(NetworkAgentState::cacheDisabled, false);
 }
 
 } // namespace blink