ScriptExecutionContext::stopActiveDOMObjects iterates a hash map that can change...
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 21 Apr 2014 04:39:11 +0000 (04:39 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 21 Apr 2014 04:39:11 +0000 (04:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=52719

Reviewed by Alexey Proskuryakov.

At least two specific ways this can happen:

1) XMLHttpRequest::stop can trigger a JavaScript garbage collection.
2) NotificationCenter::stop can delete the last references to notifications;
   those notifications are also active DOM objects.

Besides fixing the iteration in that function, did some other fixes for the
ScriptExecutionContext class, including some coding style changes. Many uses
of nullptr instead of 0, without listing each function separately below.

* Modules/webdatabase/DatabaseContext.cpp:
(WebCore::DatabaseContext::contextDestroyed): Call through to the base class
version of contextDestroyed rather than repeating what it does (with a large
comment that doesn't acknowledge the base class alread does it).
* Modules/webdatabase/DatabaseContext.h: Removed some unneeded includes.
Wrote out "private" explicitly for deriving from ActiveDOMObject. Made the
ActiveDOMObject function overrides private, and marked them override and final.

* dom/ActiveDOMObject.h: Updated comments. Replaced suspendIfNeededCalled with
assertSuspendIfNeededWasCalled, which has an empty inline version in the header.
Renamed m_suspendIfNeededCalled to m_suspendIfNeededWasCalled.

* dom/ActiveDOMObject.cpp:
(WebCore::ActiveDOMObject::ActiveDOMObject): Pass a reference instead of a pointer.
(WebCore::ActiveDOMObject::~ActiveDOMObject): Ditto.
(WebCore::ActiveDOMObject::suspendIfNeeded): Ditto.

* dom/ContextDestructionObserver.cpp:
(WebCore::ContextDestructionObserver::observeContext): Pass a reference instead of a pointer.

* dom/MessagePort.cpp:
(WebCore::MessagePort::MessagePort): Pass a reference instead of a pointer.
(WebCore::MessagePort::~MessagePort): Ditto.
(WebCore::MessagePort::disentangle): Ditto.

* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::ScriptExecutionContext): Updated flags used
for assertions so they are conditional and updated their names.
(WebCore::takeAny): Added. Helper function that we can consider for HashSet in
the future; makes loop below easier to read.
(WebCore::checkConsistency): Added. Assertions that were done multiple places below,
and should not be written over and over again.
(WebCore::ScriptExecutionContext::~ScriptExecutionContext): Changed to use C++11
for loops and the takeAny function above.
(WebCore::ScriptExecutionContext::dispatchMessagePortEvents): Ditto.
(WebCore::ScriptExecutionContext::createdMessagePort): Changed to take a reference
for clarity and so it doesn't have to do an assert the pointer is non-null.
(WebCore::ScriptExecutionContext::destroyedMessagePort): Ditto.
(WebCore::ScriptExecutionContext::canSuspendActiveDOMObjects): Changed to use
C++11 for loop and reworded comment and redid assertions.
(WebCore::ScriptExecutionContext::suspendActiveDOMObjects): Ditto.
(WebCore::ScriptExecutionContext::resumeActiveDOMObjects): Ditto.
(WebCore::ScriptExecutionContext::stopActiveDOMObjects): Changed to support
removal of an active DOM object during the stop function. Included new comments
to clarify what the rules are.
(WebCore::ScriptExecutionContext::suspendActiveDOMObjectIfNeeded): Changed to take
a reference for clarity and so it doesn't have to assert a pointer is non-null.
(WebCore::ScriptExecutionContext::didCreateActiveDOMObject): Ditto. Also changed to
use RELEASE_ASSERT instead of CRASH.
(WebCore::ScriptExecutionContext::willDestroyActiveDOMObject): Ditto.
(WebCore::ScriptExecutionContext::didCreateDestructionObserver): Ditto.
(WebCore::ScriptExecutionContext::willDestroyDestructionObserver): Ditto.
(WebCore::ScriptExecutionContext::closeMessagePorts): Moved the body of this
function into its one call site, ScriptExecutionContext::stopActiveDOMObjects,
since it's simple enough when written as a C++11 for loop.
(WebCore::ScriptExecutionContext::hasPendingActivity): Added. This function was
already exported for workers, and implementing it outside this class required
exposing the private HashSet members; more sensible to implement it here and
simply make it public in WorkerGlobalScope.

* dom/ScriptExecutionContext.h: Removed unnecessary includes and forward declarations.
Removed a long-ago-fixed FIXME. Changed various functions to take references instead of
pointers. Added a protected hasPendingActivity function, deleted the closeMessagePorts
function, deleted the ActiveDOMObjectsSet typedef, made the assertion flags be
!ASSERT_DISABLED only, and deleted the messagePorts and activeDOMObjects functions.

* workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::hasPendingActivity): Deleted. This is now implemented
in the base class.

* workers/WorkerGlobalScope.h: Make hasPendingActivity function from the base class
public instead of declaring it in this class.

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/webdatabase/DatabaseContext.cpp
Source/WebCore/Modules/webdatabase/DatabaseContext.h
Source/WebCore/dom/ActiveDOMObject.cpp
Source/WebCore/dom/ActiveDOMObject.h
Source/WebCore/dom/ContextDestructionObserver.cpp
Source/WebCore/dom/MessagePort.cpp
Source/WebCore/dom/ScriptExecutionContext.cpp
Source/WebCore/dom/ScriptExecutionContext.h
Source/WebCore/workers/WorkerGlobalScope.cpp
Source/WebCore/workers/WorkerGlobalScope.h

index 6a81f83..a5a7f79 100644 (file)
@@ -1,3 +1,93 @@
+2014-04-20  Darin Adler  <darin@apple.com>
+
+        ScriptExecutionContext::stopActiveDOMObjects iterates a hash map that can change during iteration (for multiple reasons, including GC)
+        https://bugs.webkit.org/show_bug.cgi?id=52719
+
+        Reviewed by Alexey Proskuryakov.
+
+        At least two specific ways this can happen:
+
+        1) XMLHttpRequest::stop can trigger a JavaScript garbage collection.
+        2) NotificationCenter::stop can delete the last references to notifications;
+           those notifications are also active DOM objects.
+
+        Besides fixing the iteration in that function, did some other fixes for the
+        ScriptExecutionContext class, including some coding style changes. Many uses
+        of nullptr instead of 0, without listing each function separately below.
+
+        * Modules/webdatabase/DatabaseContext.cpp:
+        (WebCore::DatabaseContext::contextDestroyed): Call through to the base class
+        version of contextDestroyed rather than repeating what it does (with a large
+        comment that doesn't acknowledge the base class alread does it).
+        * Modules/webdatabase/DatabaseContext.h: Removed some unneeded includes.
+        Wrote out "private" explicitly for deriving from ActiveDOMObject. Made the
+        ActiveDOMObject function overrides private, and marked them override and final.
+
+        * dom/ActiveDOMObject.h: Updated comments. Replaced suspendIfNeededCalled with
+        assertSuspendIfNeededWasCalled, which has an empty inline version in the header.
+        Renamed m_suspendIfNeededCalled to m_suspendIfNeededWasCalled.
+
+        * dom/ActiveDOMObject.cpp:
+        (WebCore::ActiveDOMObject::ActiveDOMObject): Pass a reference instead of a pointer.
+        (WebCore::ActiveDOMObject::~ActiveDOMObject): Ditto.
+        (WebCore::ActiveDOMObject::suspendIfNeeded): Ditto.
+
+        * dom/ContextDestructionObserver.cpp:
+        (WebCore::ContextDestructionObserver::observeContext): Pass a reference instead of a pointer.
+
+        * dom/MessagePort.cpp:
+        (WebCore::MessagePort::MessagePort): Pass a reference instead of a pointer.
+        (WebCore::MessagePort::~MessagePort): Ditto.
+        (WebCore::MessagePort::disentangle): Ditto.
+
+        * dom/ScriptExecutionContext.cpp:
+        (WebCore::ScriptExecutionContext::ScriptExecutionContext): Updated flags used
+        for assertions so they are conditional and updated their names.
+        (WebCore::takeAny): Added. Helper function that we can consider for HashSet in
+        the future; makes loop below easier to read.
+        (WebCore::checkConsistency): Added. Assertions that were done multiple places below,
+        and should not be written over and over again.
+        (WebCore::ScriptExecutionContext::~ScriptExecutionContext): Changed to use C++11
+        for loops and the takeAny function above.
+        (WebCore::ScriptExecutionContext::dispatchMessagePortEvents): Ditto.
+        (WebCore::ScriptExecutionContext::createdMessagePort): Changed to take a reference
+        for clarity and so it doesn't have to do an assert the pointer is non-null.
+        (WebCore::ScriptExecutionContext::destroyedMessagePort): Ditto.
+        (WebCore::ScriptExecutionContext::canSuspendActiveDOMObjects): Changed to use
+        C++11 for loop and reworded comment and redid assertions.
+        (WebCore::ScriptExecutionContext::suspendActiveDOMObjects): Ditto.
+        (WebCore::ScriptExecutionContext::resumeActiveDOMObjects): Ditto.
+        (WebCore::ScriptExecutionContext::stopActiveDOMObjects): Changed to support
+        removal of an active DOM object during the stop function. Included new comments
+        to clarify what the rules are.
+        (WebCore::ScriptExecutionContext::suspendActiveDOMObjectIfNeeded): Changed to take
+        a reference for clarity and so it doesn't have to assert a pointer is non-null.
+        (WebCore::ScriptExecutionContext::didCreateActiveDOMObject): Ditto. Also changed to
+        use RELEASE_ASSERT instead of CRASH.
+        (WebCore::ScriptExecutionContext::willDestroyActiveDOMObject): Ditto.
+        (WebCore::ScriptExecutionContext::didCreateDestructionObserver): Ditto.
+        (WebCore::ScriptExecutionContext::willDestroyDestructionObserver): Ditto.
+        (WebCore::ScriptExecutionContext::closeMessagePorts): Moved the body of this
+        function into its one call site, ScriptExecutionContext::stopActiveDOMObjects,
+        since it's simple enough when written as a C++11 for loop.
+        (WebCore::ScriptExecutionContext::hasPendingActivity): Added. This function was
+        already exported for workers, and implementing it outside this class required
+        exposing the private HashSet members; more sensible to implement it here and
+        simply make it public in WorkerGlobalScope.
+
+        * dom/ScriptExecutionContext.h: Removed unnecessary includes and forward declarations.
+        Removed a long-ago-fixed FIXME. Changed various functions to take references instead of
+        pointers. Added a protected hasPendingActivity function, deleted the closeMessagePorts
+        function, deleted the ActiveDOMObjectsSet typedef, made the assertion flags be
+        !ASSERT_DISABLED only, and deleted the messagePorts and activeDOMObjects functions.
+
+        * workers/WorkerGlobalScope.cpp:
+        (WebCore::WorkerGlobalScope::hasPendingActivity): Deleted. This is now implemented
+        in the base class.
+
+        * workers/WorkerGlobalScope.h: Make hasPendingActivity function from the base class
+        public instead of declaring it in this class.
+
 2014-04-20  Brent Fulgham  <bfulgham@apple.com>
 
         [Mac] Unable to select 'Off' or 'Auto' from track menu when tracks consist of unsupported track types
index bf730c8..28f076a 100644 (file)
@@ -104,7 +104,7 @@ DatabaseContext::DatabaseContext(ScriptExecutionContext* context)
     , m_hasRequestedTermination(false)
 #if PLATFORM(IOS)
     , m_paused(false)
-#endif //PLATFORM(IOS)
+#endif
 {
     // ActiveDOMObject expects this to be called to set internal flags.
     suspendIfNeeded();
@@ -128,23 +128,15 @@ DatabaseContext::~DatabaseContext()
     DatabaseManager::manager().didDestructDatabaseContext();
 }
 
-// This is called if the associated ScriptExecutionContext is destructing while
+// This is called if the associated ScriptExecutionContext is destroyed while
 // we're still associated with it. That's our cue to disassociate and shutdown.
-// To do this, we stop the database and let everything shutdown naturally
-// because the database closing process may still make use of this context.
+// To do this, we stop the database and let everything shut down naturally
+// because the database closing process might still make use of this context.
 // It is not safe to just delete the context here.
 void DatabaseContext::contextDestroyed()
 {
     stopDatabases();
-
-    // Normally, willDestroyActiveDOMObject() is called in ~ActiveDOMObject().
-    // However, we're here because the destructor hasn't been called, and the
-    // ScriptExecutionContext we're associated with is about to be destructed.
-    // So, go ahead an unregister self from the ActiveDOMObject list, and
-    // set m_scriptExecutionContext to 0 so that ~ActiveDOMObject() doesn't
-    // try to do so again.
-    m_scriptExecutionContext->willDestroyActiveDOMObject(this);
-    m_scriptExecutionContext = 0;
+    ActiveDOMObject::contextDestroyed();
 }
 
 // stop() is from stopActiveDOMObjects() which indicates that the owner Frame
@@ -166,7 +158,7 @@ DatabaseThread* DatabaseContext::databaseThread()
     if (!m_databaseThread && !m_hasOpenDatabases) {
 #if PLATFORM(IOS)
         MutexLocker lock(m_databaseThreadMutex);
-#endif //PLATFORM(IOS)
+#endif
         // It's OK to ask for the m_databaseThread after we've requested
         // termination because we're still using it to execute the closing
         // of the database. However, it is NOT OK to create a new thread
@@ -177,11 +169,12 @@ DatabaseThread* DatabaseContext::databaseThread()
         // because in that case we already had a database thread and terminated it and should not create another.
         m_databaseThread = DatabaseThread::create();
         if (!m_databaseThread->start())
-            m_databaseThread = 0;
+            m_databaseThread = nullptr;
+
 #if PLATFORM(IOS)
         if (m_databaseThread)
             m_databaseThread->setPaused(m_paused);
-#endif //PLATFORM(IOS)
+#endif
     }
 
     return m_databaseThread.get();
@@ -198,7 +191,7 @@ void DatabaseContext::setPaused(bool paused)
 }
 #endif // PLATFORM(IOS)
 
-bool DatabaseContext::stopDatabases(DatabaseTaskSynchronizer* cleanupSync)
+bool DatabaseContext::stopDatabases(DatabaseTaskSynchronizer* synchronizer)
 {
     if (m_isRegistered) {
         DatabaseManager::manager().unregisterDatabaseContext(this);
@@ -216,7 +209,7 @@ bool DatabaseContext::stopDatabases(DatabaseTaskSynchronizer* cleanupSync)
     // DatabaseThread.
 
     if (m_databaseThread && !m_hasRequestedTermination) {
-        m_databaseThread->requestTermination(cleanupSync);
+        m_databaseThread->requestTermination(synchronizer);
         m_hasRequestedTermination = true;
         return true;
     }
index 0203e98..1eac294 100644 (file)
 #if ENABLE(SQL_DATABASE)
 
 #include "ActiveDOMObject.h"
-#include "DatabaseDetails.h"
-#include <wtf/Assertions.h>
+#include <wtf/RefPtr.h>
 #include <wtf/ThreadSafeRefCounted.h>
 
 #if PLATFORM(IOS)
 #include <wtf/Threading.h>
-#endif // PLATFORM(IOS)
+#endif
 
 namespace WebCore {
 
 class Database;
+class DatabaseDetails;
 class DatabaseBackendContext;
 class DatabaseTaskSynchronizer;
 class DatabaseThread;
-class ScriptExecutionContext;
 
-class DatabaseContext : public ThreadSafeRefCounted<DatabaseContext>, ActiveDOMObject {
+class DatabaseContext : public ThreadSafeRefCounted<DatabaseContext>, private ActiveDOMObject {
 public:
     virtual ~DatabaseContext();
 
-    // For life-cycle management (inherited from ActiveDOMObject):
-    virtual void contextDestroyed();
-    virtual void stop();
-
     PassRefPtr<DatabaseBackendContext> backend();
     DatabaseThread* databaseThread();
+
 #if PLATFORM(IOS)
     void setPaused(bool);
-#endif // PLATFORM(IOS)
+#endif
 
     void setHasOpenDatabases() { m_hasOpenDatabases = true; }
     bool hasOpenDatabases() { return m_hasOpenDatabases; }
 
-    // When the database cleanup is done, cleanupSync will be signalled.
+    // When the database cleanup is done, the sychronizer will be signalled.
     bool stopDatabases(DatabaseTaskSynchronizer*);
 
     bool allowDatabaseAccess() const;
@@ -73,7 +69,10 @@ public:
 private:
     explicit DatabaseContext(ScriptExecutionContext*);
 
-    void stopDatabases() { stopDatabases(0); }
+    void stopDatabases() { stopDatabases(nullptr); }
+
+    virtual void contextDestroyed() override final;
+    virtual void stop() override final;
 
     RefPtr<DatabaseThread> m_databaseThread;
     bool m_hasOpenDatabases; // This never changes back to false, even after the database thread is closed.
@@ -86,7 +85,7 @@ private:
 #if PLATFORM(IOS)
     Mutex m_databaseThreadMutex;
     bool m_paused;
-#endif // PLATFORM(IOS)
+#endif
 };
 
 } // namespace WebCore
index 6f04127..299a1a3 100644 (file)
@@ -37,14 +37,14 @@ ActiveDOMObject::ActiveDOMObject(ScriptExecutionContext* scriptExecutionContext)
     : ContextDestructionObserver(scriptExecutionContext)
     , m_pendingActivityCount(0)
 #if !ASSERT_DISABLED
-    , m_suspendIfNeededCalled(false)
+    , m_suspendIfNeededWasCalled(false)
 #endif
 {
     if (!m_scriptExecutionContext)
         return;
 
     ASSERT(m_scriptExecutionContext->isContextThread());
-    m_scriptExecutionContext->didCreateActiveDOMObject(this);
+    m_scriptExecutionContext->didCreateActiveDOMObject(*this);
 }
 
 ActiveDOMObject::~ActiveDOMObject()
@@ -52,7 +52,7 @@ ActiveDOMObject::~ActiveDOMObject()
     if (!m_scriptExecutionContext)
         return;
 
-    ASSERT(m_suspendIfNeededCalled);
+    ASSERT(m_suspendIfNeededWasCalled);
 
     // ActiveDOMObject may be inherited by a sub-class whose life-cycle
     // exceeds that of the associated ScriptExecutionContext. In those cases,
@@ -62,22 +62,31 @@ ActiveDOMObject::~ActiveDOMObject()
     // here.
     if (m_scriptExecutionContext) {
         ASSERT(m_scriptExecutionContext->isContextThread());
-        m_scriptExecutionContext->willDestroyActiveDOMObject(this);
+        m_scriptExecutionContext->willDestroyActiveDOMObject(*this);
     }
 }
 
 void ActiveDOMObject::suspendIfNeeded()
 {
 #if !ASSERT_DISABLED
-    ASSERT(!m_suspendIfNeededCalled);
-    m_suspendIfNeededCalled = true;
+    ASSERT(!m_suspendIfNeededWasCalled);
+    m_suspendIfNeededWasCalled = true;
 #endif
     if (!m_scriptExecutionContext)
         return;
 
-    m_scriptExecutionContext->suspendActiveDOMObjectIfNeeded(this);
+    m_scriptExecutionContext->suspendActiveDOMObjectIfNeeded(*this);
 }
 
+#if !ASSERT_DISABLED
+
+void ActiveDOMObject::assertSuspendIfNeededWasCalled() const
+{
+    ASSERT(m_suspendIfNeededWasCalled);
+}
+
+#endif
+
 bool ActiveDOMObject::hasPendingActivity() const
 {
     return m_pendingActivityCount;
index 6f24ed6..f350b55 100644 (file)
@@ -37,21 +37,21 @@ class ActiveDOMObject : public ContextDestructionObserver {
 public:
     explicit ActiveDOMObject(ScriptExecutionContext*);
 
-    // suspendIfNeeded() should be called exactly once after object construction to synchronize
-    // the suspend state with that in ScriptExecutionContext.
+    // The suspendIfNeeded must be called exactly once after object construction to update
+    // the suspended state to match that of the ScriptExecutionContext.
     void suspendIfNeeded();
-#if !ASSERT_DISABLED
-    bool suspendIfNeededCalled() const { return m_suspendIfNeededCalled; }
-#endif
+    void assertSuspendIfNeededWasCalled() const;
 
     virtual bool hasPendingActivity() const;
 
-    // canSuspend() is used by the caller if there is a choice between suspending and stopping.
-    // For example, a page won't be suspended and placed in the back/forward cache if it has
-    // the objects that can not be suspended.
-    // However, 'suspend' can be called even if canSuspend() would return 'false'. That
-    // happens in step-by-step JS debugging for example - in this case it would be incorrect
-    // to stop the object. Exact semantics of suspend is up to the object then.
+    // The canSuspend function is used by the caller if there is a choice between suspending
+    // and stopping. For example, a page won't be suspended and placed in the back/forward
+    // cache if it contains any objects that cannot be suspended.
+
+    // However, the suspend function will sometimes be called even if canSuspend returns false.
+    // That happens in step-by-step JS debugging for example - in this case it would be incorrect
+    // to stop the object. Exact semantics of suspend is up to the object in cases like that.
+
     enum ReasonForSuspension {
         JavaScriptDebuggerPaused,
         WillDeferLoading,
@@ -59,16 +59,23 @@ public:
         PageWillBeSuspended,
         DocumentWillBePaused
     };
+
+    // These three functions must not have a side effect of creating or destroying
+    // any ActiveDOMObject. That means they must not result in calls to arbitrary JavaScript.
     virtual bool canSuspend() const;
     virtual void suspend(ReasonForSuspension);
     virtual void resume();
+
+    // This function must not have a side effect of creating an ActiveDOMObject.
+    // That means it must not result in calls to arbitrary JavaScript.
+    // It can, however, have a side effect of deleting an ActiveDOMObject.
     virtual void stop();
 
     template<class T> void setPendingActivity(T* thisObject)
     {
         ASSERT(thisObject == this);
         thisObject->ref();
-        m_pendingActivityCount++;
+        ++m_pendingActivityCount;
     }
 
     template<class T> void unsetPendingActivity(T* thisObject)
@@ -84,10 +91,18 @@ protected:
 private:
     unsigned m_pendingActivityCount;
 #if !ASSERT_DISABLED
-    bool m_suspendIfNeededCalled;
+    bool m_suspendIfNeededWasCalled;
 #endif
 };
 
+#if ASSERT_DISABLED
+
+inline void ActiveDOMObject::assertSuspendIfNeededWasCalled() const
+{
+}
+
+#endif
+
 } // namespace WebCore
 
 #endif // ActiveDOMObject_h
index 560c5fb..a333918 100644 (file)
 namespace WebCore {
 
 ContextDestructionObserver::ContextDestructionObserver(ScriptExecutionContext* scriptExecutionContext)
-    : m_scriptExecutionContext(0)
+    : m_scriptExecutionContext(nullptr)
 {
     observeContext(scriptExecutionContext);
 }
 
 ContextDestructionObserver::~ContextDestructionObserver()
 {
-    observeContext(0);
+    observeContext(nullptr);
 }
 
 void ContextDestructionObserver::observeContext(ScriptExecutionContext* scriptExecutionContext)
 {
     if (m_scriptExecutionContext) {
         ASSERT(m_scriptExecutionContext->isContextThread());
-        m_scriptExecutionContext->willDestroyDestructionObserver(this);
+        m_scriptExecutionContext->willDestroyDestructionObserver(*this);
     }
 
     m_scriptExecutionContext = scriptExecutionContext;
 
     if (m_scriptExecutionContext) {
         ASSERT(m_scriptExecutionContext->isContextThread());
-        m_scriptExecutionContext->didCreateDestructionObserver(this);
+        m_scriptExecutionContext->didCreateDestructionObserver(*this);
     }
 }
 
 void ContextDestructionObserver::contextDestroyed()
 {
-    m_scriptExecutionContext = 0;
+    m_scriptExecutionContext = nullptr;
 }
 
 } // namespace WebCore
index dcdf0f8..2ca2b83 100644 (file)
@@ -40,7 +40,7 @@ MessagePort::MessagePort(ScriptExecutionContext& scriptExecutionContext)
     , m_closed(false)
     , m_scriptExecutionContext(&scriptExecutionContext)
 {
-    m_scriptExecutionContext->createdMessagePort(this);
+    m_scriptExecutionContext->createdMessagePort(*this);
 
     // Don't need to call processMessagePortMessagesSoon() here, because the port will not be opened until start() is invoked.
 }
@@ -49,7 +49,7 @@ MessagePort::~MessagePort()
 {
     close();
     if (m_scriptExecutionContext)
-        m_scriptExecutionContext->destroyedMessagePort(this);
+        m_scriptExecutionContext->destroyedMessagePort(*this);
 }
 
 void MessagePort::postMessage(PassRefPtr<SerializedScriptValue> message, MessagePort* port, ExceptionCode& ec)
@@ -89,10 +89,10 @@ std::unique_ptr<MessagePortChannel> MessagePort::disentangle()
 
     m_entangledChannel->disentangle();
 
-    // We can't receive any messages or generate any events, so remove ourselves from the list of active ports.
+    // We can't receive any messages or generate any events after this, so remove ourselves from the list of active ports.
     ASSERT(m_scriptExecutionContext);
-    m_scriptExecutionContext->destroyedMessagePort(this);
-    m_scriptExecutionContext = 0;
+    m_scriptExecutionContext->destroyedMessagePort(*this);
+    m_scriptExecutionContext = nullptr;
 
     return std::move(m_entangledChannel);
 }
index 619f953..ad0228c 100644 (file)
@@ -92,33 +92,72 @@ void ScriptExecutionContext::AddConsoleMessageTask::performTask(ScriptExecutionC
 }
 
 ScriptExecutionContext::ScriptExecutionContext()
-    : m_iteratingActiveDOMObjects(false)
-    , m_inDestructor(false)
-    , m_circularSequentialID(0)
+    : m_circularSequentialID(0)
     , m_inDispatchErrorEvent(false)
     , m_activeDOMObjectsAreSuspended(false)
     , m_reasonForSuspendingActiveDOMObjects(static_cast<ActiveDOMObject::ReasonForSuspension>(-1))
     , m_activeDOMObjectsAreStopped(false)
+    , m_activeDOMObjectAdditionForbidden(false)
+#if !ASSERT_DISABLED
+    , m_inScriptExecutionContextDestructor(false)
+    , m_activeDOMObjectRemovalForbidden(false)
+#endif
 {
 }
 
-ScriptExecutionContext::~ScriptExecutionContext()
+// FIXME: We should make this a member function of HashSet.
+template<typename T> inline T takeAny(HashSet<T>& set)
 {
-    m_inDestructor = true;
-    for (HashSet<ContextDestructionObserver*>::iterator iter = m_destructionObservers.begin(); iter != m_destructionObservers.end(); iter = m_destructionObservers.begin()) {
-        ContextDestructionObserver* observer = *iter;
-        m_destructionObservers.remove(observer);
-        ASSERT(observer->scriptExecutionContext() == this);
-        observer->contextDestroyed();
-    }
+    ASSERT(!set.isEmpty());
+    auto iterator = set.begin();
+    T result = std::move(*iterator);
+    set.remove(iterator);
+    return result;
+}
+
+#if ASSERT_DISABLED
+
+inline void ScriptExecutionContext::checkConsistency() const
+{
+}
+
+#else
 
-    HashSet<MessagePort*>::iterator messagePortsEnd = m_messagePorts.end();
-    for (HashSet<MessagePort*>::iterator iter = m_messagePorts.begin(); iter != messagePortsEnd; ++iter) {
-        ASSERT((*iter)->scriptExecutionContext() == this);
-        (*iter)->contextDestroyed();
+void ScriptExecutionContext::checkConsistency() const
+{
+    for (auto* messagePort : m_messagePorts)
+        ASSERT(messagePort->scriptExecutionContext() == this);
+
+    for (auto* destructionObserver : m_destructionObservers)
+        ASSERT(destructionObserver->scriptExecutionContext() == this);
+
+    for (auto* activeDOMObject : m_activeDOMObjects) {
+        ASSERT(activeDOMObject->scriptExecutionContext() == this);
+        activeDOMObject->assertSuspendIfNeededWasCalled();
     }
 }
 
+#endif
+
+ScriptExecutionContext::~ScriptExecutionContext()
+{
+    checkConsistency();
+
+#if !ASSERT_DISABLED
+    m_inScriptExecutionContextDestructor = true;
+#endif
+
+    while (!m_destructionObservers.isEmpty())
+        takeAny(m_destructionObservers)->contextDestroyed();
+
+    for (auto* messagePort : m_messagePorts)
+        messagePort->contextDestroyed();
+
+#if !ASSERT_DISABLED
+    m_inScriptExecutionContextDestructor = false;
+#endif
+}
+
 void ScriptExecutionContext::processMessagePortMessagesSoon()
 {
     postTask(ProcessMessagesSoonTask::create());
@@ -126,59 +165,72 @@ void ScriptExecutionContext::processMessagePortMessagesSoon()
 
 void ScriptExecutionContext::dispatchMessagePortEvents()
 {
+    checkConsistency();
+
     Ref<ScriptExecutionContext> protect(*this);
 
-    // Make a frozen copy.
-    Vector<MessagePort*> ports;
-    copyToVector(m_messagePorts, ports);
-
-    unsigned portCount = ports.size();
-    for (unsigned i = 0; i < portCount; ++i) {
-        MessagePort* port = ports[i];
-        // The port may be destroyed, and another one created at the same address, but this is safe, as the worst that can happen
-        // as a result is that dispatchMessages() will be called needlessly.
-        if (m_messagePorts.contains(port) && port->started())
-            port->dispatchMessages();
+    // Make a frozen copy of the ports so we can iterate while new ones might be added or destroyed.
+    Vector<MessagePort*> possibleMessagePorts;
+    copyToVector(m_messagePorts, possibleMessagePorts);
+    for (auto* messagePort : possibleMessagePorts) {
+        // The port may be destroyed, and another one created at the same address,
+        // but this is harmless. The worst that can happen as a result is that
+        // dispatchMessages() will be called needlessly.
+        if (m_messagePorts.contains(messagePort) && messagePort->started())
+            messagePort->dispatchMessages();
     }
 }
 
-void ScriptExecutionContext::createdMessagePort(MessagePort* port)
+void ScriptExecutionContext::createdMessagePort(MessagePort& messagePort)
 {
-    ASSERT(port);
     ASSERT((isDocument() && isMainThread())
         || (isWorkerGlobalScope() && currentThread() == toWorkerGlobalScope(this)->thread().threadID()));
 
-    m_messagePorts.add(port);
+    m_messagePorts.add(&messagePort);
 }
 
-void ScriptExecutionContext::destroyedMessagePort(MessagePort* port)
+void ScriptExecutionContext::destroyedMessagePort(MessagePort& messagePort)
 {
-    ASSERT(port);
     ASSERT((isDocument() && isMainThread())
         || (isWorkerGlobalScope() && currentThread() == toWorkerGlobalScope(this)->thread().threadID()));
 
-    m_messagePorts.remove(port);
+    m_messagePorts.remove(&messagePort);
 }
 
 bool ScriptExecutionContext::canSuspendActiveDOMObjects()
 {
-    // No protection against m_activeDOMObjects changing during iteration: canSuspend() shouldn't execute arbitrary JS.
-    m_iteratingActiveDOMObjects = true;
-    ActiveDOMObjectsSet::iterator activeObjectsEnd = m_activeDOMObjects.end();
-    for (ActiveDOMObjectsSet::iterator iter = m_activeDOMObjects.begin(); iter != activeObjectsEnd; ++iter) {
-        ASSERT((*iter)->scriptExecutionContext() == this);
-        ASSERT((*iter)->suspendIfNeededCalled());
-        if (!(*iter)->canSuspend()) {
-            m_iteratingActiveDOMObjects = false;
-            return false;
+    checkConsistency();
+
+    bool canSuspend = true;
+
+    m_activeDOMObjectAdditionForbidden = true;
+#if !ASSERT_DISABLED
+    m_activeDOMObjectRemovalForbidden = true;
+#endif
+
+    // We assume that m_activeDOMObjects will not change during iteration: canSuspend
+    // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
+    // An ASSERT or RELEASE_ASSERT will fire if this happens, but it's important to code
+    // canSuspend functions so it will not happen!
+    for (auto* activeDOMObject : m_activeDOMObjects) {
+        if (!activeDOMObject->canSuspend()) {
+            canSuspend = false;
+            break;
         }
     }
-    m_iteratingActiveDOMObjects = false;
-    return true;
+
+    m_activeDOMObjectAdditionForbidden = false;
+#if !ASSERT_DISABLED
+    m_activeDOMObjectRemovalForbidden = false;
+#endif
+
+    return canSuspend;
 }
 
 void ScriptExecutionContext::suspendActiveDOMObjects(ActiveDOMObject::ReasonForSuspension why)
 {
+    checkConsistency();
+
 #if PLATFORM(IOS)
     if (m_activeDOMObjectsAreSuspended) {
         ASSERT(m_reasonForSuspendingActiveDOMObjects == ActiveDOMObject::DocumentWillBePaused);
@@ -186,101 +238,124 @@ void ScriptExecutionContext::suspendActiveDOMObjects(ActiveDOMObject::ReasonForS
     }
 #endif
 
-    // No protection against m_activeDOMObjects changing during iteration: suspend() shouldn't execute arbitrary JS.
-    m_iteratingActiveDOMObjects = true;
-    ActiveDOMObjectsSet::iterator activeObjectsEnd = m_activeDOMObjects.end();
-    for (ActiveDOMObjectsSet::iterator iter = m_activeDOMObjects.begin(); iter != activeObjectsEnd; ++iter) {
-        ASSERT((*iter)->scriptExecutionContext() == this);
-        ASSERT((*iter)->suspendIfNeededCalled());
-        (*iter)->suspend(why);
-    }
-    m_iteratingActiveDOMObjects = false;
+    m_activeDOMObjectAdditionForbidden = true;
+#if !ASSERT_DISABLED
+    m_activeDOMObjectRemovalForbidden = true;
+#endif
+
+    // We assume that m_activeDOMObjects will not change during iteration: suspend
+    // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
+    // An ASSERT or RELEASE_ASSERT will fire if this happens, but it's important to code
+    // suspend functions so it will not happen!
+    for (auto* activeDOMObject : m_activeDOMObjects)
+        activeDOMObject->suspend(why);
+
+    m_activeDOMObjectAdditionForbidden = false;
+#if !ASSERT_DISABLED
+    m_activeDOMObjectRemovalForbidden = false;
+#endif
+
     m_activeDOMObjectsAreSuspended = true;
     m_reasonForSuspendingActiveDOMObjects = why;
 }
 
 void ScriptExecutionContext::resumeActiveDOMObjects(ActiveDOMObject::ReasonForSuspension why)
 {
+    checkConsistency();
+
     if (m_reasonForSuspendingActiveDOMObjects != why)
         return;
-
     m_activeDOMObjectsAreSuspended = false;
-    // No protection against m_activeDOMObjects changing during iteration: resume() shouldn't execute arbitrary JS.
-    m_iteratingActiveDOMObjects = true;
-    ActiveDOMObjectsSet::iterator activeObjectsEnd = m_activeDOMObjects.end();
-    for (ActiveDOMObjectsSet::iterator iter = m_activeDOMObjects.begin(); iter != activeObjectsEnd; ++iter) {
-        ASSERT((*iter)->scriptExecutionContext() == this);
-        ASSERT((*iter)->suspendIfNeededCalled());
-        (*iter)->resume();
-    }
-    m_iteratingActiveDOMObjects = false;
+
+    m_activeDOMObjectAdditionForbidden = true;
+#if !ASSERT_DISABLED
+    m_activeDOMObjectRemovalForbidden = true;
+#endif
+
+    // We assume that m_activeDOMObjects will not change during iteration: resume
+    // functions should not add new active DOM objects, nor execute arbitrary JavaScript.
+    // An ASSERT or RELEASE_ASSERT will fire if this happens, but it's important to code
+    // resume functions so it will not happen!
+    for (auto* activeDOMObject : m_activeDOMObjects)
+        activeDOMObject->resume();
+
+    m_activeDOMObjectAdditionForbidden = false;
+#if !ASSERT_DISABLED
+    m_activeDOMObjectRemovalForbidden = false;
+#endif
 }
 
 void ScriptExecutionContext::stopActiveDOMObjects()
 {
+    checkConsistency();
+
     if (m_activeDOMObjectsAreStopped)
         return;
     m_activeDOMObjectsAreStopped = true;
-    // No protection against m_activeDOMObjects changing during iteration: stop() shouldn't execute arbitrary JS.
-    m_iteratingActiveDOMObjects = true;
-    ActiveDOMObjectsSet::iterator activeObjectsEnd = m_activeDOMObjects.end();
-    for (ActiveDOMObjectsSet::iterator iter = m_activeDOMObjects.begin(); iter != activeObjectsEnd; ++iter) {
-        ASSERT((*iter)->scriptExecutionContext() == this);
-        ASSERT((*iter)->suspendIfNeededCalled());
-        (*iter)->stop();
+
+    // Make a frozen copy of the objects so we can iterate while new ones might be destroyed.
+    Vector<ActiveDOMObject*> possibleActiveDOMObjects;
+    copyToVector(m_activeDOMObjects, possibleActiveDOMObjects);
+
+    m_activeDOMObjectAdditionForbidden = true;
+
+    // We assume that new objects will not be added to m_activeDOMObjects during iteration:
+    // stop functions should not add new active DOM objects, nor execute arbitrary JavaScript.
+    // A RELEASE_ASSERT will fire if this happens, but it's important to code stop functions
+    // so it will not happen!
+    for (auto* activeDOMObject : possibleActiveDOMObjects) {
+        // Check if this object was deleted already. If so, just skip it.
+        // Calling contains on a possibly-already-deleted object is OK because we guarantee
+        // no new object can be added, so even if a new object ends up allocated with the
+        // same address, that will be *after* this function exits.
+        if (!m_activeDOMObjects.contains(activeDOMObject))
+            continue;
+        activeDOMObject->stop();
     }
-    m_iteratingActiveDOMObjects = false;
 
-    // Also close MessagePorts. If they were ActiveDOMObjects (they could be) then they could be stopped instead.
-    closeMessagePorts();
+    m_activeDOMObjectAdditionForbidden = false;
+
+    // FIXME: Make message ports be active DOM objects and let them implement stop instead
+    // of having this separate mechanism just for them.
+    for (auto* messagePort : m_messagePorts)
+        messagePort->close();
 }
 
-void ScriptExecutionContext::suspendActiveDOMObjectIfNeeded(ActiveDOMObject* object)
+void ScriptExecutionContext::suspendActiveDOMObjectIfNeeded(ActiveDOMObject& activeDOMObject)
 {
-    ASSERT(m_activeDOMObjects.contains(object));
-    // Ensure all ActiveDOMObjects are suspended also newly created ones.
+    ASSERT(m_activeDOMObjects.contains(&activeDOMObject));
     if (m_activeDOMObjectsAreSuspended)
-        object->suspend(m_reasonForSuspendingActiveDOMObjects);
+        activeDOMObject.suspend(m_reasonForSuspendingActiveDOMObjects);
     if (m_activeDOMObjectsAreStopped)
-        object->stop();
+        activeDOMObject.stop();
 }
 
-void ScriptExecutionContext::didCreateActiveDOMObject(ActiveDOMObject* object)
+void ScriptExecutionContext::didCreateActiveDOMObject(ActiveDOMObject& activeDOMObject)
 {
-    ASSERT(object);
-    ASSERT(!m_inDestructor);
-    if (m_iteratingActiveDOMObjects)
-        CRASH();
-    m_activeDOMObjects.add(object);
+    // The m_activeDOMObjectAdditionForbidden check is a RELEASE_ASSERT because of the
+    // consequences of having an ActiveDOMObject that is not correctly reflected in the set.
+    // If we do have one of those, it can possibly be a security vulnerability. So we'd
+    // rather have a crash than continue running with the set possibly compromised.
+    ASSERT(!m_inScriptExecutionContextDestructor);
+    RELEASE_ASSERT(!m_activeDOMObjectAdditionForbidden);
+    m_activeDOMObjects.add(&activeDOMObject);
 }
 
-void ScriptExecutionContext::willDestroyActiveDOMObject(ActiveDOMObject* object)
+void ScriptExecutionContext::willDestroyActiveDOMObject(ActiveDOMObject& activeDOMObject)
 {
-    ASSERT(object);
-    if (m_iteratingActiveDOMObjects)
-        CRASH();
-    m_activeDOMObjects.remove(object);
+    ASSERT(!m_activeDOMObjectRemovalForbidden);
+    m_activeDOMObjects.remove(&activeDOMObject);
 }
 
-void ScriptExecutionContext::didCreateDestructionObserver(ContextDestructionObserver* observer)
+void ScriptExecutionContext::didCreateDestructionObserver(ContextDestructionObserver& observer)
 {
-    ASSERT(observer);
-    ASSERT(!m_inDestructor);
-    m_destructionObservers.add(observer);
+    ASSERT(!m_inScriptExecutionContextDestructor);
+    m_destructionObservers.add(&observer);
 }
 
-void ScriptExecutionContext::willDestroyDestructionObserver(ContextDestructionObserver* observer)
+void ScriptExecutionContext::willDestroyDestructionObserver(ContextDestructionObserver& observer)
 {
-    ASSERT(observer);
-    m_destructionObservers.remove(observer);
-}
-
-void ScriptExecutionContext::closeMessagePorts() {
-    HashSet<MessagePort*>::iterator messagePortsEnd = m_messagePorts.end();
-    for (HashSet<MessagePort*>::iterator iter = m_messagePorts.begin(); iter != messagePortsEnd; ++iter) {
-        ASSERT((*iter)->scriptExecutionContext() == this);
-        (*iter)->close();
-    }
+    m_destructionObservers.remove(&observer);
 }
 
 bool ScriptExecutionContext::sanitizeScriptError(String& errorMessage, int& lineNumber, int& columnNumber, String& sourceURL, CachedScript* cachedScript)
@@ -369,10 +444,8 @@ PublicURLManager& ScriptExecutionContext::publicURLManager()
 void ScriptExecutionContext::adjustMinimumTimerInterval(double oldMinimumTimerInterval)
 {
     if (minimumTimerInterval() != oldMinimumTimerInterval) {
-        for (TimeoutMap::iterator iter = m_timeouts.begin(); iter != m_timeouts.end(); ++iter) {
-            DOMTimer* timer = iter->value;
+        for (auto* timer : m_timeouts.values())
             timer->adjustMinimumTimerInterval(oldMinimumTimerInterval);
-        }
     }
 }
 
@@ -388,10 +461,8 @@ double ScriptExecutionContext::minimumTimerInterval() const
 
 void ScriptExecutionContext::didChangeTimerAlignmentInterval()
 {
-    for (TimeoutMap::iterator iter = m_timeouts.begin(); iter != m_timeouts.end(); ++iter) {
-        DOMTimer* timer = iter->value;
+    for (auto* timer : m_timeouts.values())
         timer->didChangeAlignmentInterval();
-    }
 }
 
 double ScriptExecutionContext::timerAlignmentInterval() const
@@ -419,4 +490,21 @@ void ScriptExecutionContext::setDatabaseContext(DatabaseContext* databaseContext
 }
 #endif
 
+bool ScriptExecutionContext::hasPendingActivity() const
+{
+    checkConsistency();
+
+    for (auto* activeDOMObject : m_activeDOMObjects) {
+        if (activeDOMObject->hasPendingActivity())
+            return true;
+    }
+
+    for (auto* messagePort : m_messagePorts) {
+        if (messagePort->hasPendingActivity())
+            return true;
+    }
+
+    return false;
+}
+
 } // namespace WebCore
index 40e4b79..28f8dbb 100644 (file)
@@ -31,7 +31,6 @@
 #include "ActiveDOMObject.h"
 #include "SecurityContext.h"
 #include "Supplementable.h"
-#include "URL.h"
 #include <runtime/ConsoleTypes.h>
 #include <wtf/HashSet.h>
 
@@ -49,14 +48,11 @@ namespace WebCore {
 class CachedScript;
 class DatabaseContext;
 class DOMTimer;
-class EventListener;
 class EventQueue;
 class EventTarget;
 class MessagePort;
-
-#if ENABLE(BLOB)
 class PublicURLManager;
-#endif
+class URL;
 
 class ScriptExecutionContext : public SecurityContext, public Supplementable<ScriptExecutionContext> {
 public:
@@ -76,11 +72,10 @@ public:
 
     virtual void disableEval(const String& errorMessage) = 0;
 
-    bool sanitizeScriptError(String& errorMessage, int& lineNumber, int& columnNumber, String& sourceURL, CachedScript* = 0);
-    // FIXME: <http://webkit.org/b/114315> ScriptExecutionContext log exception should include a column number
-    void reportException(const String& errorMessage, int lineNumber, int columnNumber, const String& sourceURL, PassRefPtr<Inspector::ScriptCallStack>, CachedScript* = 0);
+    bool sanitizeScriptError(String& errorMessage, int& lineNumber, int& columnNumber, String& sourceURL, CachedScript* = nullptr);
+    void reportException(const String& errorMessage, int lineNumber, int columnNumber, const String& sourceURL, PassRefPtr<Inspector::ScriptCallStack>, CachedScript* = nullptr);
 
-    void addConsoleMessage(MessageSource, MessageLevel, const String& message, const String& sourceURL, unsigned lineNumber, unsigned columnNumber, JSC::ExecState* = 0, unsigned long requestIdentifier = 0);
+    void addConsoleMessage(MessageSource, MessageLevel, const String& message, const String& sourceURL, unsigned lineNumber, unsigned columnNumber, JSC::ExecState* = nullptr, unsigned long requestIdentifier = 0);
     virtual void addConsoleMessage(MessageSource, MessageLevel, const String& message, unsigned long requestIdentifier = 0) = 0;
 
     virtual SecurityOrigin* topOrigin() const = 0;
@@ -101,24 +96,20 @@ public:
     bool activeDOMObjectsAreStopped() const { return m_activeDOMObjectsAreStopped; }
 
     // Called from the constructor and destructors of ActiveDOMObject.
-    void didCreateActiveDOMObject(ActiveDOMObject*);
-    void willDestroyActiveDOMObject(ActiveDOMObject*);
+    void didCreateActiveDOMObject(ActiveDOMObject&);
+    void willDestroyActiveDOMObject(ActiveDOMObject&);
 
     // Called after the construction of an ActiveDOMObject to synchronize suspend state.
-    void suspendActiveDOMObjectIfNeeded(ActiveDOMObject*);
+    void suspendActiveDOMObjectIfNeeded(ActiveDOMObject&);
 
-    typedef HashSet<ActiveDOMObject*> ActiveDOMObjectsSet;
-    const ActiveDOMObjectsSet& activeDOMObjects() const { return m_activeDOMObjects; }
-
-    void didCreateDestructionObserver(ContextDestructionObserver*);
-    void willDestroyDestructionObserver(ContextDestructionObserver*);
+    void didCreateDestructionObserver(ContextDestructionObserver&);
+    void willDestroyDestructionObserver(ContextDestructionObserver&);
 
     // MessagePort is conceptually a kind of ActiveDOMObject, but it needs to be tracked separately for message dispatch.
     void processMessagePortMessagesSoon();
     void dispatchMessagePortEvents();
-    void createdMessagePort(MessagePort*);
-    void destroyedMessagePort(MessagePort*);
-    const HashSet<MessagePort*>& messagePorts() const { return m_messagePorts; }
+    void createdMessagePort(MessagePort&);
+    void destroyedMessagePort(MessagePort&);
 
     void ref() { refScriptExecutionContext(); }
     void deref() { derefScriptExecutionContext(); }
@@ -185,26 +176,25 @@ protected:
 
     ActiveDOMObject::ReasonForSuspension reasonForSuspendingActiveDOMObjects() const { return m_reasonForSuspendingActiveDOMObjects; }
 
+    bool hasPendingActivity() const;
+
 private:
-    virtual void addMessage(MessageSource, MessageLevel, const String& message, const String& sourceURL, unsigned lineNumber, unsigned columnNumber, PassRefPtr<Inspector::ScriptCallStack>, JSC::ExecState* = 0, unsigned long requestIdentifier = 0) = 0;
+    virtual void addMessage(MessageSource, MessageLevel, const String& message, const String& sourceURL, unsigned lineNumber, unsigned columnNumber, PassRefPtr<Inspector::ScriptCallStack>, JSC::ExecState* = nullptr, unsigned long requestIdentifier = 0) = 0;
     virtual EventTarget* errorEventTarget() = 0;
     virtual void logExceptionToConsole(const String& errorMessage, const String& sourceURL, int lineNumber, int columnNumber, PassRefPtr<Inspector::ScriptCallStack>) = 0;
     bool dispatchErrorEvent(const String& errorMessage, int lineNumber, int columnNumber, const String& sourceURL, CachedScript*);
 
-    void closeMessagePorts();
-
     virtual void refScriptExecutionContext() = 0;
     virtual void derefScriptExecutionContext() = 0;
 
+    void checkConsistency() const;
+
     HashSet<MessagePort*> m_messagePorts;
     HashSet<ContextDestructionObserver*> m_destructionObservers;
-    ActiveDOMObjectsSet m_activeDOMObjects;
-    bool m_iteratingActiveDOMObjects;
-    bool m_inDestructor;
+    HashSet<ActiveDOMObject*> m_activeDOMObjects;
 
     int m_circularSequentialID;
-    typedef HashMap<int, DOMTimer*> TimeoutMap;
-    TimeoutMap m_timeouts;
+    HashMap<int, DOMTimer*> m_timeouts;
 
     bool m_inDispatchErrorEvent;
     class PendingException;
@@ -221,6 +211,13 @@ private:
 #if ENABLE(SQL_DATABASE)
     RefPtr<DatabaseContext> m_databaseContext;
 #endif
+
+    bool m_activeDOMObjectAdditionForbidden;
+
+#if !ASSERT_DISABLED
+    bool m_inScriptExecutionContextDestructor;
+    bool m_activeDOMObjectRemovalForbidden;
+#endif
 };
 
 #define SCRIPT_EXECUTION_CONTEXT_TYPE_CASTS(ToValueTypeName) \
index 2b5bdff..b6639bc 100644 (file)
@@ -159,23 +159,6 @@ WorkerNavigator* WorkerGlobalScope::navigator() const
     return m_navigator.get();
 }
 
-bool WorkerGlobalScope::hasPendingActivity() const
-{
-    ActiveDOMObjectsSet::const_iterator activeObjectsEnd = activeDOMObjects().end();
-    for (ActiveDOMObjectsSet::const_iterator iter = activeDOMObjects().begin(); iter != activeObjectsEnd; ++iter) {
-        if ((*iter)->hasPendingActivity())
-            return true;
-    }
-
-    HashSet<MessagePort*>::const_iterator messagePortsEnd = messagePorts().end();
-    for (HashSet<MessagePort*>::const_iterator iter = messagePorts().begin(); iter != messagePortsEnd; ++iter) {
-        if ((*iter)->hasPendingActivity())
-            return true;
-    }
-
-    return false;
-}
-
 void WorkerGlobalScope::postTask(PassOwnPtr<Task> task)
 {
     thread().runLoop().postTask(task);
index 1765d97..19d20ee 100644 (file)
@@ -76,7 +76,7 @@ namespace WebCore {
 
         WorkerThread& thread() const { return m_thread; }
 
-        bool hasPendingActivity() const;
+        using ScriptExecutionContext::hasPendingActivity;
 
         virtual void postTask(PassOwnPtr<Task>) override; // Executes the task on context's thread asynchronously.