Diff
Modified: trunk/LayoutTests/ChangeLog (88577 => 88578)
--- trunk/LayoutTests/ChangeLog 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/LayoutTests/ChangeLog 2011-06-11 00:11:01 UTC (rev 88578)
@@ -1,5 +1,15 @@
2011-06-10 David Levin <[email protected]>
+ Reviewed by Dmitry Titov.
+
+ Fetching a Worker with url that isn't allowed from a file based test causes DRT to crash.
+ https://bugs.webkit.org/show_bug.cgi?id=62469
+
+ * fast/workers/worker-crash-with-invalid-location-expected.txt: Added.
+ * fast/workers/worker-crash-with-invalid-location.html: Added.
+
+2011-06-10 David Levin <[email protected]>
+
Reviewed by Adam Barth.
Add tests for Web Workers at invalid urls.
Added: trunk/LayoutTests/fast/workers/worker-crash-with-invalid-location-expected.txt (0 => 88578)
--- trunk/LayoutTests/fast/workers/worker-crash-with-invalid-location-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/workers/worker-crash-with-invalid-location-expected.txt 2011-06-11 00:11:01 UTC (rev 88578)
@@ -0,0 +1,6 @@
+Blocked access to external URL http://example.com/worker.js
+Blocked access to external URL http://example.com/worker.js
+Test worker fetch of blocked url. Should print a "PASS" statement.
+
+PASS
+
Property changes on: trunk/LayoutTests/fast/workers/worker-crash-with-invalid-location-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/workers/worker-crash-with-invalid-location.html (0 => 88578)
--- trunk/LayoutTests/fast/workers/worker-crash-with-invalid-location.html (rev 0)
+++ trunk/LayoutTests/fast/workers/worker-crash-with-invalid-location.html 2011-06-11 00:11:01 UTC (rev 88578)
@@ -0,0 +1,32 @@
+<html>
+<body>
+<p>Test worker fetch of blocked url. Should print a "PASS" statement.</p>
+<div id=result></div>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function log(message)
+{
+ document.getElementById("result").innerHTML += message + "<br>";
+}
+
+try {
+ // DumpRenderTree allows a request to this url, but then blocks it
+ // during the "willSendRequest" callback (which caused a crash).
+ // In a browser, this will throw a security exception.
+ new Worker("http://example.com/worker.js");
+} catch (error) {
+}
+
+try {
+ // Ditto.
+ new SharedWorker("http://example.com/worker.js");
+} catch (error) {
+}
+
+log("PASS");
+
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/workers/worker-crash-with-invalid-location.html
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (88577 => 88578)
--- trunk/Source/WebCore/ChangeLog 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebCore/ChangeLog 2011-06-11 00:11:01 UTC (rev 88578)
@@ -1,3 +1,30 @@
+2011-06-10 David Levin <[email protected]>
+
+ Reviewed by Dmitry Titov.
+
+ Fetching a Worker with url that isn't allowed from a file based test causes DRT to crash.
+ https://bugs.webkit.org/show_bug.cgi?id=62469
+
+ Test: fast/workers/worker-crash-with-invalid-location.html
+
+ * workers/DefaultSharedWorkerRepository.cpp:
+ (WebCore::SharedWorkerScriptLoader::load): Changed to using the RefCounted version of WorkerScriptLoader.
+ * workers/Worker.cpp:
+ (WebCore::Worker::create): Ditto.
+ * workers/Worker.h: Ditto.
+ * workers/WorkerContext.cpp:
+ (WebCore::WorkerContext::importScripts): Ditto.
+ * workers/WorkerScriptLoader.cpp:
+ (WebCore::WorkerScriptLoader::~WorkerScriptLoader): Created to
+ allow removing some header includes in WorkerScriptLoader.h.
+ (WebCore::WorkerScriptLoader::loadAsynchronously): Fix the ordering
+ of setPendingActivity and keep WorkerScriptLoader alive during a
+ potential callback.
+ * workers/WorkerScriptLoader.h: Made this RefCounted to allow for
+ keeping it alive during callbacks. Also, removed unnecessary header
+ inclusions (and added a destructor to facilitate that).
+ (WebCore::WorkerScriptLoader::create):
+
2011-06-10 Alok Priyadarshi <[email protected]>
Reviewed by James Robinson.
Modified: trunk/Source/WebCore/workers/DefaultSharedWorkerRepository.cpp (88577 => 88578)
--- trunk/Source/WebCore/workers/DefaultSharedWorkerRepository.cpp 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebCore/workers/DefaultSharedWorkerRepository.cpp 2011-06-11 00:11:01 UTC (rev 88578)
@@ -271,7 +271,7 @@
RefPtr<SharedWorker> m_worker;
OwnPtr<MessagePortChannel> m_port;
RefPtr<SharedWorkerProxy> m_proxy;
- OwnPtr<WorkerScriptLoader> m_scriptLoader;
+ RefPtr<WorkerScriptLoader> m_scriptLoader;
};
SharedWorkerScriptLoader::SharedWorkerScriptLoader(PassRefPtr<SharedWorker> worker, PassOwnPtr<MessagePortChannel> port, PassRefPtr<SharedWorkerProxy> proxy)
@@ -283,13 +283,13 @@
void SharedWorkerScriptLoader::load(const KURL& url)
{
- // Mark this object as active for the duration of the load.
- m_scriptLoader = adoptPtr(new WorkerScriptLoader(ResourceRequestBase::TargetIsSharedWorker));
- m_scriptLoader->loadAsynchronously(m_worker->scriptExecutionContext(), url, DenyCrossOriginRequests, this);
-
// Stay alive (and keep the SharedWorker and JS wrapper alive) until the load finishes.
this->ref();
m_worker->setPendingActivity(m_worker.get());
+
+ // Mark this object as active for the duration of the load.
+ m_scriptLoader = WorkerScriptLoader::create(ResourceRequestBase::TargetIsSharedWorker);
+ m_scriptLoader->loadAsynchronously(m_worker->scriptExecutionContext(), url, DenyCrossOriginRequests, this);
}
void SharedWorkerScriptLoader::notifyFinished()
Modified: trunk/Source/WebCore/workers/Worker.cpp (88577 => 88578)
--- trunk/Source/WebCore/workers/Worker.cpp 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebCore/workers/Worker.cpp 2011-06-11 00:11:01 UTC (rev 88578)
@@ -64,12 +64,12 @@
if (scriptURL.isEmpty())
return 0;
- worker->m_scriptLoader = adoptPtr(new WorkerScriptLoader(ResourceRequestBase::TargetIsWorker));
- worker->m_scriptLoader->loadAsynchronously(context, scriptURL, DenyCrossOriginRequests, worker.get());
-
// The worker context does not exist while loading, so we must ensure that the worker object is not collected, nor are its event listeners.
worker->setPendingActivity(worker.get());
+ worker->m_scriptLoader = WorkerScriptLoader::create(ResourceRequestBase::TargetIsWorker);
+ worker->m_scriptLoader->loadAsynchronously(context, scriptURL, DenyCrossOriginRequests, worker.get());
+
InspectorInstrumentation::didCreateWorker(context, worker->asID(), scriptURL.string(), false);
return worker.release();
Modified: trunk/Source/WebCore/workers/Worker.h (88577 => 88578)
--- trunk/Source/WebCore/workers/Worker.h 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebCore/workers/Worker.h 2011-06-11 00:11:01 UTC (rev 88578)
@@ -37,9 +37,7 @@
#include "MessagePort.h"
#include "WorkerScriptLoaderClient.h"
#include <wtf/Forward.h>
-#include <wtf/OwnPtr.h>
#include <wtf/PassRefPtr.h>
-#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
#include <wtf/text/AtomicStringHash.h>
@@ -79,7 +77,7 @@
virtual void refEventTarget() { ref(); }
virtual void derefEventTarget() { deref(); }
- OwnPtr<WorkerScriptLoader> m_scriptLoader;
+ RefPtr<WorkerScriptLoader> m_scriptLoader;
WorkerContextProxy* m_contextProxy; // The proxy outlives the worker to perform thread shutdown.
};
Modified: trunk/Source/WebCore/workers/WorkerContext.cpp (88577 => 88578)
--- trunk/Source/WebCore/workers/WorkerContext.cpp 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebCore/workers/WorkerContext.cpp 2011-06-11 00:11:01 UTC (rev 88578)
@@ -245,19 +245,19 @@
Vector<KURL>::const_iterator end = completedURLs.end();
for (Vector<KURL>::const_iterator it = completedURLs.begin(); it != end; ++it) {
- WorkerScriptLoader scriptLoader(ResourceRequestBase::TargetIsScript);
- scriptLoader.loadSynchronously(scriptExecutionContext(), *it, AllowCrossOriginRequests);
+ RefPtr<WorkerScriptLoader> scriptLoader(WorkerScriptLoader::create(ResourceRequestBase::TargetIsScript));
+ scriptLoader->loadSynchronously(scriptExecutionContext(), *it, AllowCrossOriginRequests);
// If the fetching attempt failed, throw a NETWORK_ERR exception and abort all these steps.
- if (scriptLoader.failed()) {
+ if (scriptLoader->failed()) {
ec = XMLHttpRequestException::NETWORK_ERR;
return;
}
- InspectorInstrumentation::scriptImported(scriptExecutionContext(), scriptLoader.identifier(), scriptLoader.script());
+ InspectorInstrumentation::scriptImported(scriptExecutionContext(), scriptLoader->identifier(), scriptLoader->script());
ScriptValue exception;
- m_script->evaluate(ScriptSourceCode(scriptLoader.script(), scriptLoader.responseURL()), &exception);
+ m_script->evaluate(ScriptSourceCode(scriptLoader->script(), scriptLoader->responseURL()), &exception);
if (!exception.hasNoValue()) {
m_script->setException(exception);
return;
Modified: trunk/Source/WebCore/workers/WorkerScriptLoader.cpp (88577 => 88578)
--- trunk/Source/WebCore/workers/WorkerScriptLoader.cpp 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebCore/workers/WorkerScriptLoader.cpp 2011-06-11 00:11:01 UTC (rev 88578)
@@ -33,12 +33,16 @@
#include "CrossThreadTask.h"
#include "ResourceRequest.h"
+#include "ResourceResponse.h"
#include "ScriptExecutionContext.h"
#include "SecurityOrigin.h"
+#include "TextResourceDecoder.h"
#include "WorkerContext.h"
#include "WorkerScriptLoaderClient.h"
#include "WorkerThreadableLoader.h"
+
#include <wtf/OwnPtr.h>
+#include <wtf/RefPtr.h>
#include <wtf/UnusedParam.h>
namespace WebCore {
@@ -51,6 +55,10 @@
{
}
+WorkerScriptLoader::~WorkerScriptLoader()
+{
+}
+
void WorkerScriptLoader::loadSynchronously(ScriptExecutionContext* scriptExecutionContext, const KURL& url, CrossOriginRequestPolicy crossOriginRequestPolicy)
{
m_url = url;
@@ -84,6 +92,8 @@
options.crossOriginRequestPolicy = crossOriginRequestPolicy;
options.sendLoadCallbacks = true;
+ // During create, callbacks may happen which remove the last reference to this object.
+ RefPtr<WorkerScriptLoader> protect(this);
m_threadableLoader = ThreadableLoader::create(scriptExecutionContext, this, *request, options);
}
Modified: trunk/Source/WebCore/workers/WorkerScriptLoader.h (88577 => 88578)
--- trunk/Source/WebCore/workers/WorkerScriptLoader.h 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebCore/workers/WorkerScriptLoader.h 2011-06-11 00:11:01 UTC (rev 88578)
@@ -31,20 +31,29 @@
#if ENABLE(WORKERS)
#include "KURL.h"
-#include "ResourceRequest.h"
-#include "ResourceResponse.h"
-#include "TextResourceDecoder.h"
+#include "ResourceRequestBase.h"
#include "ThreadableLoader.h"
#include "ThreadableLoaderClient.h"
+#include <wtf/FastAllocBase.h>
+#include <wtf/PassRefPtr.h>
+#include <wtf/RefCounted.h>
+
namespace WebCore {
+ class ResourceRequest;
+ class ResourceResponse;
class ScriptExecutionContext;
+ class TextResourceDecoder;
class WorkerScriptLoaderClient;
- class WorkerScriptLoader : public ThreadableLoaderClient {
+ class WorkerScriptLoader : public RefCounted<WorkerScriptLoader>, public ThreadableLoaderClient {
+ WTF_MAKE_FAST_ALLOCATED;
public:
- explicit WorkerScriptLoader(ResourceRequestBase::TargetType);
+ static PassRefPtr<WorkerScriptLoader> create(ResourceRequestBase::TargetType targetType)
+ {
+ return adoptRef(new WorkerScriptLoader(targetType));
+ }
void loadSynchronously(ScriptExecutionContext*, const KURL&, CrossOriginRequestPolicy);
void loadAsynchronously(ScriptExecutionContext*, const KURL&, CrossOriginRequestPolicy, WorkerScriptLoaderClient*);
@@ -65,6 +74,11 @@
virtual void didReceiveAuthenticationCancellation(const ResourceResponse&);
private:
+ friend class WTF::RefCounted<WorkerScriptLoader>;
+
+ explicit WorkerScriptLoader(ResourceRequestBase::TargetType);
+ ~WorkerScriptLoader();
+
PassOwnPtr<ResourceRequest> createResourceRequest();
void notifyFinished();
Modified: trunk/Source/WebKit/chromium/ChangeLog (88577 => 88578)
--- trunk/Source/WebKit/chromium/ChangeLog 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebKit/chromium/ChangeLog 2011-06-11 00:11:01 UTC (rev 88578)
@@ -1,3 +1,21 @@
+2011-06-10 David Levin <[email protected]>
+
+ Reviewed by Dmitry Titov.
+
+ Fetching a Worker with url that isn't allowed from a file based test causes DRT to crash.
+ https://bugs.webkit.org/show_bug.cgi?id=62469
+
+ Test: fast/workers/worker-crash-with-invalid-location.html
+
+ * src/SharedWorkerRepository.cpp:
+ (WebCore::SharedWorkerScriptLoader::SharedWorkerScriptLoader): Changed to using the RefCounted version
+ of WorkerScriptLoader.
+ (WebCore::SharedWorkerScriptLoader::load): Rearranged calls as done in similar places,
+ which allows for SharedWorkerScriptLoader to be deleted during the laodAsynchronously call
+ and for unsetPendingActivity to be called.
+ (WebCore::SharedWorkerScriptLoader::notifyFinished): Changed to using the RefCounted version
+ of WorkerScriptLoader.
+
2011-06-10 Ryosuke Niwa <[email protected]>
Rolled DEPS.
Modified: trunk/Source/WebKit/chromium/src/SharedWorkerRepository.cpp (88577 => 88578)
--- trunk/Source/WebKit/chromium/src/SharedWorkerRepository.cpp 2011-06-10 23:55:31 UTC (rev 88577)
+++ trunk/Source/WebKit/chromium/src/SharedWorkerRepository.cpp 2011-06-11 00:11:01 UTC (rev 88578)
@@ -70,7 +70,7 @@
, m_name(name)
, m_webWorker(webWorker)
, m_port(port)
- , m_scriptLoader(ResourceRequestBase::TargetIsSharedWorker)
+ , m_scriptLoader(WorkerScriptLoader::create(ResourceRequestBase::TargetIsSharedWorker))
, m_loading(false)
, m_responseAppCacheID(0)
{
@@ -96,7 +96,7 @@
String m_name;
OwnPtr<WebSharedWorker> m_webWorker;
OwnPtr<MessagePortChannel> m_port;
- WorkerScriptLoader m_scriptLoader;
+ RefPtr<WorkerScriptLoader> m_scriptLoader;
bool m_loading;
long long m_responseAppCacheID;
};
@@ -134,10 +134,11 @@
if (m_webWorker->isStarted())
sendConnect();
else {
- m_scriptLoader.loadAsynchronously(m_worker->scriptExecutionContext(), m_url, DenyCrossOriginRequests, this);
// Keep the worker + JS wrapper alive until the resource load is complete in case we need to dispatch an error event.
m_worker->setPendingActivity(m_worker.get());
m_loading = true;
+
+ m_scriptLoader->loadAsynchronously(m_worker->scriptExecutionContext(), m_url, DenyCrossOriginRequests, this);
}
}
@@ -158,13 +159,13 @@
void SharedWorkerScriptLoader::notifyFinished()
{
- if (m_scriptLoader.failed()) {
+ if (m_scriptLoader->failed()) {
m_worker->dispatchEvent(Event::create(eventNames().errorEvent, false, true));
delete this;
} else {
- InspectorInstrumentation::scriptImported(m_worker->scriptExecutionContext(), m_scriptLoader.identifier(), m_scriptLoader.script());
+ InspectorInstrumentation::scriptImported(m_worker->scriptExecutionContext(), m_scriptLoader->identifier(), m_scriptLoader->script());
// Pass the script off to the worker, then send a connect event.
- m_webWorker->startWorkerContext(m_url, m_name, m_worker->scriptExecutionContext()->userAgent(m_url), m_scriptLoader.script(), m_responseAppCacheID);
+ m_webWorker->startWorkerContext(m_url, m_name, m_worker->scriptExecutionContext()->userAgent(m_url), m_scriptLoader->script(), m_responseAppCacheID);
sendConnect();
}
}
