<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[167579] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/167579">167579</a></dd>
<dt>Author</dt> <dd>darin@apple.com</dd>
<dt>Date</dt> <dd>2014-04-20 21:39:11 -0700 (Sun, 20 Apr 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModuleswebdatabaseDatabaseContextcpp">trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp</a></li>
<li><a href="#trunkSourceWebCoreModuleswebdatabaseDatabaseContexth">trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.h</a></li>
<li><a href="#trunkSourceWebCoredomActiveDOMObjectcpp">trunk/Source/WebCore/dom/ActiveDOMObject.cpp</a></li>
<li><a href="#trunkSourceWebCoredomActiveDOMObjecth">trunk/Source/WebCore/dom/ActiveDOMObject.h</a></li>
<li><a href="#trunkSourceWebCoredomContextDestructionObservercpp">trunk/Source/WebCore/dom/ContextDestructionObserver.cpp</a></li>
<li><a href="#trunkSourceWebCoredomMessagePortcpp">trunk/Source/WebCore/dom/MessagePort.cpp</a></li>
<li><a href="#trunkSourceWebCoredomScriptExecutionContextcpp">trunk/Source/WebCore/dom/ScriptExecutionContext.cpp</a></li>
<li><a href="#trunkSourceWebCoredomScriptExecutionContexth">trunk/Source/WebCore/dom/ScriptExecutionContext.h</a></li>
<li><a href="#trunkSourceWebCoreworkersWorkerGlobalScopecpp">trunk/Source/WebCore/workers/WorkerGlobalScope.cpp</a></li>
<li><a href="#trunkSourceWebCoreworkersWorkerGlobalScopeh">trunk/Source/WebCore/workers/WorkerGlobalScope.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/ChangeLog 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -1,3 +1,93 @@
</span><ins>+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.
+
</ins><span class="cx"> 2014-04-20 Brent Fulgham <bfulgham@apple.com>
</span><span class="cx">
</span><span class="cx"> [Mac] Unable to select 'Off' or 'Auto' from track menu when tracks consist of unsupported track types
</span></span></pre></div>
<a id="trunkSourceWebCoreModuleswebdatabaseDatabaseContextcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.cpp 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -104,7 +104,7 @@
</span><span class="cx"> , m_hasRequestedTermination(false)
</span><span class="cx"> #if PLATFORM(IOS)
</span><span class="cx"> , m_paused(false)
</span><del>-#endif //PLATFORM(IOS)
</del><ins>+#endif
</ins><span class="cx"> {
</span><span class="cx"> // ActiveDOMObject expects this to be called to set internal flags.
</span><span class="cx"> suspendIfNeeded();
</span><span class="lines">@@ -128,23 +128,15 @@
</span><span class="cx"> DatabaseManager::manager().didDestructDatabaseContext();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-// This is called if the associated ScriptExecutionContext is destructing while
</del><ins>+// This is called if the associated ScriptExecutionContext is destroyed while
</ins><span class="cx"> // we're still associated with it. That's our cue to disassociate and shutdown.
</span><del>-// To do this, we stop the database and let everything shutdown naturally
-// because the database closing process may still make use of this context.
</del><ins>+// 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.
</ins><span class="cx"> // It is not safe to just delete the context here.
</span><span class="cx"> void DatabaseContext::contextDestroyed()
</span><span class="cx"> {
</span><span class="cx"> stopDatabases();
</span><del>-
- // 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;
</del><ins>+ ActiveDOMObject::contextDestroyed();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // stop() is from stopActiveDOMObjects() which indicates that the owner Frame
</span><span class="lines">@@ -166,7 +158,7 @@
</span><span class="cx"> if (!m_databaseThread && !m_hasOpenDatabases) {
</span><span class="cx"> #if PLATFORM(IOS)
</span><span class="cx"> MutexLocker lock(m_databaseThreadMutex);
</span><del>-#endif //PLATFORM(IOS)
</del><ins>+#endif
</ins><span class="cx"> // It's OK to ask for the m_databaseThread after we've requested
</span><span class="cx"> // termination because we're still using it to execute the closing
</span><span class="cx"> // of the database. However, it is NOT OK to create a new thread
</span><span class="lines">@@ -177,11 +169,12 @@
</span><span class="cx"> // because in that case we already had a database thread and terminated it and should not create another.
</span><span class="cx"> m_databaseThread = DatabaseThread::create();
</span><span class="cx"> if (!m_databaseThread->start())
</span><del>- m_databaseThread = 0;
</del><ins>+ m_databaseThread = nullptr;
+
</ins><span class="cx"> #if PLATFORM(IOS)
</span><span class="cx"> if (m_databaseThread)
</span><span class="cx"> m_databaseThread->setPaused(m_paused);
</span><del>-#endif //PLATFORM(IOS)
</del><ins>+#endif
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> return m_databaseThread.get();
</span><span class="lines">@@ -198,7 +191,7 @@
</span><span class="cx"> }
</span><span class="cx"> #endif // PLATFORM(IOS)
</span><span class="cx">
</span><del>-bool DatabaseContext::stopDatabases(DatabaseTaskSynchronizer* cleanupSync)
</del><ins>+bool DatabaseContext::stopDatabases(DatabaseTaskSynchronizer* synchronizer)
</ins><span class="cx"> {
</span><span class="cx"> if (m_isRegistered) {
</span><span class="cx"> DatabaseManager::manager().unregisterDatabaseContext(this);
</span><span class="lines">@@ -216,7 +209,7 @@
</span><span class="cx"> // DatabaseThread.
</span><span class="cx">
</span><span class="cx"> if (m_databaseThread && !m_hasRequestedTermination) {
</span><del>- m_databaseThread->requestTermination(cleanupSync);
</del><ins>+ m_databaseThread->requestTermination(synchronizer);
</ins><span class="cx"> m_hasRequestedTermination = true;
</span><span class="cx"> return true;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoreModuleswebdatabaseDatabaseContexth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.h (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.h 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseContext.h 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -31,40 +31,36 @@
</span><span class="cx"> #if ENABLE(SQL_DATABASE)
</span><span class="cx">
</span><span class="cx"> #include "ActiveDOMObject.h"
</span><del>-#include "DatabaseDetails.h"
-#include <wtf/Assertions.h>
</del><ins>+#include <wtf/RefPtr.h>
</ins><span class="cx"> #include <wtf/ThreadSafeRefCounted.h>
</span><span class="cx">
</span><span class="cx"> #if PLATFORM(IOS)
</span><span class="cx"> #include <wtf/Threading.h>
</span><del>-#endif // PLATFORM(IOS)
</del><ins>+#endif
</ins><span class="cx">
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><span class="cx"> class Database;
</span><ins>+class DatabaseDetails;
</ins><span class="cx"> class DatabaseBackendContext;
</span><span class="cx"> class DatabaseTaskSynchronizer;
</span><span class="cx"> class DatabaseThread;
</span><del>-class ScriptExecutionContext;
</del><span class="cx">
</span><del>-class DatabaseContext : public ThreadSafeRefCounted<DatabaseContext>, ActiveDOMObject {
</del><ins>+class DatabaseContext : public ThreadSafeRefCounted<DatabaseContext>, private ActiveDOMObject {
</ins><span class="cx"> public:
</span><span class="cx"> virtual ~DatabaseContext();
</span><span class="cx">
</span><del>- // For life-cycle management (inherited from ActiveDOMObject):
- virtual void contextDestroyed();
- virtual void stop();
-
</del><span class="cx"> PassRefPtr<DatabaseBackendContext> backend();
</span><span class="cx"> DatabaseThread* databaseThread();
</span><ins>+
</ins><span class="cx"> #if PLATFORM(IOS)
</span><span class="cx"> void setPaused(bool);
</span><del>-#endif // PLATFORM(IOS)
</del><ins>+#endif
</ins><span class="cx">
</span><span class="cx"> void setHasOpenDatabases() { m_hasOpenDatabases = true; }
</span><span class="cx"> bool hasOpenDatabases() { return m_hasOpenDatabases; }
</span><span class="cx">
</span><del>- // When the database cleanup is done, cleanupSync will be signalled.
</del><ins>+ // When the database cleanup is done, the sychronizer will be signalled.
</ins><span class="cx"> bool stopDatabases(DatabaseTaskSynchronizer*);
</span><span class="cx">
</span><span class="cx"> bool allowDatabaseAccess() const;
</span><span class="lines">@@ -73,8 +69,11 @@
</span><span class="cx"> private:
</span><span class="cx"> explicit DatabaseContext(ScriptExecutionContext*);
</span><span class="cx">
</span><del>- void stopDatabases() { stopDatabases(0); }
</del><ins>+ void stopDatabases() { stopDatabases(nullptr); }
</ins><span class="cx">
</span><ins>+ virtual void contextDestroyed() override final;
+ virtual void stop() override final;
+
</ins><span class="cx"> RefPtr<DatabaseThread> m_databaseThread;
</span><span class="cx"> bool m_hasOpenDatabases; // This never changes back to false, even after the database thread is closed.
</span><span class="cx"> bool m_isRegistered;
</span><span class="lines">@@ -86,7 +85,7 @@
</span><span class="cx"> #if PLATFORM(IOS)
</span><span class="cx"> Mutex m_databaseThreadMutex;
</span><span class="cx"> bool m_paused;
</span><del>-#endif // PLATFORM(IOS)
</del><ins>+#endif
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCoredomActiveDOMObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ActiveDOMObject.cpp (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ActiveDOMObject.cpp 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/dom/ActiveDOMObject.cpp 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -37,14 +37,14 @@
</span><span class="cx"> : ContextDestructionObserver(scriptExecutionContext)
</span><span class="cx"> , m_pendingActivityCount(0)
</span><span class="cx"> #if !ASSERT_DISABLED
</span><del>- , m_suspendIfNeededCalled(false)
</del><ins>+ , m_suspendIfNeededWasCalled(false)
</ins><span class="cx"> #endif
</span><span class="cx"> {
</span><span class="cx"> if (!m_scriptExecutionContext)
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> ASSERT(m_scriptExecutionContext->isContextThread());
</span><del>- m_scriptExecutionContext->didCreateActiveDOMObject(this);
</del><ins>+ m_scriptExecutionContext->didCreateActiveDOMObject(*this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ActiveDOMObject::~ActiveDOMObject()
</span><span class="lines">@@ -52,7 +52,7 @@
</span><span class="cx"> if (!m_scriptExecutionContext)
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- ASSERT(m_suspendIfNeededCalled);
</del><ins>+ ASSERT(m_suspendIfNeededWasCalled);
</ins><span class="cx">
</span><span class="cx"> // ActiveDOMObject may be inherited by a sub-class whose life-cycle
</span><span class="cx"> // exceeds that of the associated ScriptExecutionContext. In those cases,
</span><span class="lines">@@ -62,22 +62,31 @@
</span><span class="cx"> // here.
</span><span class="cx"> if (m_scriptExecutionContext) {
</span><span class="cx"> ASSERT(m_scriptExecutionContext->isContextThread());
</span><del>- m_scriptExecutionContext->willDestroyActiveDOMObject(this);
</del><ins>+ m_scriptExecutionContext->willDestroyActiveDOMObject(*this);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ActiveDOMObject::suspendIfNeeded()
</span><span class="cx"> {
</span><span class="cx"> #if !ASSERT_DISABLED
</span><del>- ASSERT(!m_suspendIfNeededCalled);
- m_suspendIfNeededCalled = true;
</del><ins>+ ASSERT(!m_suspendIfNeededWasCalled);
+ m_suspendIfNeededWasCalled = true;
</ins><span class="cx"> #endif
</span><span class="cx"> if (!m_scriptExecutionContext)
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- m_scriptExecutionContext->suspendActiveDOMObjectIfNeeded(this);
</del><ins>+ m_scriptExecutionContext->suspendActiveDOMObjectIfNeeded(*this);
</ins><span class="cx"> }
</span><span class="cx">
</span><ins>+#if !ASSERT_DISABLED
+
+void ActiveDOMObject::assertSuspendIfNeededWasCalled() const
+{
+ ASSERT(m_suspendIfNeededWasCalled);
+}
+
+#endif
+
</ins><span class="cx"> bool ActiveDOMObject::hasPendingActivity() const
</span><span class="cx"> {
</span><span class="cx"> return m_pendingActivityCount;
</span></span></pre></div>
<a id="trunkSourceWebCoredomActiveDOMObjecth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ActiveDOMObject.h (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ActiveDOMObject.h 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/dom/ActiveDOMObject.h 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -37,21 +37,21 @@
</span><span class="cx"> public:
</span><span class="cx"> explicit ActiveDOMObject(ScriptExecutionContext*);
</span><span class="cx">
</span><del>- // suspendIfNeeded() should be called exactly once after object construction to synchronize
- // the suspend state with that in ScriptExecutionContext.
</del><ins>+ // The suspendIfNeeded must be called exactly once after object construction to update
+ // the suspended state to match that of the ScriptExecutionContext.
</ins><span class="cx"> void suspendIfNeeded();
</span><del>-#if !ASSERT_DISABLED
- bool suspendIfNeededCalled() const { return m_suspendIfNeededCalled; }
-#endif
</del><ins>+ void assertSuspendIfNeededWasCalled() const;
</ins><span class="cx">
</span><span class="cx"> virtual bool hasPendingActivity() const;
</span><span class="cx">
</span><del>- // 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.
</del><ins>+ // 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.
+
</ins><span class="cx"> enum ReasonForSuspension {
</span><span class="cx"> JavaScriptDebuggerPaused,
</span><span class="cx"> WillDeferLoading,
</span><span class="lines">@@ -59,16 +59,23 @@
</span><span class="cx"> PageWillBeSuspended,
</span><span class="cx"> DocumentWillBePaused
</span><span class="cx"> };
</span><ins>+
+ // 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.
</ins><span class="cx"> virtual bool canSuspend() const;
</span><span class="cx"> virtual void suspend(ReasonForSuspension);
</span><span class="cx"> virtual void resume();
</span><ins>+
+ // 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.
</ins><span class="cx"> virtual void stop();
</span><span class="cx">
</span><span class="cx"> template<class T> void setPendingActivity(T* thisObject)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(thisObject == this);
</span><span class="cx"> thisObject->ref();
</span><del>- m_pendingActivityCount++;
</del><ins>+ ++m_pendingActivityCount;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template<class T> void unsetPendingActivity(T* thisObject)
</span><span class="lines">@@ -84,10 +91,18 @@
</span><span class="cx"> private:
</span><span class="cx"> unsigned m_pendingActivityCount;
</span><span class="cx"> #if !ASSERT_DISABLED
</span><del>- bool m_suspendIfNeededCalled;
</del><ins>+ bool m_suspendIfNeededWasCalled;
</ins><span class="cx"> #endif
</span><span class="cx"> };
</span><span class="cx">
</span><ins>+#if ASSERT_DISABLED
+
+inline void ActiveDOMObject::assertSuspendIfNeededWasCalled() const
+{
+}
+
+#endif
+
</ins><span class="cx"> } // namespace WebCore
</span><span class="cx">
</span><span class="cx"> #endif // ActiveDOMObject_h
</span></span></pre></div>
<a id="trunkSourceWebCoredomContextDestructionObservercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ContextDestructionObserver.cpp (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ContextDestructionObserver.cpp 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/dom/ContextDestructionObserver.cpp 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -32,34 +32,34 @@
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><span class="cx"> ContextDestructionObserver::ContextDestructionObserver(ScriptExecutionContext* scriptExecutionContext)
</span><del>- : m_scriptExecutionContext(0)
</del><ins>+ : m_scriptExecutionContext(nullptr)
</ins><span class="cx"> {
</span><span class="cx"> observeContext(scriptExecutionContext);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ContextDestructionObserver::~ContextDestructionObserver()
</span><span class="cx"> {
</span><del>- observeContext(0);
</del><ins>+ observeContext(nullptr);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ContextDestructionObserver::observeContext(ScriptExecutionContext* scriptExecutionContext)
</span><span class="cx"> {
</span><span class="cx"> if (m_scriptExecutionContext) {
</span><span class="cx"> ASSERT(m_scriptExecutionContext->isContextThread());
</span><del>- m_scriptExecutionContext->willDestroyDestructionObserver(this);
</del><ins>+ m_scriptExecutionContext->willDestroyDestructionObserver(*this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> m_scriptExecutionContext = scriptExecutionContext;
</span><span class="cx">
</span><span class="cx"> if (m_scriptExecutionContext) {
</span><span class="cx"> ASSERT(m_scriptExecutionContext->isContextThread());
</span><del>- m_scriptExecutionContext->didCreateDestructionObserver(this);
</del><ins>+ m_scriptExecutionContext->didCreateDestructionObserver(*this);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ContextDestructionObserver::contextDestroyed()
</span><span class="cx"> {
</span><del>- m_scriptExecutionContext = 0;
</del><ins>+ m_scriptExecutionContext = nullptr;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCoredomMessagePortcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/MessagePort.cpp (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/MessagePort.cpp 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/dom/MessagePort.cpp 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -40,7 +40,7 @@
</span><span class="cx"> , m_closed(false)
</span><span class="cx"> , m_scriptExecutionContext(&scriptExecutionContext)
</span><span class="cx"> {
</span><del>- m_scriptExecutionContext->createdMessagePort(this);
</del><ins>+ m_scriptExecutionContext->createdMessagePort(*this);
</ins><span class="cx">
</span><span class="cx"> // Don't need to call processMessagePortMessagesSoon() here, because the port will not be opened until start() is invoked.
</span><span class="cx"> }
</span><span class="lines">@@ -49,7 +49,7 @@
</span><span class="cx"> {
</span><span class="cx"> close();
</span><span class="cx"> if (m_scriptExecutionContext)
</span><del>- m_scriptExecutionContext->destroyedMessagePort(this);
</del><ins>+ m_scriptExecutionContext->destroyedMessagePort(*this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void MessagePort::postMessage(PassRefPtr<SerializedScriptValue> message, MessagePort* port, ExceptionCode& ec)
</span><span class="lines">@@ -89,10 +89,10 @@
</span><span class="cx">
</span><span class="cx"> m_entangledChannel->disentangle();
</span><span class="cx">
</span><del>- // We can't receive any messages or generate any events, so remove ourselves from the list of active ports.
</del><ins>+ // We can't receive any messages or generate any events after this, so remove ourselves from the list of active ports.
</ins><span class="cx"> ASSERT(m_scriptExecutionContext);
</span><del>- m_scriptExecutionContext->destroyedMessagePort(this);
- m_scriptExecutionContext = 0;
</del><ins>+ m_scriptExecutionContext->destroyedMessagePort(*this);
+ m_scriptExecutionContext = nullptr;
</ins><span class="cx">
</span><span class="cx"> return std::move(m_entangledChannel);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoredomScriptExecutionContextcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.cpp (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.cpp 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -92,33 +92,72 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ScriptExecutionContext::ScriptExecutionContext()
</span><del>- : m_iteratingActiveDOMObjects(false)
- , m_inDestructor(false)
- , m_circularSequentialID(0)
</del><ins>+ : m_circularSequentialID(0)
</ins><span class="cx"> , m_inDispatchErrorEvent(false)
</span><span class="cx"> , m_activeDOMObjectsAreSuspended(false)
</span><span class="cx"> , m_reasonForSuspendingActiveDOMObjects(static_cast<ActiveDOMObject::ReasonForSuspension>(-1))
</span><span class="cx"> , m_activeDOMObjectsAreStopped(false)
</span><ins>+ , m_activeDOMObjectAdditionForbidden(false)
+#if !ASSERT_DISABLED
+ , m_inScriptExecutionContextDestructor(false)
+ , m_activeDOMObjectRemovalForbidden(false)
+#endif
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>-ScriptExecutionContext::~ScriptExecutionContext()
</del><ins>+// FIXME: We should make this a member function of HashSet.
+template<typename T> inline T takeAny(HashSet<T>& set)
</ins><span class="cx"> {
</span><del>- 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();
- }
</del><ins>+ ASSERT(!set.isEmpty());
+ auto iterator = set.begin();
+ T result = std::move(*iterator);
+ set.remove(iterator);
+ return result;
+}
</ins><span class="cx">
</span><del>- HashSet<MessagePort*>::iterator messagePortsEnd = m_messagePorts.end();
- for (HashSet<MessagePort*>::iterator iter = m_messagePorts.begin(); iter != messagePortsEnd; ++iter) {
- ASSERT((*iter)->scriptExecutionContext() == this);
- (*iter)->contextDestroyed();
</del><ins>+#if ASSERT_DISABLED
+
+inline void ScriptExecutionContext::checkConsistency() const
+{
+}
+
+#else
+
+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();
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+#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
+}
+
</ins><span class="cx"> void ScriptExecutionContext::processMessagePortMessagesSoon()
</span><span class="cx"> {
</span><span class="cx"> postTask(ProcessMessagesSoonTask::create());
</span><span class="lines">@@ -126,59 +165,72 @@
</span><span class="cx">
</span><span class="cx"> void ScriptExecutionContext::dispatchMessagePortEvents()
</span><span class="cx"> {
</span><ins>+ checkConsistency();
+
</ins><span class="cx"> Ref<ScriptExecutionContext> protect(*this);
</span><span class="cx">
</span><del>- // 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();
</del><ins>+ // 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();
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void ScriptExecutionContext::createdMessagePort(MessagePort* port)
</del><ins>+void ScriptExecutionContext::createdMessagePort(MessagePort& messagePort)
</ins><span class="cx"> {
</span><del>- ASSERT(port);
</del><span class="cx"> ASSERT((isDocument() && isMainThread())
</span><span class="cx"> || (isWorkerGlobalScope() && currentThread() == toWorkerGlobalScope(this)->thread().threadID()));
</span><span class="cx">
</span><del>- m_messagePorts.add(port);
</del><ins>+ m_messagePorts.add(&messagePort);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void ScriptExecutionContext::destroyedMessagePort(MessagePort* port)
</del><ins>+void ScriptExecutionContext::destroyedMessagePort(MessagePort& messagePort)
</ins><span class="cx"> {
</span><del>- ASSERT(port);
</del><span class="cx"> ASSERT((isDocument() && isMainThread())
</span><span class="cx"> || (isWorkerGlobalScope() && currentThread() == toWorkerGlobalScope(this)->thread().threadID()));
</span><span class="cx">
</span><del>- m_messagePorts.remove(port);
</del><ins>+ m_messagePorts.remove(&messagePort);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool ScriptExecutionContext::canSuspendActiveDOMObjects()
</span><span class="cx"> {
</span><del>- // 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;
</del><ins>+ 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;
</ins><span class="cx"> }
</span><span class="cx"> }
</span><del>- m_iteratingActiveDOMObjects = false;
- return true;
</del><ins>+
+ m_activeDOMObjectAdditionForbidden = false;
+#if !ASSERT_DISABLED
+ m_activeDOMObjectRemovalForbidden = false;
+#endif
+
+ return canSuspend;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ScriptExecutionContext::suspendActiveDOMObjects(ActiveDOMObject::ReasonForSuspension why)
</span><span class="cx"> {
</span><ins>+ checkConsistency();
+
</ins><span class="cx"> #if PLATFORM(IOS)
</span><span class="cx"> if (m_activeDOMObjectsAreSuspended) {
</span><span class="cx"> ASSERT(m_reasonForSuspendingActiveDOMObjects == ActiveDOMObject::DocumentWillBePaused);
</span><span class="lines">@@ -186,103 +238,126 @@
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>- // 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;
</del><ins>+ 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
+
</ins><span class="cx"> m_activeDOMObjectsAreSuspended = true;
</span><span class="cx"> m_reasonForSuspendingActiveDOMObjects = why;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ScriptExecutionContext::resumeActiveDOMObjects(ActiveDOMObject::ReasonForSuspension why)
</span><span class="cx"> {
</span><ins>+ checkConsistency();
+
</ins><span class="cx"> if (m_reasonForSuspendingActiveDOMObjects != why)
</span><span class="cx"> return;
</span><ins>+ m_activeDOMObjectsAreSuspended = false;
</ins><span class="cx">
</span><del>- 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;
</del><ins>+ 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
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ScriptExecutionContext::stopActiveDOMObjects()
</span><span class="cx"> {
</span><ins>+ checkConsistency();
+
</ins><span class="cx"> if (m_activeDOMObjectsAreStopped)
</span><span class="cx"> return;
</span><span class="cx"> m_activeDOMObjectsAreStopped = true;
</span><del>- // 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();
</del><ins>+
+ // 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();
</ins><span class="cx"> }
</span><del>- m_iteratingActiveDOMObjects = false;
</del><span class="cx">
</span><del>- // Also close MessagePorts. If they were ActiveDOMObjects (they could be) then they could be stopped instead.
- closeMessagePorts();
</del><ins>+ 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();
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void ScriptExecutionContext::suspendActiveDOMObjectIfNeeded(ActiveDOMObject* object)
</del><ins>+void ScriptExecutionContext::suspendActiveDOMObjectIfNeeded(ActiveDOMObject& activeDOMObject)
</ins><span class="cx"> {
</span><del>- ASSERT(m_activeDOMObjects.contains(object));
- // Ensure all ActiveDOMObjects are suspended also newly created ones.
</del><ins>+ ASSERT(m_activeDOMObjects.contains(&activeDOMObject));
</ins><span class="cx"> if (m_activeDOMObjectsAreSuspended)
</span><del>- object->suspend(m_reasonForSuspendingActiveDOMObjects);
</del><ins>+ activeDOMObject.suspend(m_reasonForSuspendingActiveDOMObjects);
</ins><span class="cx"> if (m_activeDOMObjectsAreStopped)
</span><del>- object->stop();
</del><ins>+ activeDOMObject.stop();
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void ScriptExecutionContext::didCreateActiveDOMObject(ActiveDOMObject* object)
</del><ins>+void ScriptExecutionContext::didCreateActiveDOMObject(ActiveDOMObject& activeDOMObject)
</ins><span class="cx"> {
</span><del>- ASSERT(object);
- ASSERT(!m_inDestructor);
- if (m_iteratingActiveDOMObjects)
- CRASH();
- m_activeDOMObjects.add(object);
</del><ins>+ // 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);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void ScriptExecutionContext::willDestroyActiveDOMObject(ActiveDOMObject* object)
</del><ins>+void ScriptExecutionContext::willDestroyActiveDOMObject(ActiveDOMObject& activeDOMObject)
</ins><span class="cx"> {
</span><del>- ASSERT(object);
- if (m_iteratingActiveDOMObjects)
- CRASH();
- m_activeDOMObjects.remove(object);
</del><ins>+ ASSERT(!m_activeDOMObjectRemovalForbidden);
+ m_activeDOMObjects.remove(&activeDOMObject);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void ScriptExecutionContext::didCreateDestructionObserver(ContextDestructionObserver* observer)
</del><ins>+void ScriptExecutionContext::didCreateDestructionObserver(ContextDestructionObserver& observer)
</ins><span class="cx"> {
</span><del>- ASSERT(observer);
- ASSERT(!m_inDestructor);
- m_destructionObservers.add(observer);
</del><ins>+ ASSERT(!m_inScriptExecutionContextDestructor);
+ m_destructionObservers.add(&observer);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void ScriptExecutionContext::willDestroyDestructionObserver(ContextDestructionObserver* observer)
</del><ins>+void ScriptExecutionContext::willDestroyDestructionObserver(ContextDestructionObserver& observer)
</ins><span class="cx"> {
</span><del>- ASSERT(observer);
- m_destructionObservers.remove(observer);
</del><ins>+ m_destructionObservers.remove(&observer);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-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();
- }
-}
-
</del><span class="cx"> bool ScriptExecutionContext::sanitizeScriptError(String& errorMessage, int& lineNumber, int& columnNumber, String& sourceURL, CachedScript* cachedScript)
</span><span class="cx"> {
</span><span class="cx"> URL targetURL = completeURL(sourceURL);
</span><span class="lines">@@ -369,10 +444,8 @@
</span><span class="cx"> void ScriptExecutionContext::adjustMinimumTimerInterval(double oldMinimumTimerInterval)
</span><span class="cx"> {
</span><span class="cx"> if (minimumTimerInterval() != oldMinimumTimerInterval) {
</span><del>- for (TimeoutMap::iterator iter = m_timeouts.begin(); iter != m_timeouts.end(); ++iter) {
- DOMTimer* timer = iter->value;
</del><ins>+ for (auto* timer : m_timeouts.values())
</ins><span class="cx"> timer->adjustMinimumTimerInterval(oldMinimumTimerInterval);
</span><del>- }
</del><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -388,10 +461,8 @@
</span><span class="cx">
</span><span class="cx"> void ScriptExecutionContext::didChangeTimerAlignmentInterval()
</span><span class="cx"> {
</span><del>- for (TimeoutMap::iterator iter = m_timeouts.begin(); iter != m_timeouts.end(); ++iter) {
- DOMTimer* timer = iter->value;
</del><ins>+ for (auto* timer : m_timeouts.values())
</ins><span class="cx"> timer->didChangeAlignmentInterval();
</span><del>- }
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> double ScriptExecutionContext::timerAlignmentInterval() const
</span><span class="lines">@@ -419,4 +490,21 @@
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+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;
+}
+
</ins><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCoredomScriptExecutionContexth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.h (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ScriptExecutionContext.h 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.h 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -31,7 +31,6 @@
</span><span class="cx"> #include "ActiveDOMObject.h"
</span><span class="cx"> #include "SecurityContext.h"
</span><span class="cx"> #include "Supplementable.h"
</span><del>-#include "URL.h"
</del><span class="cx"> #include <runtime/ConsoleTypes.h>
</span><span class="cx"> #include <wtf/HashSet.h>
</span><span class="cx">
</span><span class="lines">@@ -49,14 +48,11 @@
</span><span class="cx"> class CachedScript;
</span><span class="cx"> class DatabaseContext;
</span><span class="cx"> class DOMTimer;
</span><del>-class EventListener;
</del><span class="cx"> class EventQueue;
</span><span class="cx"> class EventTarget;
</span><span class="cx"> class MessagePort;
</span><del>-
-#if ENABLE(BLOB)
</del><span class="cx"> class PublicURLManager;
</span><del>-#endif
</del><ins>+class URL;
</ins><span class="cx">
</span><span class="cx"> class ScriptExecutionContext : public SecurityContext, public Supplementable<ScriptExecutionContext> {
</span><span class="cx"> public:
</span><span class="lines">@@ -76,11 +72,10 @@
</span><span class="cx">
</span><span class="cx"> virtual void disableEval(const String& errorMessage) = 0;
</span><span class="cx">
</span><del>- 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);
</del><ins>+ 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);
</ins><span class="cx">
</span><del>- void addConsoleMessage(MessageSource, MessageLevel, const String& message, const String& sourceURL, unsigned lineNumber, unsigned columnNumber, JSC::ExecState* = 0, unsigned long requestIdentifier = 0);
</del><ins>+ void addConsoleMessage(MessageSource, MessageLevel, const String& message, const String& sourceURL, unsigned lineNumber, unsigned columnNumber, JSC::ExecState* = nullptr, unsigned long requestIdentifier = 0);
</ins><span class="cx"> virtual void addConsoleMessage(MessageSource, MessageLevel, const String& message, unsigned long requestIdentifier = 0) = 0;
</span><span class="cx">
</span><span class="cx"> virtual SecurityOrigin* topOrigin() const = 0;
</span><span class="lines">@@ -101,24 +96,20 @@
</span><span class="cx"> bool activeDOMObjectsAreStopped() const { return m_activeDOMObjectsAreStopped; }
</span><span class="cx">
</span><span class="cx"> // Called from the constructor and destructors of ActiveDOMObject.
</span><del>- void didCreateActiveDOMObject(ActiveDOMObject*);
- void willDestroyActiveDOMObject(ActiveDOMObject*);
</del><ins>+ void didCreateActiveDOMObject(ActiveDOMObject&);
+ void willDestroyActiveDOMObject(ActiveDOMObject&);
</ins><span class="cx">
</span><span class="cx"> // Called after the construction of an ActiveDOMObject to synchronize suspend state.
</span><del>- void suspendActiveDOMObjectIfNeeded(ActiveDOMObject*);
</del><ins>+ void suspendActiveDOMObjectIfNeeded(ActiveDOMObject&);
</ins><span class="cx">
</span><del>- typedef HashSet<ActiveDOMObject*> ActiveDOMObjectsSet;
- const ActiveDOMObjectsSet& activeDOMObjects() const { return m_activeDOMObjects; }
</del><ins>+ void didCreateDestructionObserver(ContextDestructionObserver&);
+ void willDestroyDestructionObserver(ContextDestructionObserver&);
</ins><span class="cx">
</span><del>- void didCreateDestructionObserver(ContextDestructionObserver*);
- void willDestroyDestructionObserver(ContextDestructionObserver*);
-
</del><span class="cx"> // MessagePort is conceptually a kind of ActiveDOMObject, but it needs to be tracked separately for message dispatch.
</span><span class="cx"> void processMessagePortMessagesSoon();
</span><span class="cx"> void dispatchMessagePortEvents();
</span><del>- void createdMessagePort(MessagePort*);
- void destroyedMessagePort(MessagePort*);
- const HashSet<MessagePort*>& messagePorts() const { return m_messagePorts; }
</del><ins>+ void createdMessagePort(MessagePort&);
+ void destroyedMessagePort(MessagePort&);
</ins><span class="cx">
</span><span class="cx"> void ref() { refScriptExecutionContext(); }
</span><span class="cx"> void deref() { derefScriptExecutionContext(); }
</span><span class="lines">@@ -185,26 +176,25 @@
</span><span class="cx">
</span><span class="cx"> ActiveDOMObject::ReasonForSuspension reasonForSuspendingActiveDOMObjects() const { return m_reasonForSuspendingActiveDOMObjects; }
</span><span class="cx">
</span><ins>+ bool hasPendingActivity() const;
+
</ins><span class="cx"> private:
</span><del>- 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;
</del><ins>+ 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;
</ins><span class="cx"> virtual EventTarget* errorEventTarget() = 0;
</span><span class="cx"> virtual void logExceptionToConsole(const String& errorMessage, const String& sourceURL, int lineNumber, int columnNumber, PassRefPtr<Inspector::ScriptCallStack>) = 0;
</span><span class="cx"> bool dispatchErrorEvent(const String& errorMessage, int lineNumber, int columnNumber, const String& sourceURL, CachedScript*);
</span><span class="cx">
</span><del>- void closeMessagePorts();
-
</del><span class="cx"> virtual void refScriptExecutionContext() = 0;
</span><span class="cx"> virtual void derefScriptExecutionContext() = 0;
</span><span class="cx">
</span><ins>+ void checkConsistency() const;
+
</ins><span class="cx"> HashSet<MessagePort*> m_messagePorts;
</span><span class="cx"> HashSet<ContextDestructionObserver*> m_destructionObservers;
</span><del>- ActiveDOMObjectsSet m_activeDOMObjects;
- bool m_iteratingActiveDOMObjects;
- bool m_inDestructor;
</del><ins>+ HashSet<ActiveDOMObject*> m_activeDOMObjects;
</ins><span class="cx">
</span><span class="cx"> int m_circularSequentialID;
</span><del>- typedef HashMap<int, DOMTimer*> TimeoutMap;
- TimeoutMap m_timeouts;
</del><ins>+ HashMap<int, DOMTimer*> m_timeouts;
</ins><span class="cx">
</span><span class="cx"> bool m_inDispatchErrorEvent;
</span><span class="cx"> class PendingException;
</span><span class="lines">@@ -221,6 +211,13 @@
</span><span class="cx"> #if ENABLE(SQL_DATABASE)
</span><span class="cx"> RefPtr<DatabaseContext> m_databaseContext;
</span><span class="cx"> #endif
</span><ins>+
+ bool m_activeDOMObjectAdditionForbidden;
+
+#if !ASSERT_DISABLED
+ bool m_inScriptExecutionContextDestructor;
+ bool m_activeDOMObjectRemovalForbidden;
+#endif
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> #define SCRIPT_EXECUTION_CONTEXT_TYPE_CASTS(ToValueTypeName) \
</span></span></pre></div>
<a id="trunkSourceWebCoreworkersWorkerGlobalScopecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/workers/WorkerGlobalScope.cpp (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/workers/WorkerGlobalScope.cpp 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/workers/WorkerGlobalScope.cpp 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -159,23 +159,6 @@
</span><span class="cx"> return m_navigator.get();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-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;
-}
-
</del><span class="cx"> void WorkerGlobalScope::postTask(PassOwnPtr<Task> task)
</span><span class="cx"> {
</span><span class="cx"> thread().runLoop().postTask(task);
</span></span></pre></div>
<a id="trunkSourceWebCoreworkersWorkerGlobalScopeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/workers/WorkerGlobalScope.h (167578 => 167579)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/workers/WorkerGlobalScope.h 2014-04-21 04:28:20 UTC (rev 167578)
+++ trunk/Source/WebCore/workers/WorkerGlobalScope.h 2014-04-21 04:39:11 UTC (rev 167579)
</span><span class="lines">@@ -76,7 +76,7 @@
</span><span class="cx">
</span><span class="cx"> WorkerThread& thread() const { return m_thread; }
</span><span class="cx">
</span><del>- bool hasPendingActivity() const;
</del><ins>+ using ScriptExecutionContext::hasPendingActivity;
</ins><span class="cx">
</span><span class="cx"> virtual void postTask(PassOwnPtr<Task>) override; // Executes the task on context's thread asynchronously.
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>