Diff
Modified: trunk/LayoutTests/ChangeLog (205652 => 205653)
--- trunk/LayoutTests/ChangeLog 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/LayoutTests/ChangeLog 2016-09-08 18:48:15 UTC (rev 205653)
@@ -1,3 +1,17 @@
+2016-09-08 Commit Queue <[email protected]>
+
+ Unreviewed, rolling out r205652.
+ https://bugs.webkit.org/show_bug.cgi?id=161748
+
+ Crashing under ASan (Requested by yusukesuzuki on #webkit).
+
+ Reverted changeset:
+
+ "ScriptRunner should be driven by PendingScript rather than
+ ScriptElement"
+ https://bugs.webkit.org/show_bug.cgi?id=161726
+ http://trac.webkit.org/changeset/205652
+
2016-09-08 Yusuke Suzuki <[email protected]>
ScriptRunner should be driven by PendingScript rather than ScriptElement
Deleted: trunk/LayoutTests/js/dom/document-write-in-error-event-expected.txt (205652 => 205653)
--- trunk/LayoutTests/js/dom/document-write-in-error-event-expected.txt 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/LayoutTests/js/dom/document-write-in-error-event-expected.txt 2016-09-08 18:48:15 UTC (rev 205653)
@@ -1,4 +0,0 @@
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
Deleted: trunk/LayoutTests/js/dom/document-write-in-error-event.html (205652 => 205653)
--- trunk/LayoutTests/js/dom/document-write-in-error-event.html 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/LayoutTests/js/dom/document-write-in-error-event.html 2016-09-08 18:48:15 UTC (rev 205653)
@@ -1,20 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-<script src=""
-</head>
-<body _onload_="loaded()">
-<script>
- window.jsTestIsAsync = true;
- function loaded() {
- let script = document.createElement("script");
- script.src = ""
- script.addEventListener('error', function () {
- document.write("<script>finishJSTest()<\/script>");
- }, false);
- document.body.appendChild(script);
- }
-</script>
-<script src=""
-</body>
-</html>
Deleted: trunk/LayoutTests/js/dom/document-write-in-load-event-expected.txt (205652 => 205653)
--- trunk/LayoutTests/js/dom/document-write-in-load-event-expected.txt 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/LayoutTests/js/dom/document-write-in-load-event-expected.txt 2016-09-08 18:48:15 UTC (rev 205653)
@@ -1,4 +0,0 @@
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
Deleted: trunk/LayoutTests/js/dom/document-write-in-load-event.html (205652 => 205653)
--- trunk/LayoutTests/js/dom/document-write-in-load-event.html 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/LayoutTests/js/dom/document-write-in-load-event.html 2016-09-08 18:48:15 UTC (rev 205653)
@@ -1,20 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-<script src=""
-</head>
-<body _onload_="loaded()">
-<script>
- window.jsTestIsAsync = true;
- function loaded() {
- let script = document.createElement("script");
- script.src = ""
- script.addEventListener('load', function () {
- document.write("<script>finishJSTest()<\/script>");
- }, false);
- document.body.appendChild(script);
- }
-</script>
-<script src=""
-</body>
-</html>
Deleted: trunk/LayoutTests/js/dom/script-tests/dummy.js (205652 => 205653)
--- trunk/LayoutTests/js/dom/script-tests/dummy.js 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/LayoutTests/js/dom/script-tests/dummy.js 2016-09-08 18:48:15 UTC (rev 205653)
@@ -1 +0,0 @@
-// Do nothing.
Modified: trunk/Source/WebCore/ChangeLog (205652 => 205653)
--- trunk/Source/WebCore/ChangeLog 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/Source/WebCore/ChangeLog 2016-09-08 18:48:15 UTC (rev 205653)
@@ -1,3 +1,17 @@
+2016-09-08 Commit Queue <[email protected]>
+
+ Unreviewed, rolling out r205652.
+ https://bugs.webkit.org/show_bug.cgi?id=161748
+
+ Crashing under ASan (Requested by yusukesuzuki on #webkit).
+
+ Reverted changeset:
+
+ "ScriptRunner should be driven by PendingScript rather than
+ ScriptElement"
+ https://bugs.webkit.org/show_bug.cgi?id=161726
+ http://trac.webkit.org/changeset/205652
+
2016-09-08 Yusuke Suzuki <[email protected]>
ScriptRunner should be driven by PendingScript rather than ScriptElement
Modified: trunk/Source/WebCore/dom/ScriptElement.cpp (205652 => 205653)
--- trunk/Source/WebCore/dom/ScriptElement.cpp 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/Source/WebCore/dom/ScriptElement.cpp 2016-09-08 18:48:15 UTC (rev 205653)
@@ -75,6 +75,11 @@
m_startLineNumber = m_element.document().scriptableDocumentParser()->textPosition().m_line;
}
+ScriptElement::~ScriptElement()
+{
+ stopLoadRequest();
+}
+
bool ScriptElement::shouldCallFinishedInsertingSubtree(ContainerNode& insertionPoint)
{
return insertionPoint.inDocument() && !m_parserInserted;
@@ -240,9 +245,11 @@
ASSERT(m_loadableScript);
m_willExecuteInOrder = true;
document.scriptRunner()->queueScriptForExecution(this, *m_loadableScript, ScriptRunner::IN_ORDER_EXECUTION);
+ m_loadableScript->addClient(*this);
} else if (hasSourceAttribute()) {
ASSERT(m_loadableScript);
m_element.document().scriptRunner()->queueScriptForExecution(this, *m_loadableScript, ScriptRunner::ASYNC_EXECUTION);
+ m_loadableScript->addClient(*this);
} else {
// Reset line numbering for nested writes.
TextPosition position = document.isInDocumentWrite() ? TextPosition() : scriptStartPosition;
@@ -329,6 +336,15 @@
}
}
+void ScriptElement::stopLoadRequest()
+{
+ if (m_loadableScript) {
+ if (!m_willBeParserExecuted)
+ m_loadableScript->removeClient(*this);
+ m_loadableScript = nullptr;
+ }
+}
+
void ScriptElement::executeScriptAndDispatchEvent(LoadableScript& loadableScript)
{
if (Optional<LoadableScript::Error> error = loadableScript.wasErrored()) {
@@ -342,8 +358,16 @@
}
}
-void ScriptElement::executeScriptForScriptRunner(PendingScript& pendingScript)
+void ScriptElement::executeScriptForScriptRunner(LoadableScript& loadableScript)
{
+ ASSERT(!m_willBeParserExecuted);
+ executeScriptAndDispatchEvent(loadableScript);
+ loadableScript.removeClient(*this);
+}
+
+void ScriptElement::executeScriptForHTMLScriptRunner(PendingScript& pendingScript)
+{
+ IgnoreDestructiveWriteCountIncrementer ignoreDestructiveWriteCountIncrementer(&m_element.document());
if (auto* loadableScript = pendingScript.loadableScript())
executeScriptAndDispatchEvent(*loadableScript);
else {
@@ -353,6 +377,25 @@
}
}
+void ScriptElement::notifyFinished(LoadableScript&)
+{
+ ASSERT(!m_willBeParserExecuted);
+
+ // LoadableScript possibly invokes this notifyFinished() more than
+ // once because ScriptElement doesn't unsubscribe itself from
+ // LoadableScript here and does it in execute() instead.
+ // We use m_loadableScript to check if this function is already called.
+ if (!m_loadableScript)
+ return;
+
+ if (m_willExecuteInOrder)
+ m_element.document().scriptRunner()->notifyScriptReady(this, ScriptRunner::IN_ORDER_EXECUTION);
+ else
+ m_element.document().scriptRunner()->notifyScriptReady(this, ScriptRunner::ASYNC_EXECUTION);
+
+ m_loadableScript = nullptr;
+}
+
bool ScriptElement::ignoresLoadRequest() const
{
return m_alreadyStarted || m_isExternalScript || m_parserInserted || !m_element.inDocument();
Modified: trunk/Source/WebCore/dom/ScriptElement.h (205652 => 205653)
--- trunk/Source/WebCore/dom/ScriptElement.h 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/Source/WebCore/dom/ScriptElement.h 2016-09-08 18:48:15 UTC (rev 205653)
@@ -38,9 +38,9 @@
class ScriptElement;
class ScriptSourceCode;
-class ScriptElement {
+class ScriptElement : private LoadableScriptClient {
public:
- virtual ~ScriptElement() { }
+ virtual ~ScriptElement();
Element& element() { return m_element; }
const Element& element() const { return m_element; }
@@ -52,7 +52,8 @@
WEBCORE_EXPORT String scriptContent() const;
void executeScript(const ScriptSourceCode&);
- void executeScriptForScriptRunner(PendingScript&);
+ void executeScriptForScriptRunner(LoadableScript&);
+ void executeScriptForHTMLScriptRunner(PendingScript&);
// XML parser calls these
virtual void dispatchLoadEvent() = 0;
@@ -62,7 +63,6 @@
bool willBeParserExecuted() const { return m_willBeParserExecuted; }
bool readyToBeParserExecuted() const { return m_readyToBeParserExecuted; }
bool willExecuteWhenDocumentFinishedParsing() const { return m_willExecuteWhenDocumentFinishedParsing; }
- bool willExecuteInOrder() const { return m_willExecuteInOrder; }
LoadableScript* loadableScript() { return m_loadableScript.get(); }
protected:
@@ -92,7 +92,10 @@
CachedResourceHandle<CachedScript> requestScriptWithCache(const URL&, const String& nonceAttribute, const String& crossoriginAttribute);
bool requestClassicScript(const String& sourceURL);
+ void stopLoadRequest();
+ void notifyFinished(LoadableScript&) override;
+
virtual String sourceAttributeValue() const = 0;
virtual String charsetAttributeValue() const = 0;
virtual String typeAttributeValue() const = 0;
Modified: trunk/Source/WebCore/dom/ScriptRunner.cpp (205652 => 205653)
--- trunk/Source/WebCore/dom/ScriptRunner.cpp 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/Source/WebCore/dom/ScriptRunner.cpp 2016-09-08 18:48:15 UTC (rev 205653)
@@ -40,20 +40,12 @@
ScriptRunner::~ScriptRunner()
{
- for (auto& pendingScript : m_scriptsToExecuteSoon) {
- UNUSED_PARAM(pendingScript);
+ for (size_t i = 0; i < m_scriptsToExecuteSoon.size(); ++i)
m_document.decrementLoadEventDelayCount();
- }
- for (auto& pendingScript : m_scriptsToExecuteInOrder) {
- if (pendingScript->watchingForLoad())
- pendingScript->clearClient();
+ for (size_t i = 0; i < m_scriptsToExecuteInOrder.size(); ++i)
m_document.decrementLoadEventDelayCount();
- }
- for (auto& pendingScript : m_pendingAsyncScripts) {
- if (pendingScript->watchingForLoad())
- const_cast<PendingScript&>(pendingScript.get()).clearClient();
+ for (unsigned i = 0; i < m_pendingAsyncScripts.size(); ++i)
m_document.decrementLoadEventDelayCount();
- }
}
void ScriptRunner::queueScriptForExecution(ScriptElement* scriptElement, LoadableScript& loadableScript, ExecutionType executionType)
@@ -65,17 +57,15 @@
m_document.incrementLoadEventDelayCount();
- Ref<PendingScript> pendingScript = PendingScript::create(element, loadableScript);
switch (executionType) {
case ASYNC_EXECUTION:
- m_pendingAsyncScripts.add(pendingScript.copyRef());
+ m_pendingAsyncScripts.add(scriptElement, PendingScript::create(element, loadableScript));
break;
case IN_ORDER_EXECUTION:
- m_scriptsToExecuteInOrder.append(pendingScript.copyRef());
+ m_scriptsToExecuteInOrder.append(PendingScript::create(element, loadableScript));
break;
}
- pendingScript->setClient(this);
}
void ScriptRunner::suspend()
@@ -89,17 +79,18 @@
m_timer.startOneShot(0);
}
-void ScriptRunner::notifyFinished(PendingScript& pendingScript)
+void ScriptRunner::notifyScriptReady(ScriptElement* scriptElement, ExecutionType executionType)
{
- auto* scriptElement = toScriptElementIfPossible(&pendingScript.element());
- ASSERT(scriptElement);
- if (scriptElement->willExecuteInOrder())
+ switch (executionType) {
+ case ASYNC_EXECUTION:
+ ASSERT(m_pendingAsyncScripts.contains(scriptElement));
+ m_scriptsToExecuteSoon.append(m_pendingAsyncScripts.take(scriptElement)->ptr());
+ break;
+
+ case IN_ORDER_EXECUTION:
ASSERT(!m_scriptsToExecuteInOrder.isEmpty());
- else {
- ASSERT(m_pendingAsyncScripts.contains(pendingScript));
- m_scriptsToExecuteSoon.append(m_pendingAsyncScripts.take(pendingScript)->ptr());
+ break;
}
- pendingScript.clearClient();
m_timer.startOneShot(0);
}
@@ -125,7 +116,7 @@
auto* scriptElement = toScriptElementIfPossible(&script->element());
ASSERT(scriptElement);
ASSERT(script->needsLoading());
- scriptElement->executeScriptForScriptRunner(*script);
+ scriptElement->executeScriptForScriptRunner(*script->loadableScript());
m_document.decrementLoadEventDelayCount();
}
}
Modified: trunk/Source/WebCore/dom/ScriptRunner.h (205652 => 205653)
--- trunk/Source/WebCore/dom/ScriptRunner.h 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/Source/WebCore/dom/ScriptRunner.h 2016-09-08 18:48:15 UTC (rev 205653)
@@ -25,9 +25,8 @@
#pragma once
-#include "PendingScriptClient.h"
#include "Timer.h"
-#include <wtf/HashSet.h>
+#include <wtf/HashMap.h>
#include <wtf/Noncopyable.h>
#include <wtf/Vector.h>
@@ -38,7 +37,7 @@
class ScriptElement;
class LoadableScript;
-class ScriptRunner : private PendingScriptClient {
+class ScriptRunner {
WTF_MAKE_NONCOPYABLE(ScriptRunner); WTF_MAKE_FAST_ALLOCATED;
public:
explicit ScriptRunner(Document&);
@@ -54,12 +53,10 @@
private:
void timerFired();
- void notifyFinished(PendingScript&) override;
-
Document& m_document;
Vector<Ref<PendingScript>> m_scriptsToExecuteInOrder;
Vector<RefPtr<PendingScript>> m_scriptsToExecuteSoon; // http://www.whatwg.org/specs/web-apps/current-work/#set-of-scripts-that-will-execute-as-soon-as-possible
- HashSet<Ref<PendingScript>> m_pendingAsyncScripts;
+ HashMap<ScriptElement*, Ref<PendingScript>> m_pendingAsyncScripts;
Timer m_timer;
};
Modified: trunk/Source/WebCore/html/parser/HTMLScriptRunner.cpp (205652 => 205653)
--- trunk/Source/WebCore/html/parser/HTMLScriptRunner.cpp 2016-09-08 17:49:43 UTC (rev 205652)
+++ trunk/Source/WebCore/html/parser/HTMLScriptRunner.cpp 2016-09-08 18:48:15 UTC (rev 205653)
@@ -121,7 +121,7 @@
if (auto* scriptElement = toScriptElementIfPossible(&pendingScript->element())) {
NestingLevelIncrementer nestingLevelIncrementer(m_scriptNestingLevel);
- scriptElement->executeScriptForScriptRunner(*pendingScript);
+ scriptElement->executeScriptForHTMLScriptRunner(*pendingScript);
}
ASSERT(!isExecutingScript());
}
