[webkit-changes] [261249] trunk/Source/WebKit

Wed, 06 May 2020 13:35:24 -0700

Title: [261249] trunk/Source/WebKit
Revision
261249
Author
krol...@apple.com
Date
2020-05-06 13:35:11 -0700 (Wed, 06 May 2020)

Log Message

Better surfacing of the presenting parent PID in the Network process
https://bugs.webkit.org/show_bug.cgi?id=211495
<rdar://problem/62917205>

Reviewed by Youenn Fablet.

In Bug 205295, NetworkResourceLoader logging was improved, with one of
the changes being that the PID of parent process that invoked the
Network process was logged at the start of
NetworkResourceLoader::start(). However, there was an execution path
that skipped calling start()
(NetworkConnectionToWebProcess::scheduleResourceLoad ->
NetworkResourceLoader::startWithServiceWorker ->
serviceWorkerDidNotHandle -> restartNetworkLoad -> startNetworkLoad),
and so the the logging would not show the parent PID. This logging is
needed for diagnostic purposes, so tweak the logging a little bit more
in order to ensure we emit it. In this change, logging is added to
various loading-related NetworkConnectionToWebProcess entry points
(not just scheduleResourceLoad) in order to (a) ensure we emit the
logging we want and (b) to give a clearer picture of what operations
are being invoked in the Network process.

No new tests -- no new or changed functionality.

* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::hasUploadStateChanged):
(WebKit::NetworkConnectionToWebProcess::resolveBlobReferences):
(WebKit::NetworkConnectionToWebProcess::scheduleResourceLoad):
(WebKit::NetworkConnectionToWebProcess::performSynchronousLoad):
(WebKit::NetworkConnectionToWebProcess::loadPing):
(WebKit::NetworkConnectionToWebProcess::preconnectTo):
(WebKit::NetworkConnectionToWebProcess::serverToContextConnectionNoLongerNeeded):
* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::start):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (261248 => 261249)


--- trunk/Source/WebKit/ChangeLog	2020-05-06 20:11:39 UTC (rev 261248)
+++ trunk/Source/WebKit/ChangeLog	2020-05-06 20:35:11 UTC (rev 261249)
@@ -1,3 +1,40 @@
+2020-05-06  Keith Rollin  <krol...@apple.com>
+
+        Better surfacing of the presenting parent PID in the Network process
+        https://bugs.webkit.org/show_bug.cgi?id=211495
+        <rdar://problem/62917205>
+
+        Reviewed by Youenn Fablet.
+
+        In Bug 205295, NetworkResourceLoader logging was improved, with one of
+        the changes being that the PID of parent process that invoked the
+        Network process was logged at the start of
+        NetworkResourceLoader::start(). However, there was an execution path
+        that skipped calling start()
+        (NetworkConnectionToWebProcess::scheduleResourceLoad ->
+        NetworkResourceLoader::startWithServiceWorker ->
+        serviceWorkerDidNotHandle -> restartNetworkLoad -> startNetworkLoad),
+        and so the the logging would not show the parent PID. This logging is
+        needed for diagnostic purposes, so tweak the logging a little bit more
+        in order to ensure we emit it. In this change, logging is added to
+        various loading-related NetworkConnectionToWebProcess entry points
+        (not just scheduleResourceLoad) in order to (a) ensure we emit the
+        logging we want and (b) to give a clearer picture of what operations
+        are being invoked in the Network process.
+
+        No new tests -- no new or changed functionality.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+        (WebKit::NetworkConnectionToWebProcess::hasUploadStateChanged):
+        (WebKit::NetworkConnectionToWebProcess::resolveBlobReferences):
+        (WebKit::NetworkConnectionToWebProcess::scheduleResourceLoad):
+        (WebKit::NetworkConnectionToWebProcess::performSynchronousLoad):
+        (WebKit::NetworkConnectionToWebProcess::loadPing):
+        (WebKit::NetworkConnectionToWebProcess::preconnectTo):
+        (WebKit::NetworkConnectionToWebProcess::serverToContextConnectionNoLongerNeeded):
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::start):
+
 2020-05-06  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Cut and paste from Google Doc to Notes in several (non-Latin) languages doesn't work

Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (261248 => 261249)


--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp	2020-05-06 20:11:39 UTC (rev 261248)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp	2020-05-06 20:35:11 UTC (rev 261249)
@@ -81,7 +81,7 @@
 #endif
 
 #undef RELEASE_LOG_IF_ALLOWED
-#define RELEASE_LOG_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_IF(m_sessionID.isAlwaysOnLoggingAllowed(), channel, "%p - NetworkConnectionToWebProcess::" fmt, this, ##__VA_ARGS__)
+#define RELEASE_LOG_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_IF(m_sessionID.isAlwaysOnLoggingAllowed(), channel, "%p - [webProcessIdentifier=%" PRIu64 "] NetworkConnectionToWebProcess::" fmt, this, webProcessIdentifier().toUInt64(), ##__VA_ARGS__)
 
 #define NETWORK_PROCESS_MESSAGE_CHECK(assertion) NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(assertion, (void)0)
 #define NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(assertion, completion) do { \
@@ -154,7 +154,7 @@
 
 void NetworkConnectionToWebProcess::hasUploadStateChanged(bool hasUpload)
 {
-    RELEASE_LOG_IF_ALLOWED(Loading, "hasUploadStateChanged - WebProcess %llu - hasUpload: %d", webProcessIdentifier().toUInt64(), hasUpload);
+    RELEASE_LOG_IF_ALLOWED(Loading, "hasUploadStateChanged: (hasUpload=%d)", hasUpload);
     m_networkProcess->parentProcessConnection()->send(Messages::NetworkProcessProxy::SetWebProcessHasUploads(m_webProcessIdentifier, hasUpload), 0);
 }
 
@@ -409,8 +409,10 @@
     return networkProcess().networkSession(m_sessionID);
 }
 
-Vector<RefPtr<WebCore::BlobDataFileReference>> NetworkConnectionToWebProcess::resolveBlobReferences(const NetworkResourceLoadParameters& parameters)
+Vector<RefPtr<WebCore::BlobDataFileReference>> NetworkConnectionToWebProcess::resolveBlobReferences(const NetworkResourceLoadParameters& loadParameters)
 {
+    RELEASE_LOG_IF_ALLOWED(Loading, "resolveBlobReferences: (parentPID=%d, pageProxyID=%" PRIu64 ", webPageID=%" PRIu64 ", frameID=%" PRIu64 ", resourceID=%" PRIu64 ")", loadParameters.parentPID, loadParameters.webPageProxyID.toUInt64(), loadParameters.webPageID.toUInt64(), loadParameters.webFrameID.toUInt64(), loadParameters.identifier);
+
     auto* session = networkSession();
     if (!session)
         return { };
@@ -418,12 +420,12 @@
     auto& blobRegistry = session->blobRegistry();
 
     Vector<RefPtr<WebCore::BlobDataFileReference>> files;
-    if (auto* body = parameters.request.httpBody()) {
+    if (auto* body = loadParameters.request.httpBody()) {
         for (auto& element : body->elements()) {
             if (auto* blobData = WTF::get_if<FormDataElement::EncodedBlobData>(element.data))
                 files.appendVector(blobRegistry.filesInBlob(blobData->url));
         }
-        const_cast<WebCore::ResourceRequest&>(parameters.request).setHTTPBody(body->resolveBlobReferences(&blobRegistry));
+        const_cast<WebCore::ResourceRequest&>(loadParameters.request).setHTTPBody(body->resolveBlobReferences(&blobRegistry));
     }
 
     return files;
@@ -438,6 +440,8 @@
 
 void NetworkConnectionToWebProcess::scheduleResourceLoad(NetworkResourceLoadParameters&& loadParameters)
 {
+    RELEASE_LOG_IF_ALLOWED(Loading, "scheduleResourceLoad: (parentPID=%d, pageProxyID=%" PRIu64 ", webPageID=%" PRIu64 ", frameID=%" PRIu64 ", resourceID=%" PRIu64 ")", loadParameters.parentPID, loadParameters.webPageProxyID.toUInt64(), loadParameters.webPageID.toUInt64(), loadParameters.webFrameID.toUInt64(), loadParameters.identifier);
+
 #if ENABLE(SERVICE_WORKER)
     auto& server = m_networkProcess->swServerForSession(m_sessionID);
     if (!server.isImportCompleted()) {
@@ -459,7 +463,6 @@
 
 #if ENABLE(SERVICE_WORKER)
     loader->startWithServiceWorker();
-    return;
 #else
     loader->start();
 #endif
@@ -467,6 +470,8 @@
 
 void NetworkConnectionToWebProcess::performSynchronousLoad(NetworkResourceLoadParameters&& loadParameters, Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::DelayedReply&& reply)
 {
+    RELEASE_LOG_IF_ALLOWED(Loading, "performSynchronousLoad: (parentPID=%d, pageProxyID=%" PRIu64 ", webPageID=%" PRIu64 ", frameID=%" PRIu64 ", resourceID=%" PRIu64 ")", loadParameters.parentPID, loadParameters.webPageProxyID.toUInt64(), loadParameters.webPageID.toUInt64(), loadParameters.webFrameID.toUInt64(), loadParameters.identifier);
+
     auto identifier = loadParameters.identifier;
     RELEASE_ASSERT(identifier);
     RELEASE_ASSERT(RunLoop::isMain());
@@ -487,6 +492,8 @@
 
 void NetworkConnectionToWebProcess::loadPing(NetworkResourceLoadParameters&& loadParameters)
 {
+    RELEASE_LOG_IF_ALLOWED(Loading, "loadPing: (parentPID=%d, pageProxyID=%" PRIu64 ", webPageID=%" PRIu64 ", frameID=%" PRIu64 ", resourceID=%" PRIu64 ")", loadParameters.parentPID, loadParameters.webPageProxyID.toUInt64(), loadParameters.webPageID.toUInt64(), loadParameters.webFrameID.toUInt64(), loadParameters.identifier);
+
     auto completionHandler = [connection = m_connection.copyRef(), identifier = loadParameters.identifier] (const ResourceError& error, const ResourceResponse& response) {
         connection->send(Messages::NetworkProcessConnection::DidFinishPingLoad(identifier, error, response), 0);
     };
@@ -540,10 +547,12 @@
     m_networkProcess->prefetchDNS(hostname);
 }
 
-void NetworkConnectionToWebProcess::preconnectTo(Optional<uint64_t> preconnectionIdentifier, NetworkResourceLoadParameters&& parameters)
+void NetworkConnectionToWebProcess::preconnectTo(Optional<uint64_t> preconnectionIdentifier, NetworkResourceLoadParameters&& loadParameters)
 {
-    ASSERT(!parameters.request.httpBody());
+    RELEASE_LOG_IF_ALLOWED(Loading, "preconnectTo: (parentPID=%d, pageProxyID=%" PRIu64 ", webPageID=%" PRIu64 ", frameID=%" PRIu64 ", resourceID=%" PRIu64 ")", loadParameters.parentPID, loadParameters.webPageProxyID.toUInt64(), loadParameters.webPageID.toUInt64(), loadParameters.webFrameID.toUInt64(), loadParameters.identifier);
 
+    ASSERT(!loadParameters.request.httpBody());
+
     auto completionHandler = [this, protectedThis = makeRef(*this), preconnectionIdentifier = WTFMove(preconnectionIdentifier)](const ResourceError& error) {
         if (preconnectionIdentifier)
             didFinishPreconnection(*preconnectionIdentifier, error);
@@ -550,8 +559,8 @@
     };
 
 #if ENABLE(LEGACY_CUSTOM_PROTOCOL_MANAGER)
-    if (networkProcess().supplement<LegacyCustomProtocolManager>()->supportsScheme(parameters.request.url().protocol().toString())) {
-        completionHandler(internalError(parameters.request.url()));
+    if (networkProcess().supplement<LegacyCustomProtocolManager>()->supportsScheme(loadParameters.request.url().protocol().toString())) {
+        completionHandler(internalError(loadParameters.request.url()));
         return;
     }
 #endif
@@ -559,13 +568,13 @@
 #if ENABLE(SERVER_PRECONNECT)
     auto* session = networkSession();
     if (session && session->allowsServerPreconnect()) {
-        new PreconnectTask(networkProcess(), sessionID(), WTFMove(parameters), [completionHandler = WTFMove(completionHandler)] (const ResourceError& error) {
+        new PreconnectTask(networkProcess(), sessionID(), WTFMove(loadParameters), [completionHandler = WTFMove(completionHandler)] (const ResourceError& error) {
             completionHandler(error);
         });
         return;
     }
 #endif
-    completionHandler(internalError(parameters.request.url()));
+    completionHandler(internalError(loadParameters.request.url()));
 }
 
 void NetworkConnectionToWebProcess::didFinishPreconnection(uint64_t preconnectionIdentifier, const ResourceError& error)
@@ -1033,7 +1042,7 @@
 
 void NetworkConnectionToWebProcess::serverToContextConnectionNoLongerNeeded()
 {
-    RELEASE_LOG_IF_ALLOWED(ServiceWorker, "serverToContextConnectionNoLongerNeeded - WebProcess %llu no longer useful for running service workers", webProcessIdentifier().toUInt64());
+    RELEASE_LOG_IF_ALLOWED(ServiceWorker, "serverToContextConnectionNoLongerNeeded: WebProcess no longer useful for running service workers");
     m_networkProcess->parentProcessConnection()->send(Messages::NetworkProcessProxy::WorkerContextConnectionNoLongerNeeded { webProcessIdentifier() }, 0);
 
     m_swContextConnection = nullptr;

Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (261248 => 261249)


--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2020-05-06 20:11:39 UTC (rev 261248)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2020-05-06 20:35:11 UTC (rev 261249)
@@ -175,7 +175,7 @@
 void NetworkResourceLoader::start()
 {
     ASSERT(RunLoop::isMain());
-    RELEASE_LOG_IF_ALLOWED("start: parentPID=%d, hasNetworkLoadChecker=%d", m_parameters.parentPID, !!m_networkLoadChecker);
+    RELEASE_LOG_IF_ALLOWED("start: hasNetworkLoadChecker=%d", !!m_networkLoadChecker);
 
     m_networkActivityTracker = m_connection->startTrackingResourceLoad(m_parameters.webPageID, m_parameters.identifier, isMainFrameLoad());
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to