[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.20-204-g221d8e8
eric at webkit.org
eric at webkit.org
Wed Feb 10 22:11:18 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit a9fe2e447bafa49a5cbf4907f2ca23439a3dada2
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Feb 3 20:14:38 2010 +0000
2010-02-03 Drew Wilson <atwilson at chromium.org>
Reviewed by Alexey Proskuryakov.
SharedWorkerScriptLoader should not be an ActiveDOMObject
https://bugs.webkit.org/show_bug.cgi?id=34513
Test: Existing tests suffice (fixes test downstream in Chrome).
* workers/DefaultSharedWorkerRepository.cpp:
(WebCore::SharedWorkerScriptLoader::SharedWorkerScriptLoader):
Changed to no longer derive from ActiveDOMObject (handles its own refcounting).
(WebCore::SharedWorkerScriptLoader::load):
Now increments own refcount when a load is pending.
(WebCore::SharedWorkerScriptLoader::notifyFinished):
Changed to decrement refcount when load is complete.
* workers/WorkerScriptLoaderClient.h:
Documentation change about reliability of notifyFinished() when used from worker context.
2010-02-03 Drew Wilson <atwilson at chromium.org>
Reviewed by Alexey Proskuryakov.
SharedWorkerScriptLoader should not be an ActiveDOMObject
https://bugs.webkit.org/show_bug.cgi?id=34513
* src/SharedWorkerRepository.cpp:
(WebCore::SharedWorkerScriptLoader::SharedWorkerScriptLoader):
Changed SharedWorkerScriptLoader to manage its own lifecycle without using ActiveDOMObject.
(WebCore::SharedWorkerScriptLoader::parentContext):
(WebCore::pendingLoaders):
Now we manually track pending loads so we can shut them down when the parent context shuts down.
(WebCore::SharedWorkerScriptLoader::contextDetached):
Shuts down/frees any pending worker loads.
(WebCore::SharedWorkerScriptLoader::~SharedWorkerScriptLoader):
Marks the SharedWorker object as not having pending activity if there was a load active (handles case where load was pending when parent document exits).
(WebCore::SharedWorkerScriptLoader::load):
(WebCore::SharedWorkerRepository::documentDetached):
Now calls SharedWorkerScriptLoader::contextDetached() to shutdown any pending worker loads.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@54292 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index bacd408..fbacc54 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2010-02-03 Drew Wilson <atwilson at chromium.org>
+
+ Reviewed by Alexey Proskuryakov.
+
+ SharedWorkerScriptLoader should not be an ActiveDOMObject
+ https://bugs.webkit.org/show_bug.cgi?id=34513
+
+ Test: Existing tests suffice (fixes test downstream in Chrome).
+
+ * workers/DefaultSharedWorkerRepository.cpp:
+ (WebCore::SharedWorkerScriptLoader::SharedWorkerScriptLoader):
+ Changed to no longer derive from ActiveDOMObject (handles its own refcounting).
+ (WebCore::SharedWorkerScriptLoader::load):
+ Now increments own refcount when a load is pending.
+ (WebCore::SharedWorkerScriptLoader::notifyFinished):
+ Changed to decrement refcount when load is complete.
+ * workers/WorkerScriptLoaderClient.h:
+ Documentation change about reliability of notifyFinished() when used from worker context.
+
2010-02-03 Pavel Feldman <pfeldman at chromium.org>
Reviewed by Timothy Hatcher.
diff --git a/WebCore/workers/DefaultSharedWorkerRepository.cpp b/WebCore/workers/DefaultSharedWorkerRepository.cpp
index 655b90e..e4fa5d3 100644
--- a/WebCore/workers/DefaultSharedWorkerRepository.cpp
+++ b/WebCore/workers/DefaultSharedWorkerRepository.cpp
@@ -248,7 +248,7 @@ private:
};
// Loads the script on behalf of a worker.
-class SharedWorkerScriptLoader : public RefCounted<SharedWorkerScriptLoader>, public ActiveDOMObject, private WorkerScriptLoaderClient {
+class SharedWorkerScriptLoader : public RefCounted<SharedWorkerScriptLoader>, private WorkerScriptLoaderClient {
public:
SharedWorkerScriptLoader(PassRefPtr<SharedWorker>, PassOwnPtr<MessagePortChannel>, PassRefPtr<SharedWorkerProxy>);
void load(const KURL&);
@@ -264,8 +264,7 @@ private:
};
SharedWorkerScriptLoader::SharedWorkerScriptLoader(PassRefPtr<SharedWorker> worker, PassOwnPtr<MessagePortChannel> port, PassRefPtr<SharedWorkerProxy> proxy)
- : ActiveDOMObject(worker->scriptExecutionContext(), this)
- , m_worker(worker)
+ : m_worker(worker)
, m_port(port)
, m_proxy(proxy)
{
@@ -274,25 +273,27 @@ SharedWorkerScriptLoader::SharedWorkerScriptLoader(PassRefPtr<SharedWorker> work
void SharedWorkerScriptLoader::load(const KURL& url)
{
// Mark this object as active for the duration of the load.
- ASSERT(!hasPendingActivity());
m_scriptLoader = new WorkerScriptLoader();
- m_scriptLoader->loadAsynchronously(scriptExecutionContext(), url, DenyCrossOriginRequests, this);
+ m_scriptLoader->loadAsynchronously(m_worker->scriptExecutionContext(), url, DenyCrossOriginRequests, this);
- // Stay alive until the load finishes.
- setPendingActivity(this);
+ // Stay alive (and keep the SharedWorker and JS wrapper alive) until the load finishes.
+ this->ref();
m_worker->setPendingActivity(m_worker.get());
}
void SharedWorkerScriptLoader::notifyFinished()
{
+ // FIXME: This method is not guaranteed to be invoked if we are loading from WorkerContext (see comment for WorkerScriptLoaderClient::notifyFinished()).
+ // We need to address this before supporting nested workers.
+
// Hand off the just-loaded code to the repository to start up the worker thread.
if (m_scriptLoader->failed())
m_worker->dispatchEvent(Event::create(eventNames().errorEvent, false, true));
else
- DefaultSharedWorkerRepository::instance().workerScriptLoaded(*m_proxy, scriptExecutionContext()->userAgent(m_scriptLoader->url()), m_scriptLoader->script(), m_port.release());
+ DefaultSharedWorkerRepository::instance().workerScriptLoaded(*m_proxy, m_worker->scriptExecutionContext()->userAgent(m_scriptLoader->url()), m_scriptLoader->script(), m_port.release());
m_worker->unsetPendingActivity(m_worker.get());
- unsetPendingActivity(this); // This frees this object - must be the last action in this function.
+ this->deref(); // This frees this object - must be the last action in this function.
}
DefaultSharedWorkerRepository& DefaultSharedWorkerRepository::instance()
diff --git a/WebCore/workers/WorkerScriptLoaderClient.h b/WebCore/workers/WorkerScriptLoaderClient.h
index e3903c0..7dc3a1e 100644
--- a/WebCore/workers/WorkerScriptLoaderClient.h
+++ b/WebCore/workers/WorkerScriptLoaderClient.h
@@ -34,8 +34,10 @@ namespace WebCore {
class WorkerScriptLoaderClient {
public:
+ // FIXME: notifyFinished() is not currently guaranteed to be invoked if used from worker context and the worker shuts down in the middle of an operation.
+ // This will cause leaks when we support nested workers.
virtual void notifyFinished() { }
-
+
protected:
virtual ~WorkerScriptLoaderClient() { }
};
diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
index b1c592b..956d6bd 100644
--- a/WebKit/chromium/ChangeLog
+++ b/WebKit/chromium/ChangeLog
@@ -1,3 +1,24 @@
+2010-02-03 Drew Wilson <atwilson at chromium.org>
+
+ Reviewed by Alexey Proskuryakov.
+
+ SharedWorkerScriptLoader should not be an ActiveDOMObject
+ https://bugs.webkit.org/show_bug.cgi?id=34513
+
+ * src/SharedWorkerRepository.cpp:
+ (WebCore::SharedWorkerScriptLoader::SharedWorkerScriptLoader):
+ Changed SharedWorkerScriptLoader to manage its own lifecycle without using ActiveDOMObject.
+ (WebCore::SharedWorkerScriptLoader::parentContext):
+ (WebCore::pendingLoaders):
+ Now we manually track pending loads so we can shut them down when the parent context shuts down.
+ (WebCore::SharedWorkerScriptLoader::contextDetached):
+ Shuts down/frees any pending worker loads.
+ (WebCore::SharedWorkerScriptLoader::~SharedWorkerScriptLoader):
+ Marks the SharedWorker object as not having pending activity if there was a load active (handles case where load was pending when parent document exits).
+ (WebCore::SharedWorkerScriptLoader::load):
+ (WebCore::SharedWorkerRepository::documentDetached):
+ Now calls SharedWorkerScriptLoader::contextDetached() to shutdown any pending worker loads.
+
2010-02-03 Alexander Pavlov <apavlov at chromium.org>
Reviewed by Timothy Hatcher.
diff --git a/WebKit/chromium/src/SharedWorkerRepository.cpp b/WebKit/chromium/src/SharedWorkerRepository.cpp
index 5e5bc46..c803aac 100644
--- a/WebKit/chromium/src/SharedWorkerRepository.cpp
+++ b/WebKit/chromium/src/SharedWorkerRepository.cpp
@@ -61,26 +61,30 @@ using WebKit::WebSharedWorker;
using WebKit::WebSharedWorkerRepository;
// Callback class that keeps the SharedWorker and WebSharedWorker objects alive while loads are potentially happening, and also translates load errors into error events on the worker.
-class SharedWorkerScriptLoader : private WorkerScriptLoaderClient, private WebSharedWorker::ConnectListener, private ActiveDOMObject {
+class SharedWorkerScriptLoader : private WorkerScriptLoaderClient, private WebSharedWorker::ConnectListener {
public:
SharedWorkerScriptLoader(PassRefPtr<SharedWorker> worker, const KURL& url, const String& name, PassOwnPtr<MessagePortChannel> port, PassOwnPtr<WebSharedWorker> webWorker)
- : ActiveDOMObject(worker->scriptExecutionContext(), this)
- , m_worker(worker)
+ : m_worker(worker)
, m_url(url)
, m_name(name)
, m_webWorker(webWorker)
, m_port(port)
+ , m_loading(false)
{
}
+ ~SharedWorkerScriptLoader();
void load();
- virtual void contextDestroyed();
+ static void stopAllLoadersForContext(ScriptExecutionContext*);
+
private:
// WorkerScriptLoaderClient callback
virtual void notifyFinished();
virtual void connected();
+ const ScriptExecutionContext* loadingContext() { return m_worker->scriptExecutionContext(); }
+
void sendConnect();
RefPtr<SharedWorker> m_worker;
@@ -89,15 +93,47 @@ private:
OwnPtr<WebSharedWorker> m_webWorker;
OwnPtr<MessagePortChannel> m_port;
WorkerScriptLoader m_scriptLoader;
+ bool m_loading;
};
+static Vector<SharedWorkerScriptLoader*>& pendingLoaders()
+{
+ AtomicallyInitializedStatic(Vector<SharedWorkerScriptLoader*>&, loaders = *new Vector<SharedWorkerScriptLoader*>);
+ return loaders;
+}
+
+void SharedWorkerScriptLoader::stopAllLoadersForContext(ScriptExecutionContext* context)
+{
+ // Walk our list of pending loaders and shutdown any that belong to this context.
+ Vector<SharedWorkerScriptLoader*>& loaders = pendingLoaders();
+ for (unsigned i = 0; i < loaders.size(); ) {
+ SharedWorkerScriptLoader* loader = loaders[i];
+ if (context == loader->loadingContext()) {
+ loaders.remove(i);
+ delete loader;
+ } else
+ i++;
+ }
+}
+
+SharedWorkerScriptLoader::~SharedWorkerScriptLoader()
+{
+ if (m_loading)
+ m_worker->unsetPendingActivity(m_worker.get());
+}
+
void SharedWorkerScriptLoader::load()
{
+ ASSERT(!m_loading);
// If the shared worker is not yet running, load the script resource for it, otherwise just send it a connect event.
if (m_webWorker->isStarted())
sendConnect();
- else
+ 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;
+ }
}
// Extracts a WebMessagePortChannel from a MessagePortChannel.
@@ -128,12 +164,6 @@ void SharedWorkerScriptLoader::sendConnect()
m_webWorker->connect(getWebPort(m_port.release()), this);
}
-void SharedWorkerScriptLoader::contextDestroyed()
-{
- ActiveDOMObject::contextDestroyed();
- delete this;
-}
-
void SharedWorkerScriptLoader::connected()
{
// Connect event has been sent, so free ourselves (this releases the SharedWorker so it can be freed as well if unreferenced).
@@ -185,6 +215,10 @@ void SharedWorkerRepository::documentDetached(Document* document)
WebSharedWorkerRepository* repo = WebKit::webKitClient()->sharedWorkerRepository();
if (repo)
repo->documentDetached(getId(document));
+
+ // Stop the creation of any pending SharedWorkers for this context.
+ // FIXME: Need a way to invoke this for WorkerContexts as well when we support for nested workers.
+ SharedWorkerScriptLoader::stopAllLoadersForContext(document);
}
bool SharedWorkerRepository::hasSharedWorkers(Document* document)
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list