Log Message
[soup] Crash while loading http://www.jusco.cn https://bugs.webkit.org/show_bug.cgi?id=68238
Patch by Martin Robinson <[email protected]> on 2012-04-09 Reviewed by Philippe Normand. .: * configure.ac: Bumped the libsoup dependency to 2.37.90. Source/WebCore: Test: http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html When running synchronous XMLHttpRequests, push a new inner thread default context, so that other sources from timers and network activity do not run. This will make synchronous requests truly synchronous with the rest of WebCore. * platform/network/soup/ResourceHandleSoup.cpp: (WebCoreSynchronousLoader): Clean up the method definitions a bit by writing them inline. (WebCore::WebCoreSynchronousLoader::WebCoreSynchronousLoader): Push a new thread default context to prevent other sources from running. (WebCore::WebCoreSynchronousLoader::~WebCoreSynchronousLoader): Pop the inner thread default context. (WebCore::closeCallback): If the client is synchronous call didFinishLoading now. (WebCore::readCallback): Only call didFinishLoading if the client isn't synchronous. (WebCore::ResourceHandle::defaultSession): Activate use-thread-context so that the soup session respects the inner thread context. LayoutTests: * http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt: Added. * http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html: Added.
Modified Paths
- trunk/ChangeLog
- trunk/LayoutTests/ChangeLog
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp
Added Paths
Diff
Modified: trunk/ChangeLog (113603 => 113604)
--- trunk/ChangeLog 2012-04-09 19:06:15 UTC (rev 113603)
+++ trunk/ChangeLog 2012-04-09 19:28:42 UTC (rev 113604)
@@ -1,3 +1,12 @@
+2012-04-09 Martin Robinson <[email protected]>
+
+ [soup] Crash while loading http://www.jusco.cn
+ https://bugs.webkit.org/show_bug.cgi?id=68238
+
+ Reviewed by Philippe Normand.
+
+ * configure.ac: Bumped the libsoup dependency to 2.37.90.
+
2012-04-09 Abhishek Arya <[email protected]>
Crash due to floats not cleared before starting SVG <text> layout.
Modified: trunk/LayoutTests/ChangeLog (113603 => 113604)
--- trunk/LayoutTests/ChangeLog 2012-04-09 19:06:15 UTC (rev 113603)
+++ trunk/LayoutTests/ChangeLog 2012-04-09 19:28:42 UTC (rev 113604)
@@ -1,5 +1,15 @@
2012-04-09 Martin Robinson <[email protected]>
+ [soup] Crash while loading http://www.jusco.cn
+ https://bugs.webkit.org/show_bug.cgi?id=68238
+
+ Reviewed by Philippe Normand.
+
+ * http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt: Added.
+ * http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html: Added.
+
+2012-04-09 Martin Robinson <[email protected]>
+
[GTK] Toggle buttons do not size appropriately in some themes
https://bugs.webkit.org/show_bug.cgi?id=82833
Added: trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt (0 => 113604)
--- trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt 2012-04-09 19:28:42 UTC (rev 113604)
@@ -0,0 +1,4 @@
+Test for: bug 68238: [soup] Crash while loading http://www.jusco.cn This test verifies that WebCore timers do not fire during synchronous XMLHttpRequests.
+
+PASS
+
Added: trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html (0 => 113604)
--- trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html 2012-04-09 19:28:42 UTC (rev 113604)
@@ -0,0 +1,32 @@
+<html><body>
+
+<p> Test for: <a href="" 68238<a>: [soup] Crash while loading http://www.jusco.cn</a> This test verifies that WebCore timers do not fire during synchronous XMLHttpRequests.
+<pre id=log></pre>
+
+<script type="text/_javascript_">
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function log(message)
+{
+ document.getElementById("log").innerHTML += message + "\n";
+}
+
+var timerEverFired = false;
+var intervalId = setInterval(function() {
+ timerEverFired = true;
+}, 10);
+
+try {
+ var req = new XMLHttpRequest();
+ req.open("GET", "resources/download-with-delay.php?iteration=5&delay=50", false);
+ req.send(null);
+} catch (ex) {
+ log(ex);
+}
+
+clearInterval(intervalId);
+log(timerEverFired ? "FAIL" : "PASS");
+</script>
+
+</body></html>
Modified: trunk/Source/WebCore/ChangeLog (113603 => 113604)
--- trunk/Source/WebCore/ChangeLog 2012-04-09 19:06:15 UTC (rev 113603)
+++ trunk/Source/WebCore/ChangeLog 2012-04-09 19:28:42 UTC (rev 113604)
@@ -1,3 +1,27 @@
+2012-04-09 Martin Robinson <[email protected]>
+
+ [soup] Crash while loading http://www.jusco.cn
+ https://bugs.webkit.org/show_bug.cgi?id=68238
+
+ Reviewed by Philippe Normand.
+
+ Test: http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html
+
+ When running synchronous XMLHttpRequests, push a new inner thread default
+ context, so that other sources from timers and network activity do not run.
+ This will make synchronous requests truly synchronous with the rest of
+ WebCore.
+
+ * platform/network/soup/ResourceHandleSoup.cpp:
+ (WebCoreSynchronousLoader): Clean up the method definitions a bit by writing them inline.
+ (WebCore::WebCoreSynchronousLoader::WebCoreSynchronousLoader): Push a new thread default
+ context to prevent other sources from running.
+ (WebCore::WebCoreSynchronousLoader::~WebCoreSynchronousLoader): Pop the inner thread default context.
+ (WebCore::closeCallback): If the client is synchronous call didFinishLoading now.
+ (WebCore::readCallback): Only call didFinishLoading if the client isn't synchronous.
+ (WebCore::ResourceHandle::defaultSession): Activate use-thread-context so that the soup session
+ respects the inner thread context.
+
2012-04-09 Dana Jansens <[email protected]>
[chromium] Flip transition painting delayed with threaded animations
Modified: trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp (113603 => 113604)
--- trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2012-04-09 19:06:15 UTC (rev 113603)
+++ trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2012-04-09 19:28:42 UTC (rev 113604)
@@ -69,19 +69,67 @@
#define READ_BUFFER_SIZE 8192
+static bool loadingSynchronousRequest = false;
+
class WebCoreSynchronousLoader : public ResourceHandleClient {
WTF_MAKE_NONCOPYABLE(WebCoreSynchronousLoader);
public:
- WebCoreSynchronousLoader(ResourceError&, ResourceResponse &, Vector<char>&);
- ~WebCoreSynchronousLoader();
- virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse&);
- virtual void didReceiveData(ResourceHandle*, const char*, int, int encodedDataLength);
- virtual void didFinishLoading(ResourceHandle*, double /*finishTime*/);
- virtual void didFail(ResourceHandle*, const ResourceError&);
+ WebCoreSynchronousLoader(ResourceError& error, ResourceResponse& response, Vector<char>& data)
+ : m_error(error)
+ , m_response(response)
+ , m_data(data)
+ , m_finished(false)
+ {
+ // We don't want any timers to fire while we are doing our synchronous load
+ // so we replace the thread default main context. The main loop iterations
+ // will only process GSources associated with this inner context.
+ loadingSynchronousRequest = true;
+ GRefPtr<GMainContext> innerMainContext = adoptGRef(g_main_context_new());
+ g_main_context_push_thread_default(innerMainContext.get());
+ m_mainLoop = g_main_loop_new(innerMainContext.get(), false);
+ }
- void run();
+ ~WebCoreSynchronousLoader()
+ {
+ g_main_context_pop_thread_default(g_main_context_get_thread_default());
+ loadingSynchronousRequest = false;
+ }
+ virtual bool isSynchronousClient()
+ {
+ return true;
+ }
+
+ virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
+ {
+ m_response = response;
+ }
+
+ virtual void didReceiveData(ResourceHandle*, const char* data, int length, int)
+ {
+ m_data.append(data, length);
+ }
+
+ virtual void didFinishLoading(ResourceHandle*, double)
+ {
+ if (g_main_loop_is_running(m_mainLoop.get()))
+ g_main_loop_quit(m_mainLoop.get());
+ m_finished = true;
+ }
+
+ virtual void didFail(ResourceHandle* handle, const ResourceError& error)
+ {
+ m_error = error;
+ didFinishLoading(handle, 0);
+ }
+
+ void run()
+ {
+ if (!m_finished)
+ g_main_loop_run(m_mainLoop.get());
+ }
+
private:
ResourceError& m_error;
ResourceResponse& m_response;
@@ -90,47 +138,6 @@
GRefPtr<GMainLoop> m_mainLoop;
};
-WebCoreSynchronousLoader::WebCoreSynchronousLoader(ResourceError& error, ResourceResponse& response, Vector<char>& data)
- : m_error(error)
- , m_response(response)
- , m_data(data)
- , m_finished(false)
-{
- m_mainLoop = adoptGRef(g_main_loop_new(0, false));
-}
-
-WebCoreSynchronousLoader::~WebCoreSynchronousLoader()
-{
-}
-
-void WebCoreSynchronousLoader::didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
-{
- m_response = response;
-}
-
-void WebCoreSynchronousLoader::didReceiveData(ResourceHandle*, const char* data, int length, int)
-{
- m_data.append(data, length);
-}
-
-void WebCoreSynchronousLoader::didFinishLoading(ResourceHandle*, double)
-{
- g_main_loop_quit(m_mainLoop.get());
- m_finished = true;
-}
-
-void WebCoreSynchronousLoader::didFail(ResourceHandle* handle, const ResourceError& error)
-{
- m_error = error;
- didFinishLoading(handle, 0);
-}
-
-void WebCoreSynchronousLoader::run()
-{
- if (!m_finished)
- g_main_loop_run(m_mainLoop.get());
-}
-
static void cleanupSoupRequestOperation(ResourceHandle*, bool isDestroying);
static void sendRequestCallback(GObject*, GAsyncResult*, gpointer);
static void readCallback(GObject*, GAsyncResult*, gpointer);
@@ -634,9 +641,14 @@
static void closeCallback(GObject* source, GAsyncResult* res, gpointer data)
{
RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);
+ ResourceHandleInternal* d = handle->getInternal();
- ResourceHandleInternal* d = handle->getInternal();
g_input_stream_close_finish(d->m_inputStream.get(), res, 0);
+
+ ResourceHandleClient* client = handle->client();
+ if (client && loadingSynchronousRequest)
+ client->didFinishLoading(handle.get(), 0);
+
cleanupSoupRequestOperation(handle.get());
}
@@ -667,8 +679,14 @@
if (!bytesRead) {
// We inform WebCore of load completion now instead of waiting for the input
- // stream to close because the input stream is closed asynchronously.
- client->didFinishLoading(handle.get(), 0);
+ // stream to close because the input stream is closed asynchronously. If this
+ // is a synchronous request, we wait until the closeCallback, because we don't
+ // want to halt the internal main loop before the input stream closes.
+ if (client && !loadingSynchronousRequest) {
+ client->didFinishLoading(handle.get(), 0);
+ handle->setClient(0); // Unset the client so that we do not try to access th
+ // client in the closeCallback.
+ }
g_input_stream_close_async(d->m_inputStream.get(), G_PRIORITY_DEFAULT, 0, closeCallback, handle.get());
return;
}
@@ -740,6 +758,7 @@
SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_DECODER,
SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_SNIFFER,
SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_PROXY_RESOLVER_DEFAULT,
+ SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
NULL);
}
_______________________________________________ webkit-changes mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
