Refactor InspectorNetworkAgent callbacks from ThreadableLoader and its clients

State is stored when starting certain ThreadableLoaderClients, to be
used when the DocumentThreadableLoader is constructed. In any case
where a DocumentThreadableLoader begins a fetch, WillSendRequest() will
be called, so perfrom this work in WillSendRequest() and reduce the
number of calls to InspectorNetworkAgent

Change-Id: Ia8a5e130df73b0022755ddcde9bcbb754ceaca3c
Reviewed-on: https://chromium-review.googlesource.com/1070617Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561906}

third_party/WebKit/LayoutTests/http/tests/devtools/network/network-worker-fetch-blocked-expected.txt

@@ -5,6 +5,6 @@ http://127.0.0.1:8000/devtools/network/resources/fetch-worker.js
 resource.type: other
 request.failed: false
 http://127.0.0.1:8000/devtools/network/resources/resource.php
-resource.type: other
+resource.type: fetch
 request.failed: true
 

third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-network-fetch-blocked-expected.txt

@@ -5,6 +5,6 @@ http://127.0.0.1:8000/devtools/service-workers/resources/network-fetch-worker.js
 resource.type: other
 request.failed: false
 http://127.0.0.1:8000/devtools/network/resources/resource.php
-resource.type: other
+resource.type: fetch
 request.failed: true
 

third_party/blink/renderer/core/fetch/fetch_manager.cc

@@ -690,7 +690,6 @@ void FetchManager::Loader::Start() {
 }
 
 void FetchManager::Loader::Dispose() {
-  probe::detachClientRequest(execution_context_, this);
   // Prevent notification
   fetch_manager_ = nullptr;
   if (threadable_loader_) {

third_party/blink/renderer/core/inspector/inspector_network_agent.cc

@@ -774,7 +774,8 @@ void InspectorNetworkAgent::WillSendRequestInternal(
   if (initiator_info.name == FetchInitiatorTypeNames::xmlhttprequest)
     type = InspectorPageAgent::kXHRResource;
 
-  resources_data_->SetResourceType(request_id, type);
+  resources_data_->SetResourceType(
+      request_id, pending_request_ ? pending_request_type_ : type);
 
   if (is_navigation)
     return;
@@ -817,8 +818,15 @@ void InspectorNetworkAgent::WillSendRequestInternal(
       CurrentTimeTicksInSeconds(), CurrentTime(), std::move(initiator_object),
       BuildObjectForResourceResponse(redirect_response), resource_type,
       std::move(maybe_frame_id), request.HasUserGesture());
-  if (pending_xhr_replay_data_ && !pending_xhr_replay_data_->Async())
-    GetFrontend()->flush();
+
+  if (pending_xhr_replay_data_) {
+    resources_data_->SetXHRReplayData(request_id,
+                                      pending_xhr_replay_data_.Get());
+    if (!pending_xhr_replay_data_->Async())
+      GetFrontend()->flush();
+    pending_xhr_replay_data_.Clear();
+  }
+  pending_request_ = nullptr;
 }
 
 void InspectorNetworkAgent::WillSendRequest(
@@ -1077,51 +1085,12 @@ void InspectorNetworkAgent::DidReceiveScriptResponse(unsigned long identifier) {
       InspectorPageAgent::kScriptResource);
 }
 
-void InspectorNetworkAgent::ClearPendingRequestData() {
-  if (pending_request_type_ == InspectorPageAgent::kXHRResource)
-    pending_xhr_replay_data_.Clear();
-  pending_request_ = nullptr;
-}
-
 // static
 bool InspectorNetworkAgent::IsNavigation(DocumentLoader* loader,
                                          unsigned long identifier) {
   return loader && loader->MainResourceIdentifier() == identifier;
 }
 
-void InspectorNetworkAgent::DocumentThreadableLoaderStartedLoadingForClient(
-    unsigned long identifier,
-    ThreadableLoaderClient* client) {
-  if (!client)
-    return;
-  if (client != pending_request_) {
-    DCHECK(!pending_request_);
-    return;
-  }
-
-  String request_id = IdentifiersFactory::SubresourceRequestId(identifier);
-  resources_data_->SetResourceType(request_id, pending_request_type_);
-  if (pending_request_type_ == InspectorPageAgent::kXHRResource) {
-    resources_data_->SetXHRReplayData(request_id,
-                                      pending_xhr_replay_data_.Get());
-  }
-
-  ClearPendingRequestData();
-}
-
-void InspectorNetworkAgent::
-    DocumentThreadableLoaderFailedToStartLoadingForClient(
-        ThreadableLoaderClient* client) {
-  if (!client)
-    return;
-  if (client != pending_request_) {
-    DCHECK(!pending_request_);
-    return;
-  }
-
-  ClearPendingRequestData();
-}
-
 void InspectorNetworkAgent::WillLoadXHR(XMLHttpRequest* xhr,
                                         ThreadableLoaderClient* client,
                                         const AtomicString& method,
@@ -1139,14 +1108,6 @@ void InspectorNetworkAgent::WillLoadXHR(XMLHttpRequest* xhr,
     pending_xhr_replay_data_->AddHeader(header.key, header.value);
 }
 
-void InspectorNetworkAgent::DelayedRemoveReplayXHR(XMLHttpRequest* xhr) {
-  if (!replay_xhrs_.Contains(xhr))
-    return;
-  replay_xhrs_to_be_deleted_.insert(xhr);
-  replay_xhrs_.erase(xhr);
-  remove_finished_replay_xhr_timer_.StartOneShot(TimeDelta(), FROM_HERE);
-}
-
 void InspectorNetworkAgent::DidFailXHRLoading(ExecutionContext* context,
                                               XMLHttpRequest* xhr,
                                               ThreadableLoaderClient* client,
@@ -1169,11 +1130,13 @@ void InspectorNetworkAgent::DidFinishXHRInternal(ExecutionContext* context,
                                                  const AtomicString& method,
                                                  const String& url,
                                                  bool success) {
-  ClearPendingRequestData();
-
   // This method will be called from the XHR.
   // We delay deleting the replay XHR, as deleting here may delete the caller.
-  DelayedRemoveReplayXHR(xhr);
+  if (!replay_xhrs_.Contains(xhr))
+    return;
+  replay_xhrs_to_be_deleted_.insert(xhr);
+  replay_xhrs_.erase(xhr);
+  remove_finished_replay_xhr_timer_.StartOneShot(TimeDelta(), FROM_HERE);
 }
 
 void InspectorNetworkAgent::WillStartFetch(ThreadableLoaderClient* client) {
@@ -1200,24 +1163,6 @@ void InspectorNetworkAgent::WillDispatchEventSourceEvent(
       data);
 }
 
-void InspectorNetworkAgent::DidFinishEventSourceRequest(
-    ThreadableLoaderClient* event_source) {
-  ClearPendingRequestData();
-}
-
-void InspectorNetworkAgent::DetachClientRequest(
-    ThreadableLoaderClient* client) {
-  // This method is called by loader clients when finalizing
-  // (i.e., from their "prefinalizers".) The client reference must
-  // no longer be held onto upon completion.
-  if (pending_request_ == client) {
-    pending_request_ = nullptr;
-    if (pending_request_type_ == InspectorPageAgent::kXHRResource) {
-      pending_xhr_replay_data_.Clear();
-    }
-  }
-}
-
 void InspectorNetworkAgent::ApplyUserAgentOverride(String* user_agent) {
   String user_agent_override;
   state_->getString(NetworkAgentState::kUserAgentOverride,

third_party/blink/renderer/core/inspector/inspector_network_agent.h

@@ -125,10 +125,6 @@ class CORE_EXPORT InspectorNetworkAgent final
   void ShouldBlockRequest(const KURL&, bool* result);
   void ShouldBypassServiceWorker(bool* result);
 
-  void DocumentThreadableLoaderStartedLoadingForClient(unsigned long identifier,
-                                                       ThreadableLoaderClient*);
-  void DocumentThreadableLoaderFailedToStartLoadingForClient(
-      ThreadableLoaderClient*);
   void WillLoadXHR(XMLHttpRequest*,
                    ThreadableLoaderClient*,
                    const AtomicString& method,
@@ -154,10 +150,6 @@ class CORE_EXPORT InspectorNetworkAgent final
                                     const AtomicString& event_name,
                                     const AtomicString& event_id,
                                     const String& data);
-  void DidFinishEventSourceRequest(ThreadableLoaderClient*);
-
-  // Detach and remove all references to the given client.
-  void DetachClientRequest(ThreadableLoaderClient*);
 
   void WillDestroyResource(Resource*);
 
@@ -253,7 +245,6 @@ class CORE_EXPORT InspectorNetworkAgent final
                                const ResourceResponse& redirect_response,
                                const FetchInitiatorInfo&,
                                InspectorPageAgent::ResourceType);
-  void DelayedRemoveReplayXHR(XMLHttpRequest*);
   void RemoveFinishedReplayXHRFired(TimerBase*);
   void DidFinishXHRInternal(ExecutionContext*,
                             XMLHttpRequest*,
@@ -265,7 +256,6 @@ class CORE_EXPORT InspectorNetworkAgent final
   bool CanGetResponseBodyBlob(const String& request_id);
   void GetResponseBodyBlob(const String& request_id,
                            std::unique_ptr<GetResponseBodyCallback>);
-  void ClearPendingRequestData();
 
   static std::unique_ptr<protocol::Network::Initiator> BuildInitiatorObject(
       Document*,
@@ -282,7 +272,7 @@ class CORE_EXPORT InspectorNetworkAgent final
 
   // Stores the pending ThreadableLoaderClient till an identifier for
   // the load is generated by the loader and passed to the inspector
-  // via the documentThreadableLoaderStartedLoadingForClient() method.
+  // via the WillSendRequest() method.
   ThreadableLoaderClient* pending_request_;
   InspectorPageAgent::ResourceType pending_request_type_;
 

third_party/blink/renderer/core/loader/document_threadable_loader.cc

@@ -309,8 +309,6 @@ void DocumentThreadableLoader::Start(const ResourceRequest& request) {
   // corresponds to this line, but divert |cors_flag_| here for convenience.
   if (cors_flag_ && request.GetFetchRequestMode() ==
                         network::mojom::FetchRequestMode::kSameOrigin) {
-    probe::documentThreadableLoaderFailedToStartLoadingForClient(
-        GetExecutionContext(), client_);
     ThreadableLoaderClient* client = client_;
     Clear();
     ResourceError error = ResourceError::CancelledDueToAccessCheckError(
@@ -434,8 +432,6 @@ void DocumentThreadableLoader::MakeCrossOriginAccessRequest(
   // send a request, preflighted or not, that's guaranteed to be denied.
   if (!SchemeRegistry::ShouldTreatURLSchemeAsCORSEnabled(
           request.Url().Protocol())) {
-    probe::documentThreadableLoaderFailedToStartLoadingForClient(
-        GetExecutionContext(), client_);
     DispatchDidFailAccessControlCheck(
         ResourceError::CancelledDueToAccessCheckError(
             request.Url(), ResourceRequestBlockedReason::kOther,
@@ -1166,15 +1162,6 @@ void DocumentThreadableLoader::LoadRequestAsync(
     RawResource::Fetch(new_params, fetcher, this);
   }
   checker_.WillAddClient();
-
-  if (GetResource()->IsLoading()) {
-    unsigned long identifier = GetResource()->Identifier();
-    probe::documentThreadableLoaderStartedLoadingForClient(
-        GetExecutionContext(), identifier, client_);
-  } else {
-    probe::documentThreadableLoaderFailedToStartLoadingForClient(
-        GetExecutionContext(), client_);
-  }
 }
 
 void DocumentThreadableLoader::LoadRequestSync(
@@ -1193,8 +1180,6 @@ void DocumentThreadableLoader::LoadRequestSync(
       fetch_params, loading_context_->GetResourceFetcher());
   ResourceResponse response = resource->GetResponse();
   unsigned long identifier = resource->Identifier();
-  probe::documentThreadableLoaderStartedLoadingForClient(GetExecutionContext(),
-                                                         identifier, client_);
   ThreadableLoaderClient* client = client_;
   const KURL& request_url = request.Url();
 

third_party/blink/renderer/core/probe/CoreProbes.json5

@@ -104,7 +104,6 @@
     InspectorNetworkAgent: {
       probes: [
         "applyUserAgentOverride",
-        "detachClientRequest",
         "didBlockRequest",
         "didChangeResourcePriority",
         "didCloseWebSocket",
@@ -112,7 +111,6 @@
         "didCreateWebSocket",
         "didFailLoading",
         "didFailXHRLoading",
-        "didFinishEventSourceRequest",
         "didFinishLoading",
         "didFinishXHRLoading",
         "didReceiveCORSRedirectResponse",
@@ -125,8 +123,6 @@
         "didReceiveWebSocketFrameError",
         "didReceiveWebSocketHandshakeResponse",
         "didSendWebSocketFrame",
-        "documentThreadableLoaderFailedToStartLoadingForClient",
-        "documentThreadableLoaderStartedLoadingForClient",
         "frameClearedScheduledClientNavigation",
         "frameClearedScheduledNavigation",
         "frameScheduledClientNavigation",

third_party/blink/renderer/core/probe/CoreProbes.pidl

@@ -101,17 +101,12 @@ interface CoreProbes {
   void didFinishLoading(ExecutionContext*, unsigned long identifier, DocumentLoader*, TimeTicks finishTime, int64_t encoded_data_length, int64_t decodedBodyLength, bool blocked_cross_site_document);
   void didReceiveCORSRedirectResponse(ExecutionContext*, unsigned long identifier, DocumentLoader*, const ResourceResponse&, Resource*);
   void didFailLoading(ExecutionContext*, unsigned long identifier, DocumentLoader*, const ResourceError&);
-  void documentThreadableLoaderStartedLoadingForClient(ExecutionContext*, unsigned long identifier, ThreadableLoaderClient* client);
-  void documentThreadableLoaderFailedToStartLoadingForClient(ExecutionContext*, ThreadableLoaderClient* client);
   void willSendEventSourceRequest(ExecutionContext*, ThreadableLoaderClient* eventSource);
   void willDispatchEventSourceEvent(ExecutionContext*, unsigned long identifier, const AtomicString& eventName, const AtomicString& eventId, const String& data);
-  void didFinishEventSourceRequest(ExecutionContext*, ThreadableLoaderClient* eventSource);
   void willLoadXHR(ExecutionContext*, XMLHttpRequest* xhr, ThreadableLoaderClient* client, const AtomicString& method, const KURL& url, bool async, const HTTPHeaderMap& headers, bool includeCredentials);
   void didFailXHRLoading([Keep] ExecutionContext*, XMLHttpRequest* xhr, ThreadableLoaderClient* client, const AtomicString& method, const String& url);
   void didFinishXHRLoading([Keep] ExecutionContext*, XMLHttpRequest* xhr, ThreadableLoaderClient* client, const AtomicString& method, const String& url);
   void willStartFetch(ExecutionContext*, ThreadableLoaderClient*);
-  // Detach and remove all references to the given client.
-  void detachClientRequest(ExecutionContext*, ThreadableLoaderClient*);
   void scriptImported(ExecutionContext*, unsigned long identifier, const String& sourceString);
   void scriptExecutionBlockedByCSP(ExecutionContext*, const String& directiveText);
   void didReceiveScriptResponse(ExecutionContext*, unsigned long identifier);

third_party/blink/renderer/core/xmlhttprequest/xml_http_request.cc

@@ -1180,7 +1180,6 @@ void XMLHttpRequest::abort() {
 }
 
 void XMLHttpRequest::Dispose() {
-  probe::detachClientRequest(GetExecutionContext(), this);
   progress_event_throttle_->Stop();
   InternalAbort();
   // TODO(yhirano): Remove this CHECK: see https://crbug.com/570946.

third_party/blink/renderer/modules/eventsource/event_source.cc

@@ -110,10 +110,6 @@ EventSource::~EventSource() {
   DCHECK(!loader_);
 }
 
-void EventSource::Dispose() {
-  probe::detachClientRequest(GetExecutionContext(), this);
-}
-
 void EventSource::ScheduleInitialConnect() {
   DCHECK_EQ(kConnecting, state_);
   DCHECK(!loader_);
@@ -170,8 +166,6 @@ void EventSource::Connect() {
 }
 
 void EventSource::NetworkRequestEnded() {
-  probe::didFinishEventSourceRequest(GetExecutionContext(), this);
-
   loader_ = nullptr;
 
   if (state_ != kClosed)
@@ -360,7 +354,6 @@ void EventSource::AbortConnectionAttempt() {
 }
 
 void EventSource::ContextDestroyed(ExecutionContext*) {
-  probe::detachClientRequest(GetExecutionContext(), this);
   close();
 }
 

third_party/blink/renderer/modules/eventsource/event_source.h

@@ -59,7 +59,6 @@ class MODULES_EXPORT EventSource final
       public EventSourceParser::Client {
   DEFINE_WRAPPERTYPEINFO();
   USING_GARBAGE_COLLECTED_MIXIN(EventSource);
-  USING_PRE_FINALIZER(EventSource, Dispose);
 
  public:
   static EventSource* Create(ExecutionContext*,
@@ -103,8 +102,6 @@ class MODULES_EXPORT EventSource final
  private:
   EventSource(ExecutionContext*, const KURL&, const EventSourceInit&);
 
-  void Dispose();
-
   void DidReceiveResponse(unsigned long,
                           const ResourceResponse&,
                           std::unique_ptr<WebDataConsumerHandle>) override;