Commit e57cf263 authored by atwilson@chromium.org's avatar atwilson@chromium.org

WebCore: SharedWorkers do not exit when the last parent document exits

https://bugs.webkit.org/show_bug.cgi?id=28170

Reviewed by David Levin.

Prevents Documents from being suspended/placed in the page cache if they are associated with shared workers.

Added vector cache instead of nested hash tables for SharedWorker repository.

Added SharedWorkerRepository::documentDetached API.

* dom/Document.cpp:
(WebCore::Document::detach):
Notifies SharedWorkerRepository when the document is closing.
* loader/FrameLoader.cpp:
Updated FrameLoader to not cache the Document if it is associated with a SharedWorker (since we can't suspend workers yet, we need to shut them down).
(WebCore::FrameLoader::canCachePageContainingThisFrame):
(WebCore::FrameLoader::logCanCacheFrameDecision):
* workers/DefaultSharedWorkerRepository.cpp:
(WebCore::SharedWorkerProxy::create):
(WebCore::SharedWorkerProxy::isClosing):
Renamed from closing().
(WebCore::SharedWorkerProxy::matches):
Added manual equality function to replace old hash lookup.
(WebCore::SharedWorkerProxy::isDocumentInWorkerDocuments):
Checks to see if a document is in the worker's list of documents. Used to determine if page is suspendable.
(WebCore::SharedWorkerProxy::SharedWorkerProxy):
(WebCore::SharedWorkerProxy::addToWorkerDocuments):
Added tracking of the worker's list of documents for lifecycle purposes.
(WebCore::SharedWorkerProxy::documentDetached):
Shuts down the proxy when the last associated document is detached.
(WebCore::SharedWorkerProxy::close):
Marks the proxy as closed so it is no longer shared with new instances.
(WebCore::SharedWorkerProxy::workerContextDestroyed):
Removes the proxy from the repository/frees the proxy when the associated SharedWorkerContext is destroyed.
(WebCore::DefaultSharedWorkerRepository::workerScriptLoaded):
closing()->isClosing()
(WebCore::SharedWorkerRepository::documentDetached):
(WebCore::SharedWorkerRepository::hasSharedWorkers):
Used by FrameLoader to determine if a page has shared workers and so cannot be suspended/cached.
(WebCore::DefaultSharedWorkerRepository::hasSharedWorkers):
(WebCore::DefaultSharedWorkerRepository::removeProxy):
Invoked by workerContextDestroyed() to remove a SharedWorkerProxy from the repository.
(WebCore::DefaultSharedWorkerRepository::documentDetached):
(WebCore::DefaultSharedWorkerRepository::connectToWorker):
(WebCore::DefaultSharedWorkerRepository::getProxy):
* workers/DefaultSharedWorkerRepository.h:
* workers/SharedWorkerRepository.h:

LayoutTests: SharedWorkers do not exit when the last parent document exits.
https://bugs.webkit.org/show_bug.cgi?id=28170

Reviewed by David Levin.

Added more tests to check that previous incarnations of the SharedWorker "name" are shut down.

* fast/workers/shared-worker-replace-global-constructor.html-disabled:
Fixed incorrect path to common script.
* fast/workers/shared-worker-shared-expected.txt:
* fast/workers/shared-worker-shared.html-disabled:
Added more tests for sharing, including initial test to make sure previous incarnations of shared worker were closed.
* fast/workers/worker-replace-global-constructor.html:
Removed extraneous closing HTML tag.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@47078 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 30a60681
2009-08-11 Drew Wilson <atwilson@google.com>
Reviewed by David Levin.
SharedWorkers do not exit when the last parent document exits.
https://bugs.webkit.org/show_bug.cgi?id=28170
Added more tests to check that previous incarnations of the SharedWorker "name" are shut down.
* fast/workers/shared-worker-replace-global-constructor.html-disabled:
Fixed incorrect path to common script.
* fast/workers/shared-worker-shared-expected.txt:
* fast/workers/shared-worker-shared.html-disabled:
Added more tests for sharing, including initial test to make sure previous incarnations of shared worker were closed.
* fast/workers/worker-replace-global-constructor.html:
Removed extraneous closing HTML tag.
2009-08-11 Chris Fleizach <cfleizach@apple.com>
Reviewed by Darin Adler.
......
......@@ -2,6 +2,6 @@
<p>Test replacing global constructors in a worker context.</p>
<div id=result></div>
<script src="resources/shared-worker-create-common.js"></script>
<script src="worker-replace-global-constructor.js">
<script src="resources/worker-replace-global-constructor.js">
</script>
</body>
Test simple shared worker sharing cases. Should print several PASS lines followed by DONE.
PASS: Exception thrown when creating SharedWorker with different URLs but same name: Error: URL_MISMATCH_ERR: DOM Exception 21
PASS: Accessing new instance of shared worker: self.foo: undefined
PASS: Setting global variable in shared worker: self.foo = 1234: 1234
PASS: Accessing simultaneously-loaded instance of shared worker: self.foo: 1234
PASS: Accessing new instance of shared worker: self.foo: undefined
......
......@@ -13,6 +13,7 @@ if (window.layoutTestController) {
}
// Load two workers simultaneously, to ensure that simultaneous loads also yield the same instance.
// Loading a worker named "name" tests that workers shutdown when the parent document exits, because other tests also create workers with that same name but with different URLs.
var worker = new SharedWorker('resources/shared-worker-common.js', 'name');
var worker2 = new SharedWorker('resources/shared-worker-common.js', 'name');
......@@ -25,16 +26,20 @@ try {
// Set something in global context in one worker, read value back on other worker, to make sure they are truly shared.
worker.port.postMessage("eval self.foo = 1234");
worker.port.postMessage("eval self.foo");
worker.port.onmessage = function(event)
{
log((event.data == "self.foo = 1234: 1234" ? "PASS: " : "FAIL: ") + "Setting global variable in shared worker: " + event.data);
worker2.port.postMessage("eval self.foo");
worker2.port.onmessage = function(event)
log((event.data == "self.foo: undefined" ? "PASS: " : "FAIL: ") + "Accessing new instance of shared worker: " + event.data);
worker.port.postMessage("eval self.foo = 1234");
worker.port.onmessage = function(event)
{
log((event.data == "self.foo: 1234" ? "PASS: " : "FAIL: ") + "Accessing simultaneously-loaded instance of shared worker: " + event.data);
testNewWorker();
log((event.data == "self.foo = 1234: 1234" ? "PASS: " : "FAIL: ") + "Setting global variable in shared worker: " + event.data);
worker2.port.postMessage("eval self.foo");
worker2.port.onmessage = function(event)
{
log((event.data == "self.foo: 1234" ? "PASS: " : "FAIL: ") + "Accessing simultaneously-loaded instance of shared worker: " + event.data);
testNewWorker();
}
}
}
......
......@@ -5,4 +5,3 @@
<script src="resources/worker-replace-global-constructor.js">
</script>
</body>
</html>
2009-08-11 Drew Wilson <atwilson@google.com>
Reviewed by David Levin.
SharedWorkers do not exit when the last parent document exits
https://bugs.webkit.org/show_bug.cgi?id=28170
Prevents Documents from being suspended/placed in the page cache if they are associated with shared workers.
Added vector cache instead of nested hash tables for SharedWorker repository.
Added SharedWorkerRepository::documentDetached API.
* dom/Document.cpp:
(WebCore::Document::detach):
Notifies SharedWorkerRepository when the document is closing.
* loader/FrameLoader.cpp:
Updated FrameLoader to not cache the Document if it is associated with a SharedWorker (since we can't suspend workers yet, we need to shut them down).
(WebCore::FrameLoader::canCachePageContainingThisFrame):
(WebCore::FrameLoader::logCanCacheFrameDecision):
* workers/DefaultSharedWorkerRepository.cpp:
(WebCore::SharedWorkerProxy::create):
(WebCore::SharedWorkerProxy::isClosing):
Renamed from closing().
(WebCore::SharedWorkerProxy::matches):
Added manual equality function to replace old hash lookup.
(WebCore::SharedWorkerProxy::isDocumentInWorkerDocuments):
Checks to see if a document is in the worker's list of documents. Used to determine if page is suspendable.
(WebCore::SharedWorkerProxy::SharedWorkerProxy):
(WebCore::SharedWorkerProxy::addToWorkerDocuments):
Added tracking of the worker's list of documents for lifecycle purposes.
(WebCore::SharedWorkerProxy::documentDetached):
Shuts down the proxy when the last associated document is detached.
(WebCore::SharedWorkerProxy::close):
Marks the proxy as closed so it is no longer shared with new instances.
(WebCore::SharedWorkerProxy::workerContextDestroyed):
Removes the proxy from the repository/frees the proxy when the associated SharedWorkerContext is destroyed.
(WebCore::DefaultSharedWorkerRepository::workerScriptLoaded):
closing()->isClosing()
(WebCore::SharedWorkerRepository::documentDetached):
(WebCore::SharedWorkerRepository::hasSharedWorkers):
Used by FrameLoader to determine if a page has shared workers and so cannot be suspended/cached.
(WebCore::DefaultSharedWorkerRepository::hasSharedWorkers):
(WebCore::DefaultSharedWorkerRepository::removeProxy):
Invoked by workerContextDestroyed() to remove a SharedWorkerProxy from the repository.
(WebCore::DefaultSharedWorkerRepository::documentDetached):
(WebCore::DefaultSharedWorkerRepository::connectToWorker):
(WebCore::DefaultSharedWorkerRepository::getProxy):
* workers/DefaultSharedWorkerRepository.h:
* workers/SharedWorkerRepository.h:
2009-08-11 Chris Fleizach <cfleizach@apple.com>
Reviewed by Darin Adler.
......
......@@ -108,6 +108,7 @@
#include "SegmentedString.h"
#include "SelectionController.h"
#include "Settings.h"
#include "SharedWorkerRepository.h"
#include "StyleSheetList.h"
#include "TextEvent.h"
#include "TextIterator.h"
......@@ -1348,7 +1349,11 @@ void Document::detach()
// Send out documentWillBecomeInactive() notifications to registered elements,
// in order to stop media elements
documentWillBecomeInactive();
#if ENABLE(SHARED_WORKERS)
SharedWorkerRepository::documentDetached(this);
#endif
if (m_frame) {
FrameView* view = m_frame->view();
if (view)
......
......@@ -87,6 +87,7 @@
#include "SecurityOrigin.h"
#include "SegmentedString.h"
#include "Settings.h"
#include "SharedWorkerRepository.h"
#include "TextResourceDecoder.h"
#include "WindowFeatures.h"
#include "XMLHttpRequest.h"
......@@ -1811,6 +1812,9 @@ bool FrameLoader::canCachePageContainingThisFrame()
&& (!m_frame->domWindow() || !m_frame->domWindow()->hasEventListener(eventNames().unloadEvent))
#if ENABLE(DATABASE)
&& !m_frame->document()->hasOpenDatabases()
#endif
#if ENABLE(SHARED_WORKERS)
&& !SharedWorkerRepository::hasSharedWorkers(m_frame->document())
#endif
&& !m_frame->document()->usingGeolocation()
&& m_currentHistoryItem
......@@ -1957,6 +1961,10 @@ bool FrameLoader::logCanCacheFrameDecision(int indentLevel)
#if ENABLE(DATABASE)
if (m_frame->document()->hasOpenDatabases())
{ PCLOG(" -Frame has open database handles"); cannotCache = true; }
#endif
#if ENABLE(SHARED_WORKERS)
if (SharedWorkerRepository::hasSharedWorkers(m_frame->document()))
{ PCLOG(" -Frame has associated SharedWorkers"); cannotCache = true; }
#endif
if (m_frame->document()->usingGeolocation())
{ PCLOG(" -Frame uses Geolocation"); cannotCache = true; }
......
......@@ -35,6 +35,7 @@
#include "DefaultSharedWorkerRepository.h"
#include "ActiveDOMObject.h"
#include "Document.h"
#include "MessagePort.h"
#include "NotImplemented.h"
#include "PlatformString.h"
......@@ -42,25 +43,28 @@
#include "SecurityOriginHash.h"
#include "SharedWorker.h"
#include "SharedWorkerContext.h"
#include "SharedWorkerRepository.h"
#include "SharedWorkerThread.h"
#include "WorkerLoaderProxy.h"
#include "WorkerReportingProxy.h"
#include "WorkerScriptLoader.h"
#include "WorkerScriptLoaderClient.h"
#include <wtf/HashSet.h>
#include <wtf/Threading.h>
namespace WebCore {
class SharedWorkerProxy : public ThreadSafeShared<SharedWorkerProxy>, public WorkerLoaderProxy, public WorkerReportingProxy {
public:
static PassRefPtr<SharedWorkerProxy> create(const String& name, const KURL& url) { return adoptRef(new SharedWorkerProxy(name, url)); }
static PassRefPtr<SharedWorkerProxy> create(const String& name, const KURL& url, PassRefPtr<SecurityOrigin> origin) { return adoptRef(new SharedWorkerProxy(name, url, origin)); }
void setThread(PassRefPtr<SharedWorkerThread> thread) { m_thread = thread; }
SharedWorkerThread* thread() { return m_thread.get(); }
bool closing() const { return m_closing; }
bool isClosing() const { return m_closing; }
void close();
KURL url() const { return m_url.copy(); }
String name() const { return m_name.copy(); }
bool matches(const String& name, PassRefPtr<SecurityOrigin> origin) const { return name == m_name && origin->equal(m_origin.get()); }
// WorkerLoaderProxy
// FIXME: Implement WorkerLoaderProxy APIs by proxying to an active document.
......@@ -76,19 +80,30 @@ public:
// Updates the list of the worker's documents, per section 4.5 of the WebWorkers spec.
void addToWorkerDocuments(ScriptExecutionContext*);
bool isInWorkerDocuments(Document* document) { return m_workerDocuments.contains(document); }
// Removes a detached document from the list of worker's documents. May set the closing flag if this is the last document in the list.
void documentDetached(Document*);
private:
SharedWorkerProxy(const String& name, const KURL&);
SharedWorkerProxy(const String& name, const KURL&, PassRefPtr<SecurityOrigin>);
bool m_closing;
String m_name;
KURL m_url;
// The thread is freed when the proxy is destroyed, so we need to make sure that the proxy stays around until the SharedWorkerContext exits.
RefPtr<SharedWorkerThread> m_thread;
RefPtr<SecurityOrigin> m_origin;
HashSet<Document*> m_workerDocuments;
};
SharedWorkerProxy::SharedWorkerProxy(const String& name, const KURL& url)
SharedWorkerProxy::SharedWorkerProxy(const String& name, const KURL& url, PassRefPtr<SecurityOrigin> origin)
: m_closing(false)
, m_name(name.copy())
, m_url(url.copy())
, m_origin(origin)
{
// We should be the sole owner of the SecurityOrigin, as we will free it on another thread.
ASSERT(m_origin->hasOneRef());
}
void SharedWorkerProxy::postExceptionToWorkerObject(const String&, int, const String&)
......@@ -118,7 +133,52 @@ void SharedWorkerProxy::addToWorkerDocuments(ScriptExecutionContext* context)
{
// Nested workers are not yet supported, so passed-in context should always be a Document.
ASSERT(context->isDocument());
// FIXME: track referring documents so we can shutdown the thread when the last one exits and remove the proxy from the cache.
ASSERT(!isClosing());
Document* document = static_cast<Document*>(context);
m_workerDocuments.add(document);
}
void SharedWorkerProxy::documentDetached(Document* document)
{
if (isClosing())
return;
// Remove the document from our set (if it's there) and if that was the last document in the set, mark the proxy as closed.
m_workerDocuments.remove(document);
if (!m_workerDocuments.size())
close();
}
void SharedWorkerProxy::close()
{
ASSERT(!isClosing());
m_closing = true;
// Stop the worker thread - the proxy will stay around until we get workerThreadExited() notification.
if (m_thread)
m_thread->stop();
}
void SharedWorkerProxy::postExceptionToWorkerObject(const String&, int, const String&)
{
// FIXME: Log exceptions to all parent documents.
notImplemented();
}
void SharedWorkerProxy::postConsoleMessageToWorkerObject(MessageDestination, MessageSource, MessageType, MessageLevel, const String&, int, const String&)
{
// FIXME: Log console messages to all parent documents.
notImplemented();
}
// When close() is invoked, mark the proxy as closing so we don't share it with any new requests.
void SharedWorkerProxy::workerContextClosed()
{
m_closing = true;
}
void SharedWorkerProxy::workerContextDestroyed()
{
// The proxy may be freed by this call, so do not reference it any further.
DefaultSharedWorkerRepository::instance().removeProxy(this);
}
class SharedWorkerConnectTask : public ScriptExecutionContext::Task {
......@@ -203,7 +263,7 @@ DefaultSharedWorkerRepository& DefaultSharedWorkerRepository::instance()
void DefaultSharedWorkerRepository::workerScriptLoaded(SharedWorkerProxy& proxy, const String& userAgent, const String& workerScript, PassOwnPtr<MessagePortChannel> port)
{
MutexLocker lock(m_lock);
if (proxy.closing())
if (proxy.isClosing())
return;
// Another loader may have already started up a thread for this proxy - if so, just send a connect to the pre-existing thread.
......@@ -220,6 +280,44 @@ void SharedWorkerRepository::connect(PassRefPtr<SharedWorker> worker, PassOwnPtr
DefaultSharedWorkerRepository::instance().connectToWorker(worker, port, url, name, ec);
}
void SharedWorkerRepository::documentDetached(Document* document)
{
DefaultSharedWorkerRepository::instance().documentDetached(document);
}
bool SharedWorkerRepository::hasSharedWorkers(Document* document)
{
return DefaultSharedWorkerRepository::instance().hasSharedWorkers(document);
}
bool DefaultSharedWorkerRepository::hasSharedWorkers(Document* document)
{
MutexLocker lock(m_lock);
for (unsigned i = 0; i < m_proxies.size(); i++) {
if (m_proxies[i]->isInWorkerDocuments(document))
return true;
}
return false;
}
void DefaultSharedWorkerRepository::removeProxy(SharedWorkerProxy* proxy)
{
MutexLocker lock(m_lock);
for (unsigned i = 0; i < m_proxies.size(); i++) {
if (proxy == m_proxies[i].get()) {
m_proxies.remove(i);
return;
}
}
}
void DefaultSharedWorkerRepository::documentDetached(Document* document)
{
MutexLocker lock(m_lock);
for (unsigned i = 0; i < m_proxies.size(); i++)
m_proxies[i]->documentDetached(document);
}
void DefaultSharedWorkerRepository::connectToWorker(PassRefPtr<SharedWorker> worker, PassOwnPtr<MessagePortChannel> port, const KURL& url, const String& name, ExceptionCode& ec)
{
MutexLocker lock(m_lock);
......@@ -247,18 +345,14 @@ PassRefPtr<SharedWorkerProxy> DefaultSharedWorkerRepository::getProxy(const Stri
// Look for an existing worker, and create one if it doesn't exist.
// Items in the cache are freed on another thread, so copy the URL before creating the origin, to make sure no references to external strings linger.
RefPtr<SecurityOrigin> origin = SecurityOrigin::create(url.copy());
SharedWorkerNameMap* nameMap = m_cache.get(origin);
if (!nameMap) {
nameMap = new SharedWorkerNameMap();
m_cache.set(origin, nameMap);
}
RefPtr<SharedWorkerProxy> proxy = nameMap->get(name);
if (!proxy.get()) {
proxy = SharedWorkerProxy::create(name, url);
nameMap->set(proxy->name(), proxy);
for (unsigned i = 0; i < m_proxies.size(); i++) {
if (!m_proxies[i]->isClosing() && m_proxies[i]->matches(name, origin))
return m_proxies[i];
}
return proxy;
// Proxy is not in the repository currently - create a new one.
RefPtr<SharedWorkerProxy> proxy = SharedWorkerProxy::create(name, url, origin.release());
m_proxies.append(proxy);
return proxy.release();
}
DefaultSharedWorkerRepository::DefaultSharedWorkerRepository()
......
......@@ -33,22 +33,24 @@
#if ENABLE(SHARED_WORKERS)
#include "SharedWorkerRepository.h"
#include "ExceptionCode.h"
#include "StringHash.h"
#include <wtf/HashMap.h>
#include <wtf/Noncopyable.h>
#include <wtf/PassOwnPtr.h>
#include <wtf/PassRefPtr.h>
#include <wtf/RefPtr.h>
#include <wtf/Threading.h>
namespace WebCore {
class Document;
class KURL;
class MessagePortChannel;
class ScriptExecutionContext;
class SecurityOrigin;
class SharedWorker;
class SharedWorkerProxy;
struct SecurityOriginHash;
struct SecurityOriginTraits;
class String;
// Platform-specific implementation of the SharedWorkerRepository static interface.
class DefaultSharedWorkerRepository : public Noncopyable {
......@@ -59,6 +61,14 @@ namespace WebCore {
// Internal implementation of SharedWorkerRepository::connect()
void connectToWorker(PassRefPtr<SharedWorker>, PassOwnPtr<MessagePortChannel>, const KURL&, const String& name, ExceptionCode&);
// Notification that a document has been detached.
void documentDetached(Document*);
// Removes the passed SharedWorkerProxy from the repository.
void removeProxy(SharedWorkerProxy*);
bool hasSharedWorkers(Document*);
static DefaultSharedWorkerRepository& instance();
private:
DefaultSharedWorkerRepository();
......@@ -68,11 +78,9 @@ namespace WebCore {
// Mutex used to protect internal data structures.
Mutex m_lock;
typedef HashMap<String, RefPtr<SharedWorkerProxy> > SharedWorkerNameMap;
typedef HashMap<RefPtr<SecurityOrigin>, SharedWorkerNameMap*, SecurityOriginHash> SharedWorkerProxyCache;
// Items in this cache may be freed on another thread, so all keys and values must be either copied before insertion or thread safe.
SharedWorkerProxyCache m_cache;
// List of shared workers. Expectation is that there will be a limited number of shared workers, and so tracking them in a Vector is more efficient than nested HashMaps.
typedef Vector<RefPtr<SharedWorkerProxy> > SharedWorkerProxyRepository;
SharedWorkerProxyRepository m_proxies;
};
} // namespace WebCore
......
......@@ -34,12 +34,12 @@
#if ENABLE(SHARED_WORKERS)
#include "ExceptionCode.h"
#include <wtf/PassOwnPtr.h>
#include <wtf/PassRefPtr.h>
namespace WebCore {
class Document;
class KURL;
class MessagePortChannel;
class SharedWorker;
......@@ -50,6 +50,12 @@ namespace WebCore {
public:
// Connects the passed SharedWorker object with the specified worker thread, creating a new thread if necessary.
static void connect(PassRefPtr<SharedWorker>, PassOwnPtr<MessagePortChannel>, const KURL&, const String& name, ExceptionCode&);
// Invoked when a document has been detached.
static void documentDetached(Document*);
// Returns true if the passed document is associated with any SharedWorkers.
static bool hasSharedWorkers(Document*);
private:
SharedWorkerRepository() { }
};
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment