Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(885)

Unified Diff: Source/core/dom/ScriptRunner.cpp

Issue 866273005: Teach ScriptRunner how to yield and post on loading task queue (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Needed to shutdown the sheduler owned by MockPlatform in TearDown() Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/dom/ScriptRunner.cpp
diff --git a/Source/core/dom/ScriptRunner.cpp b/Source/core/dom/ScriptRunner.cpp
index 88ed1e1c360659213e8affaf7f2ffebd37670c3f..09f17c37c4a69e58c29d137d32fe23856a96d915 100644
--- a/Source/core/dom/ScriptRunner.cpp
+++ b/Source/core/dom/ScriptRunner.cpp
@@ -30,12 +30,14 @@
#include "core/dom/Element.h"
#include "core/dom/ScriptLoader.h"
#include "platform/heap/Handle.h"
+#include "platform/scheduler/Scheduler.h"
+#include "wtf/Functional.h"
namespace blink {
ScriptRunner::ScriptRunner(Document* document)
: m_document(document)
- , m_timer(this, &ScriptRunner::timerFired)
+ , m_executeScriptsTaskFactory(WTF::bind(&ScriptRunner::executeScripts, this))
{
ASSERT(document);
}
@@ -77,13 +79,13 @@ void ScriptRunner::queueScriptForExecution(ScriptLoader* scriptLoader, Execution
void ScriptRunner::suspend()
{
- m_timer.stop();
+ m_executeScriptsTaskFactory.cancel();
}
void ScriptRunner::resume()
{
if (hasPendingScripts())
- m_timer.startOneShot(0, FROM_HERE);
+ Scheduler::shared()->postLoadingTask(FROM_HERE, m_executeScriptsTaskFactory.task());
}
void ScriptRunner::notifyScriptReady(ScriptLoader* scriptLoader, ExecutionType executionType)
@@ -99,7 +101,7 @@ void ScriptRunner::notifyScriptReady(ScriptLoader* scriptLoader, ExecutionType e
ASSERT(!m_scriptsToExecuteInOrder.isEmpty());
break;
}
- m_timer.startOneShot(0, FROM_HERE);
+ Scheduler::shared()->postLoadingTask(FROM_HERE, m_executeScriptsTaskFactory.task());
Sami 2015/02/19 12:52:13 Like we talked about, let's rename |task| to somet
alex clarke (OOO till 29th) 2015/02/19 15:11:33 I'd prefer to do this in a follow up patch. I'll
}
void ScriptRunner::notifyScriptLoadError(ScriptLoader* scriptLoader, ExecutionType executionType)
@@ -153,24 +155,33 @@ void ScriptRunner::movePendingAsyncScript(ScriptRunner* newRunner, ScriptLoader*
}
}
-void ScriptRunner::timerFired(Timer<ScriptRunner>* timer)
+void ScriptRunner::executeScripts()
{
- ASSERT_UNUSED(timer, timer == &m_timer);
-
RefPtrWillBeRawPtr<Document> protect(m_document.get());
- WillBeHeapVector<RawPtrWillBeMember<ScriptLoader> > scriptLoaders;
- scriptLoaders.swap(m_scriptsToExecuteSoon);
-
- size_t numInOrderScriptsToExecute = 0;
- for (; numInOrderScriptsToExecute < m_scriptsToExecuteInOrder.size() && m_scriptsToExecuteInOrder[numInOrderScriptsToExecute]->isReady(); ++numInOrderScriptsToExecute)
- scriptLoaders.append(m_scriptsToExecuteInOrder[numInOrderScriptsToExecute]);
- if (numInOrderScriptsToExecute)
- m_scriptsToExecuteInOrder.remove(0, numInOrderScriptsToExecute);
+ // New scripts are always appened to m_scriptsToExecuteSoon and m_scriptsToExecuteInOrder (never prepended)
Sami 2015/02/19 12:52:14 typo: appended
alex clarke (OOO till 29th) 2015/02/19 15:11:33 Done.
+ // so as long as we keep track of the current totals, we can ensure the order of execution if new scripts
+ // are added while executing the current ones.
+ size_t numScriptsToExecuteSoon = m_scriptsToExecuteSoon.size();
+ size_t numScriptsToExecuteInOrder = m_scriptsToExecuteInOrder.size();
+ for (size_t i = 0; i < numScriptsToExecuteSoon; i++) {
+ if (Scheduler::shared()->shouldYieldForHighPriorityWork()) {
Sami 2015/02/19 12:52:13 Let's add a test that yields at every point and ch
Sami 2015/02/19 13:40:00 Do we have different behavior in this scenario? 1
alex clarke (OOO till 29th) 2015/02/19 15:11:33 Yes that does appear to be the case, and I think i
+ Scheduler::shared()->postLoadingTask(FROM_HERE, m_executeScriptsTaskFactory.task());
+ return;
+ }
+ m_scriptsToExecuteSoon.takeFirst()->execute();
+ m_document->decrementLoadEventDelayCount();
+ }
- size_t size = scriptLoaders.size();
- for (size_t i = 0; i < size; ++i) {
- scriptLoaders[i]->execute();
+ for (size_t i = 0; i < numScriptsToExecuteInOrder; i++) {
+ if (!m_scriptsToExecuteInOrder.first()->isReady()) {
+ break;
+ }
+ if (Scheduler::shared()->shouldYieldForHighPriorityWork()) {
+ Scheduler::shared()->postLoadingTask(FROM_HERE, m_executeScriptsTaskFactory.task());
+ return;
+ }
+ m_scriptsToExecuteInOrder.takeFirst()->execute();
m_document->decrementLoadEventDelayCount();
}
}

Powered by Google App Engine
This is Rietveld 408576698