Changeset 139516 in webkit

Timestamp:

Jan 11, 2013 4:08:30 PM (11 years ago)

Author:

ap@apple.com

Message:

[WK2] Network process unblocks all waiting threads when any sync reply arrives
​https://bugs.webkit.org/show_bug.cgi?id=106713

Reviewed by Brady Eidson.

Switch to sending sync CoreIPC messages, which is now possible.

• NetworkProcess/NetworkConnectionToWebProcess.h:
• NetworkProcess/NetworkConnectionToWebProcess.cpp: (WebKit::NetworkConnectionToWebProcess::didClose): We no longer have response maps.

• NetworkProcess/NetworkResourceLoader.cpp: (WebKit::NetworkResourceLoader::willSendRequest): Just send a sync message. (WebKit::NetworkResourceLoader::canAuthenticateAgainstProtectionSpace): Ditto. (WebKit::NetworkResourceLoader::didReceiveDataArray): Added an unrelated assertion.

• NetworkProcess/NetworkResourceLoader.h:
• NetworkProcess/NetworkResourceLoader.messages.in: Removed no longer used reply messages and their handlers.

• Shared/BlockingResponseMap.h: Removed a bool version, which was not perfectly safe, and only used in NetworkProcess. (BlockingResponseMap::didReceiveResponse): Updated a still valid FIXME to not refer to network process. (BlockingResponseMap::cancel): Ditto.

• WebProcess/Network/NetworkProcessConnection.h:
• WebProcess/Network/NetworkProcessConnection.cpp: (WebKit::NetworkProcessConnection::didReceiveSyncMessage): Plumbing to handle sync messages.

• WebProcess/Network/WebResourceLoader.cpp: (WebKit::WebResourceLoader::willSendRequest): (WebKit::WebResourceLoader::canAuthenticateAgainstProtectionSpace):
• WebProcess/Network/WebResourceLoader.h:
• WebProcess/Network/WebResourceLoader.messages.in: Updated (simplified) sync messages and their handlers.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/NetworkConnectionToWebProcess.cpp (modified) (1 diff)
• NetworkProcess/NetworkConnectionToWebProcess.h (modified) (2 diffs)
• NetworkProcess/NetworkResourceLoader.cpp (modified) (3 diffs)
• NetworkProcess/NetworkResourceLoader.h (modified) (1 diff)
• NetworkProcess/NetworkResourceLoader.messages.in (modified) (1 diff)
• Shared/BlockingResponseMap.h (modified) (3 diffs)
• WebProcess/Network/NetworkProcessConnection.cpp (modified) (1 diff)
• WebProcess/Network/NetworkProcessConnection.h (modified) (1 diff)
• WebProcess/Network/WebResourceLoader.cpp (modified) (2 diffs)
• WebProcess/Network/WebResourceLoader.h (modified) (3 diffs)
• WebProcess/Network/WebResourceLoader.messages.in (modified) (2 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2013-01-11  Alexey Proskuryakov  <ap@apple.com>
+
+        [WK2] Network process unblocks all waiting threads when any sync reply arrives
+        https://bugs.webkit.org/show_bug.cgi?id=106713
+
+        Reviewed by Brady Eidson.
+
+        Switch to sending sync CoreIPC messages, which is now possible.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.h:
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp: (WebKit::NetworkConnectionToWebProcess::didClose):
+        We no longer have response maps.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::willSendRequest): Just send a sync message.
+        (WebKit::NetworkResourceLoader::canAuthenticateAgainstProtectionSpace): Ditto.
+        (WebKit::NetworkResourceLoader::didReceiveDataArray): Added an unrelated assertion.
+
+        * NetworkProcess/NetworkResourceLoader.h:
+        * NetworkProcess/NetworkResourceLoader.messages.in:
+        Removed no longer used reply messages and their handlers.
+
+        * Shared/BlockingResponseMap.h: Removed a bool version, which was not perfectly
+        safe, and only used in NetworkProcess.
+        (BlockingResponseMap::didReceiveResponse): Updated a still valid FIXME to not refer
+        to network process.
+        (BlockingResponseMap::cancel): Ditto.
+
+        * WebProcess/Network/NetworkProcessConnection.h:
+        * WebProcess/Network/NetworkProcessConnection.cpp:
+        (WebKit::NetworkProcessConnection::didReceiveSyncMessage):
+        Plumbing to handle sync messages.
+
+        * WebProcess/Network/WebResourceLoader.cpp:
+        (WebKit::WebResourceLoader::willSendRequest):
+        (WebKit::WebResourceLoader::canAuthenticateAgainstProtectionSpace):
+        * WebProcess/Network/WebResourceLoader.h:
+        * WebProcess/Network/WebResourceLoader.messages.in:
+        Updated (simplified) sync messages and their handlers.
+
 2013-01-11  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp

@@ -104 +104 @@
     NetworkProcess::shared().removeNetworkConnectionToWebProcess(this);
 
-    // Unblock waiting threads.
-    m_willSendRequestResponseMap.cancel();
-    m_canAuthenticateAgainstProtectionSpaceResponseMap.cancel();
-
     Vector<NetworkConnectionToWebProcessObserver*> observers;
     copyToVector(m_observers, observers);

trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h

@@ -63 +63 @@
     bool isSerialLoadingEnabled() const { return m_serialLoadingEnabled; }
 
-    BlockingResponseMap<WebCore::ResourceRequest*>& willSendRequestResponseMap() { return m_willSendRequestResponseMap; }
-    BlockingBoolResponseMap& canAuthenticateAgainstProtectionSpaceResponseMap() { return m_canAuthenticateAgainstProtectionSpaceResponseMap; }
-
 private:
     NetworkConnectionToWebProcess(CoreIPC::Connection::Identifier);
@@ -97 +94 @@
     HashSet<NetworkConnectionToWebProcessObserver*> m_observers;
 
-    BlockingResponseMap<WebCore::ResourceRequest*> m_willSendRequestResponseMap;
-    BlockingBoolResponseMap m_canAuthenticateAgainstProtectionSpaceResponseMap;
-
     bool m_serialLoadingEnabled;
 };

trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp

@@ -183 +183 @@
 }
 
-static uint64_t generateWillSendRequestID()
-{
-    static int64_t uniqueWillSendRequestID;
-    return atomicIncrement(&uniqueWillSendRequestID);
-}
-
 void NetworkResourceLoader::willSendRequest(ResourceHandle*, ResourceRequest& request, const ResourceResponse& redirectResponse)
 {
     // We only expect to get the willSendRequest callback from ResourceHandle as the result of a redirect.
     ASSERT(!redirectResponse.isNull());
-
-    uint64_t requestID = generateWillSendRequestID();
-
-    if (!send(Messages::WebResourceLoader::WillSendRequest(requestID, request, redirectResponse))) {
+    ASSERT(!isMainThread());
+
+    if (!sendSync(Messages::WebResourceLoader::WillSendRequest(request, redirectResponse), Messages::WebResourceLoader::WillSendRequest::Reply(request)))
         request = ResourceRequest();
-        return;
-    }
-
-    OwnPtr<ResourceRequest> newRequest = m_connection->willSendRequestResponseMap().waitForResponse(requestID);
-    request = newRequest ? *newRequest : ResourceRequest();
-
-    RunLoop::main()->dispatch(WTF::bind(&NetworkResourceLoadScheduler::receivedRedirect, &NetworkProcess::shared().networkResourceLoadScheduler(), m_identifier, request.url()));
-}
-
-void NetworkResourceLoader::willSendRequestHandled(uint64_t requestID, const WebCore::ResourceRequest& newRequest)
-{
-    m_connection->willSendRequestResponseMap().didReceiveResponse(requestID, adoptPtr(new ResourceRequest(newRequest)));
 }
 
@@ -303 +284 @@
 
 #if USE(PROTECTION_SPACE_AUTH_CALLBACK)
-static uint64_t generateCanAuthenticateAgainstProtectionSpaceID()
-{
-    static int64_t uniqueCanAuthenticateAgainstProtectionSpaceID;
-    return atomicIncrement(&uniqueCanAuthenticateAgainstProtectionSpaceID);
-}
-
 bool NetworkResourceLoader::canAuthenticateAgainstProtectionSpace(ResourceHandle*, const ProtectionSpace& protectionSpace)
 {
-    uint64_t requestID = generateCanAuthenticateAgainstProtectionSpaceID();
-
-    if (!send(Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace(requestID, protectionSpace)))
+    ASSERT(!isMainThread());
+
+    bool result;
+    if (!sendSync(Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace(protectionSpace), Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace::Reply(result)))
         return false;
 
-    return m_connection->canAuthenticateAgainstProtectionSpaceResponseMap().waitForResponse(requestID);
-}
-
-void NetworkResourceLoader::canAuthenticateAgainstProtectionSpaceHandled(uint64_t requestID, bool canAuthenticate)
-{
-    m_connection->canAuthenticateAgainstProtectionSpaceResponseMap().didReceiveResponse(requestID, canAuthenticate);
+    return result;
 }
 #endif
@@ -334 +305 @@
 void NetworkResourceLoader::didReceiveDataArray(WebCore::ResourceHandle*, CFArrayRef)
 {
+    ASSERT_NOT_REACHED();
     notImplemented();
 }

trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h

@@ -111 +111 @@
     NetworkResourceLoader(const NetworkResourceLoadParameters&, ResourceLoadIdentifier, NetworkConnectionToWebProcess*);
 
-    void willSendRequestHandled(uint64_t requestID, const WebCore::ResourceRequest&);
-    void canAuthenticateAgainstProtectionSpaceHandled(uint64_t requestID, bool canAuthenticate);
     void receivedAuthenticationCredential(const WebCore::AuthenticationChallenge&, const WebCore::Credential&);
     void receivedRequestToContinueWithoutAuthenticationCredential(const WebCore::AuthenticationChallenge&);

trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.messages.in

@@ -24 +24 @@
 
 messages -> NetworkResourceLoader {
-    WillSendRequestHandled(uint64_t requestID, WebCore::ResourceRequest newRequest)
-    CanAuthenticateAgainstProtectionSpaceHandled(uint64_t requestID, bool canAuthenticate)
-    
     ReceivedAuthenticationCredential(WebCore::AuthenticationChallenge challenge, WebCore::Credential credential)
     ReceivedRequestToContinueWithoutAuthenticationCredential(WebCore::AuthenticationChallenge challenge)

trunk/Source/WebKit2/Shared/BlockingResponseMap.h

@@ -62 +62 @@
 
         m_responses.set(requestID, response);
-        // FIXME (NetworkProcess): <rdar://problem/12886430>: Waking up all threads is quite inefficient.
+        // FIXME: Waking up all threads is quite inefficient.
         m_condition.broadcast();
     }
@@ -70 +70 @@
         m_canceled = true;
 
-        // FIXME (NetworkProcess): <rdar://problem/12886430>: Waking up all threads is quite inefficient.
+        // FIXME: Waking up all threads is quite inefficient.
         m_condition.broadcast();
     }
@@ -82 +82 @@
 };
 
-class BlockingBoolResponseMap {
-WTF_MAKE_NONCOPYABLE(BlockingBoolResponseMap);
-public:
-    BlockingBoolResponseMap() : m_canceled(false) { }
-    ~BlockingBoolResponseMap() { ASSERT(m_responses.isEmpty()); }
-
-    bool waitForResponse(uint64_t requestID)
-    {
-        while (true) {
-            MutexLocker locker(m_mutex);
-
-            // FIXME: Differentiate between canceled wait and a negative response.
-            if (m_canceled)
-                return false;
-
-            HashMap<uint64_t, bool>::iterator iter = m_responses.find(requestID);
-            if (iter != m_responses.end()) {
-                bool result = iter->value;
-                m_responses.remove(iter);
-                return result;
-            }
-
-            m_condition.wait(m_mutex);
-        }
-
-        return false;
-    }
-
-    void didReceiveResponse(uint64_t requestID, bool response)
-    {
-        MutexLocker locker(m_mutex);
-        ASSERT(!m_responses.contains(requestID));
-
-        m_responses.set(requestID, response);
-        // FIXME (NetworkProcess): <rdar://problem/12886430>: Waking up all threads is quite inefficient.
-        m_condition.broadcast();
-    }
-
-    void cancel()
-    {
-        m_canceled = true;
-
-        // FIXME (NetworkProcess): <rdar://problem/12886430>: Waking up all threads is quite inefficient.
-        m_condition.broadcast();
-    }
-
-private:
-    Mutex m_mutex;
-    ThreadCondition m_condition;
-
-    HashMap<uint64_t, bool> m_responses;
-    bool m_canceled;
-};
-
 #endif // BlockingResponseMap_h

trunk/Source/WebKit2/WebProcess/Network/NetworkProcessConnection.cpp

@@ -63 +63 @@
 }
 
+void NetworkProcessConnection::didReceiveSyncMessage(CoreIPC::Connection* connection, CoreIPC::MessageID messageID, CoreIPC::MessageDecoder& decoder, OwnPtr<CoreIPC::MessageEncoder>& replyEncoder)
+{
+    if (messageID.is<CoreIPC::MessageClassWebResourceLoader>()) {
+        if (WebResourceLoader* webResourceLoader = WebProcess::shared().webResourceLoadScheduler().webResourceLoaderForIdentifier(decoder.destinationID()))
+            webResourceLoader->didReceiveSyncWebResourceLoaderMessage(connection, messageID, decoder, replyEncoder);
+        
+        return;
+    }
+
+    ASSERT_NOT_REACHED();
+}
+
 void NetworkProcessConnection::didClose(CoreIPC::Connection*)
 {

trunk/Source/WebKit2/WebProcess/Network/NetworkProcessConnection.h

@@ -62 +62 @@
     // CoreIPC::Connection::Client
     virtual void didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&);
+    virtual void didReceiveSyncMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&, OwnPtr<CoreIPC::MessageEncoder>&);
     virtual void didClose(CoreIPC::Connection*);
     virtual void didReceiveInvalidMessage(CoreIPC::Connection*, CoreIPC::StringReference messageReceiverName, CoreIPC::StringReference messageName) OVERRIDE;

trunk/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp

@@ -69 +69 @@
 }
 
-void WebResourceLoader::willSendRequest(uint64_t requestID, const ResourceRequest& proposedRequest, const ResourceResponse& redirectResponse)
+void WebResourceLoader::willSendRequest(const ResourceRequest& proposedRequest, const ResourceResponse& redirectResponse, ResourceRequest& newRequest)
 {
     LOG(Network, "(WebProcess) WebResourceLoader::willSendRequest to '%s'", proposedRequest.url().string().utf8().data());
     
-    ResourceRequest newRequest = proposedRequest;
+    newRequest = proposedRequest;
     m_coreLoader->willSendRequest(newRequest, redirectResponse);
-
-    send(Messages::NetworkResourceLoader::WillSendRequestHandled(requestID, newRequest));
 }
 
@@ -123 +121 @@
 }
 
-void WebResourceLoader::canAuthenticateAgainstProtectionSpace(uint64_t requestID, const ProtectionSpace& protectionSpace)
+void WebResourceLoader::canAuthenticateAgainstProtectionSpace(const ProtectionSpace& protectionSpace, bool& result)
 {
-    send(Messages::NetworkResourceLoader::CanAuthenticateAgainstProtectionSpaceHandled(requestID, m_coreLoader->canAuthenticateAgainstProtectionSpace(protectionSpace)));
+    result = m_coreLoader->canAuthenticateAgainstProtectionSpace(protectionSpace);
 }
 

trunk/Source/WebKit2/WebProcess/Network/WebResourceLoader.h

@@ -67 +67 @@
 
     void didReceiveWebResourceLoaderMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&);
+    void didReceiveSyncWebResourceLoaderMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&, OwnPtr<CoreIPC::MessageEncoder>&);
 
     using RefCounted<WebResourceLoader>::ref;
@@ -80 +81 @@
     WebResourceLoader(PassRefPtr<WebCore::ResourceLoader>);
 
-    void willSendRequest(uint64_t requestID, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse);
+    void willSendRequest(const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse, WebCore::ResourceRequest& newRequest);
     void didReceiveResponseWithCertificateInfo(const WebCore::ResourceResponse&, const PlatformCertificateInfo&);
     void didReceiveData(const CoreIPC::DataReference&, int64_t encodedDataLength, bool allAtOnce);
@@ -87 +88 @@
     void didReceiveResource(const ShareableResource::Handle&, double finishTime);
 
-    void canAuthenticateAgainstProtectionSpace(uint64_t requestID, const WebCore::ProtectionSpace&);
+    void canAuthenticateAgainstProtectionSpace(const WebCore::ProtectionSpace&, bool& result);
     void didReceiveAuthenticationChallenge(const WebCore::AuthenticationChallenge&);
     void didCancelAuthenticationChallenge(const WebCore::AuthenticationChallenge&);

trunk/Source/WebKit2/WebProcess/Network/WebResourceLoader.messages.in

@@ -24 +24 @@
 
     // FIXME (NetworkProcess): We'll need much more granularity for response messages.
-    WillSendRequest(uint64_t requestID, WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse)
+    WillSendRequest(WebCore::ResourceRequest request, WebCore::ResourceResponse redirectResponse) -> (WebCore::ResourceRequest newRequest)
     DidReceiveResponseWithCertificateInfo(WebCore::ResourceResponse response, WebKit::PlatformCertificateInfo certificateInfo)
     DidReceiveData(CoreIPC::DataReference data, int64_t encodedDataLength, bool allAtOnce)
@@ -30 +30 @@
     DidFailResourceLoad(WebCore::ResourceError error)
     
-    CanAuthenticateAgainstProtectionSpace(uint64_t requestID, WebCore::ProtectionSpace protectionSpace)
+    CanAuthenticateAgainstProtectionSpace(WebCore::ProtectionSpace protectionSpace) -> (bool response)
     DidReceiveAuthenticationChallenge(WebCore::AuthenticationChallenge challenge)
     DidCancelAuthenticationChallenge(WebCore::AuthenticationChallenge challenge)