Reviewed by Adam.
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Feb 2008 21:33:27 +0000 (21:33 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Feb 2008 21:33:27 +0000 (21:33 +0000)
        - fix <rdar://problem/4527931> showModalDialog calls from onload functions fail (Aspect)

        No automated regression test because showModalDialog doesn't work in DumpRenderTree.

        * platform/network/mac/ResourceHandleMac.mm:
        (WebCore::CallbackGuard::CallbackGuard): Added.
        (WebCore::CallbackGuard::~CallbackGuard): Added.
        (WebCore::ResourceHandle::supportsBufferedData): Simplified to take advantage of how
        static initialization works in C++.
        (WebCore::ResourceHandle::loadsBlocked): Always return false on Leopard. The problem in
        NSURLConnection that created the need to block loads is fixed in Leoaprd. This is the
        bug fix.
        (-[WebCoreResourceHandleAsDelegate connection:willSendRequest:redirectResponse:]):
        Use CallbackGuard instead of directly incrementing the count; allows us to omit the code
        entirely on Leopard.
        (-[WebCoreResourceHandleAsDelegate connection:didReceiveAuthenticationChallenge:]):
        Ditto.
        (-[WebCoreResourceHandleAsDelegate connection:didCancelAuthenticationChallenge:]):
        Ditto.
        (-[WebCoreResourceHandleAsDelegate connection:didReceiveResponse:]):
        Ditto.
        (-[WebCoreResourceHandleAsDelegate connection:didReceiveData:lengthReceived:]):
        Ditto.
        (-[WebCoreResourceHandleAsDelegate connection:willStopBufferingData:]):
        Ditto.
        (-[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:]):
        Ditto.
        (-[WebCoreResourceHandleAsDelegate connection:didFailWithError:]):
        Ditto.
        (-[WebCoreResourceHandleAsDelegate connection:willCacheResponse:]):
        Ditto. Fixes a problem where this could leave inNSURLConnectionCallback set
        permanently in one of the code paths; this would break showModalDialog from that
        point on. This problem doesn't affect Safari.

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

WebCore/ChangeLog
WebCore/platform/network/mac/ResourceHandleMac.mm

index 3b39d8a..a7aedd5 100644 (file)
@@ -1,5 +1,43 @@
 2008-02-01  Darin Adler  <darin@apple.com>
 
+        Reviewed by Adam.
+
+        - fix <rdar://problem/4527931> showModalDialog calls from onload functions fail (Aspect)
+
+        No automated regression test because showModalDialog doesn't work in DumpRenderTree.
+
+        * platform/network/mac/ResourceHandleMac.mm:
+        (WebCore::CallbackGuard::CallbackGuard): Added.
+        (WebCore::CallbackGuard::~CallbackGuard): Added.
+        (WebCore::ResourceHandle::supportsBufferedData): Simplified to take advantage of how
+        static initialization works in C++.
+        (WebCore::ResourceHandle::loadsBlocked): Always return false on Leopard. The problem in
+        NSURLConnection that created the need to block loads is fixed in Leoaprd. This is the
+        bug fix.
+        (-[WebCoreResourceHandleAsDelegate connection:willSendRequest:redirectResponse:]):
+        Use CallbackGuard instead of directly incrementing the count; allows us to omit the code
+        entirely on Leopard.
+        (-[WebCoreResourceHandleAsDelegate connection:didReceiveAuthenticationChallenge:]):
+        Ditto.
+        (-[WebCoreResourceHandleAsDelegate connection:didCancelAuthenticationChallenge:]):
+        Ditto.
+        (-[WebCoreResourceHandleAsDelegate connection:didReceiveResponse:]):
+        Ditto.
+        (-[WebCoreResourceHandleAsDelegate connection:didReceiveData:lengthReceived:]):
+        Ditto.
+        (-[WebCoreResourceHandleAsDelegate connection:willStopBufferingData:]):
+        Ditto.
+        (-[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:]):
+        Ditto.
+        (-[WebCoreResourceHandleAsDelegate connection:didFailWithError:]):
+        Ditto.
+        (-[WebCoreResourceHandleAsDelegate connection:willCacheResponse:]):
+        Ditto. Fixes a problem where this could leave inNSURLConnectionCallback set
+        permanently in one of the code paths; this would break showModalDialog from that
+        point on. This problem doesn't affect Safari.
+
+2008-02-01  Darin Adler  <darin@apple.com>
+
         * platform/network/mac/ResourceHandleMac.mm:
         (WebCore::ResourceHandle::loadsBlocked): Roll out accidentally checked in file.
 
index dc83473..5b9effb 100644 (file)
@@ -75,14 +75,32 @@ static NSString *WebCoreSynchronousLoaderRunLoopMode = @"WebCoreSynchronousLoade
 #endif
 
 namespace WebCore {
-   
+
+#ifdef BUILDING_ON_TIGER
 static unsigned inNSURLConnectionCallback;
-static bool NSURLConnectionSupportsBufferedData;
-    
+#endif
+
 #ifndef NDEBUG
 static bool isInitializingConnection;
 #endif
     
+class CallbackGuard {
+public:
+    CallbackGuard()
+    {
+#ifdef BUILDING_ON_TIGER
+        ++inNSURLConnectionCallback;
+#endif
+    }
+    ~CallbackGuard()
+    {
+#ifdef BUILDING_ON_TIGER
+        ASSERT(inNSURLConnectionCallback > 0);
+        --inNSURLConnectionCallback;
+#endif
+    }
+};
+
 ResourceHandleInternal::~ResourceHandleInternal()
 {
 }
@@ -181,13 +199,8 @@ void ResourceHandle::releaseDelegate()
 
 bool ResourceHandle::supportsBufferedData()
 {
-    static bool initialized = false;
-    if (!initialized) {
-        NSURLConnectionSupportsBufferedData = [NSURLConnection instancesRespondToSelector:@selector(_bufferedData)];
-        initialized = true;
-    }
-
-    return NSURLConnectionSupportsBufferedData;
+    static bool supportsBufferedData = [NSURLConnection instancesRespondToSelector:@selector(_bufferedData)];
+    return supportsBufferedData;
 }
 
 PassRefPtr<SharedBuffer> ResourceHandle::bufferedData()
@@ -213,7 +226,14 @@ NSURLConnection *ResourceHandle::connection() const
 
 bool ResourceHandle::loadsBlocked()
 {
+#ifndef BUILDING_ON_TIGER
+    return false;
+#else
+    // On Tiger, if we're in a NSURLConnection callback, that blocks all other NSURLConnection callbacks.
+    // On Leopard and newer, it blocks only callbacks on that same NSURLConnection object, which is not
+    // a problem and practice.
     return inNSURLConnectionCallback != 0;
+#endif
 }
 
 bool ResourceHandle::willLoadFromCache(ResourceRequest& request)
@@ -367,10 +387,9 @@ void ResourceHandle::receivedCancellation(const AuthenticationChallenge& challen
     if (!redirectResponse)
         return newRequest;
     
-    ++inNSURLConnectionCallback;
+    CallbackGuard guard;
     ResourceRequest request = newRequest;
     m_handle->client()->willSendRequest(m_handle, request, redirectResponse);
-    --inNSURLConnectionCallback;
 #ifndef BUILDING_ON_TIGER
     NSURL *copy = [[request.nsURLRequest() URL] copy];
     [m_url release];
@@ -400,27 +419,24 @@ void ResourceHandle::receivedCancellation(const AuthenticationChallenge& challen
     
     if (!m_handle)
         return;
-    ++inNSURLConnectionCallback;
+    CallbackGuard guard;
     m_handle->didReceiveAuthenticationChallenge(core(challenge));
-    --inNSURLConnectionCallback;
 }
 
 - (void)connection:(NSURLConnection *)con didCancelAuthenticationChallenge:(NSURLAuthenticationChallenge *)challenge
 {
     if (!m_handle)
         return;
-    ++inNSURLConnectionCallback;
+    CallbackGuard guard;
     m_handle->didCancelAuthenticationChallenge(core(challenge));
-    --inNSURLConnectionCallback;
 }
 
 - (void)connection:(NSURLConnection *)con didReceiveResponse:(NSURLResponse *)r
 {
     if (!m_handle || !m_handle->client())
         return;
-    ++inNSURLConnectionCallback;
+    CallbackGuard guard;
     m_handle->client()->didReceiveResponse(m_handle, r);
-    --inNSURLConnectionCallback;
 }
 
 - (void)connection:(NSURLConnection *)con didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived
@@ -430,9 +446,8 @@ void ResourceHandle::receivedCancellation(const AuthenticationChallenge& challen
     // FIXME: If we get more than 2B bytes in a single chunk, this code won't do the right thing.
     // However, with today's computers and networking speeds, this won't happen in practice.
     // Could be an issue with a giant local file.
-    ++inNSURLConnectionCallback;
+    CallbackGuard guard;
     m_handle->client()->didReceiveData(m_handle, (const char*)[data bytes], [data length], static_cast<int>(lengthReceived));
-    --inNSURLConnectionCallback;
 }
 
 - (void)connection:(NSURLConnection *)con willStopBufferingData:(NSData *)data
@@ -442,27 +457,24 @@ void ResourceHandle::receivedCancellation(const AuthenticationChallenge& challen
     // FIXME: If we get a resource with more than 2B bytes, this code won't do the right thing.
     // However, with today's computers and networking speeds, this won't happen in practice.
     // Could be an issue with a giant local file.
-    ++inNSURLConnectionCallback;
+    CallbackGuard guard;
     m_handle->client()->willStopBufferingData(m_handle, (const char*)[data bytes], static_cast<int>([data length]));
-    --inNSURLConnectionCallback;
 }
 
 - (void)connectionDidFinishLoading:(NSURLConnection *)con
 {
     if (!m_handle || !m_handle->client())
         return;
-    ++inNSURLConnectionCallback;
+    CallbackGuard guard;
     m_handle->client()->didFinishLoading(m_handle);
-    --inNSURLConnectionCallback;
 }
 
 - (void)connection:(NSURLConnection *)con didFailWithError:(NSError *)error
 {
     if (!m_handle || !m_handle->client())
         return;
-    ++inNSURLConnectionCallback;
+    CallbackGuard guard;
     m_handle->client()->didFail(m_handle, error);
-    --inNSURLConnectionCallback;
 }
 
 #ifdef BUILDING_ON_TIGER
@@ -507,9 +519,10 @@ void ResourceHandle::receivedCancellation(const AuthenticationChallenge& challen
 #endif
     if (!m_handle || !m_handle->client())
         return nil;
-    ++inNSURLConnectionCallback;
+
+    CallbackGuard guard;
     
-    NSCachedURLResponse * newResponse = m_handle->client()->willCacheResponse(m_handle, cachedResponse);
+    NSCachedURLResponse *newResponse = m_handle->client()->willCacheResponse(m_handle, cachedResponse);
     if (newResponse != cachedResponse)
         return newResponse;
     
@@ -519,11 +532,10 @@ void ResourceHandle::receivedCancellation(const AuthenticationChallenge& challen
 
     if (static_cast<NSURLCacheStoragePolicy>(policy) != [newResponse storagePolicy])
         newResponse = [[[NSCachedURLResponse alloc] initWithResponse:[newResponse response]
-                                                                   data:[newResponse data]
-                                                               userInfo:[newResponse userInfo]
-                                                          storagePolicy:static_cast<NSURLCacheStoragePolicy>(policy)] autorelease];
+                                                                data:[newResponse data]
+                                                            userInfo:[newResponse userInfo]
+                                                       storagePolicy:static_cast<NSURLCacheStoragePolicy>(policy)] autorelease];
 
-    --inNSURLConnectionCallback;
     return newResponse;
 }