Move and update WebLoaderStrategy logging statement
authorkrollin@apple.com <krollin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Apr 2017 21:51:27 +0000 (21:51 +0000)
committerkrollin@apple.com <krollin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Apr 2017 21:51:27 +0000 (21:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170140

Reviewed by Alex Christensen.

WebLoaderStrategy::scheduleLoad has a logging statement that says, in
part: "Resource has been queued for scheduling with the
NetworkProcess". This statement is emitted after the
ScheduleResourceLoad message has been successfully sent to the
NetworkProcess. The logging statement was added at this location to
indicate that the resource-load had been successfully handed off; it
pairs a similar logging statement that is emitted if the sending of
the ScheduleResourceLoad message fails.

I think it would be better to move this logging statement before the
ScheduleResourceLoad message is sent to the NetworkProcess (and change
its wording to "Resource is being scheduled with the NetworkProcess").
The reason for this change is to help make sure that the sequence of
logging statements is more deterministic. In the current form, the
message "Resource has been queued for scheduling with the
NetworkProcess" normally appears before any NetworkProcess logging
statements that indicate that the resource-loading is continuing
there, but in rare occasions the logging statements can be reversed.
This change in the ordering of the statements has caused a problem in
a script I've written that examines the resource-loading process and
looks for errors. By ensuring that the WebLoaderStrategy statement
always appears before the NetworkResourceLoader statement, the flow
makes better sense and the script can be more robust.

In making this change, we are probably not giving up any assurance
that the ScheduleResourceLoad message has been sent to the
NetworkResourceLoader. If the message is successfully sent, we'll see
logging in the NetworkProcess. If the message has not been sent, we'll
see WebLoaderStrategy logging an error.

* WebProcess/Network/WebLoaderStrategy.cpp:
(WebKit::WebLoaderStrategy::scheduleLoad):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215433 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Network/WebLoaderStrategy.cpp

index 7828a5c..c5e01a6 100644 (file)
@@ -1,3 +1,43 @@
+2017-04-17  Keith Rollin  <krollin@apple.com>
+
+        Move and update WebLoaderStrategy logging statement
+        https://bugs.webkit.org/show_bug.cgi?id=170140
+
+        Reviewed by Alex Christensen.
+
+        WebLoaderStrategy::scheduleLoad has a logging statement that says, in
+        part: "Resource has been queued for scheduling with the
+        NetworkProcess". This statement is emitted after the
+        ScheduleResourceLoad message has been successfully sent to the
+        NetworkProcess. The logging statement was added at this location to
+        indicate that the resource-load had been successfully handed off; it
+        pairs a similar logging statement that is emitted if the sending of
+        the ScheduleResourceLoad message fails.
+
+        I think it would be better to move this logging statement before the
+        ScheduleResourceLoad message is sent to the NetworkProcess (and change
+        its wording to "Resource is being scheduled with the NetworkProcess").
+        The reason for this change is to help make sure that the sequence of
+        logging statements is more deterministic. In the current form, the
+        message "Resource has been queued for scheduling with the
+        NetworkProcess" normally appears before any NetworkProcess logging
+        statements that indicate that the resource-loading is continuing
+        there, but in rare occasions the logging statements can be reversed.
+        This change in the ordering of the statements has caused a problem in
+        a script I've written that examines the resource-loading process and
+        looks for errors. By ensuring that the WebLoaderStrategy statement
+        always appears before the NetworkResourceLoader statement, the flow
+        makes better sense and the script can be more robust.
+
+        In making this change, we are probably not giving up any assurance
+        that the ScheduleResourceLoad message has been sent to the
+        NetworkResourceLoader. If the message is successfully sent, we'll see
+        logging in the NetworkProcess. If the message has not been sent, we'll
+        see WebLoaderStrategy logging an error.
+
+        * WebProcess/Network/WebLoaderStrategy.cpp:
+        (WebKit::WebLoaderStrategy::scheduleLoad):
+
 2017-04-17  Anders Carlsson  <andersca@apple.com>
 
         Stop using deprecated APIs, part 1
index fbd3dc8..f372d74 100644 (file)
@@ -230,6 +230,7 @@ void WebLoaderStrategy::scheduleLoad(ResourceLoader& resourceLoader, CachedResou
 
     ASSERT((loadParameters.webPageID && loadParameters.webFrameID) || loadParameters.clientCredentialPolicy == ClientCredentialPolicy::CannotAskClientForCredentials);
 
+    RELEASE_LOG_IF_ALLOWED(resourceLoader, "scheduleLoad: Resource is being scheduled with the NetworkProcess (frame = %p, priority = %d, pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", resourceLoader.frame(), static_cast<int>(resourceLoader.request().priority()), loadParameters.webPageID, loadParameters.webFrameID, loadParameters.identifier);
     if (!WebProcess::singleton().networkConnection().connection().send(Messages::NetworkConnectionToWebProcess::ScheduleResourceLoad(loadParameters), 0)) {
         RELEASE_LOG_ERROR_IF_ALLOWED(resourceLoader, "scheduleLoad: Unable to schedule resource with the NetworkProcess (frame = %p, priority = %d, pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", resourceLoader.frame(), static_cast<int>(resourceLoader.request().priority()), loadParameters.webPageID, loadParameters.webFrameID, loadParameters.identifier);
         // We probably failed to schedule this load with the NetworkProcess because it had crashed.
@@ -238,9 +239,7 @@ void WebLoaderStrategy::scheduleLoad(ResourceLoader& resourceLoader, CachedResou
         return;
     }
 
-    auto webResourceLoader = WebResourceLoader::create(resourceLoader, trackingParameters);
-    RELEASE_LOG_IF_ALLOWED(resourceLoader, "scheduleLoad: Resource has been queued for scheduling with the NetworkProcess (frame = %p, priority = %d, pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", WebResourceLoader = %p)", resourceLoader.frame(), static_cast<int>(resourceLoader.request().priority()), loadParameters.webPageID, loadParameters.webFrameID, loadParameters.identifier, webResourceLoader.ptr());
-    m_webResourceLoaders.set(identifier, WTFMove(webResourceLoader));
+    m_webResourceLoaders.set(identifier, WebResourceLoader::create(resourceLoader, trackingParameters));
 }
 
 void WebLoaderStrategy::scheduleInternallyFailedLoad(WebCore::ResourceLoader& resourceLoader)