Teach ScriptRunner how to yield and post on loading task queue

Source/core/dom/ScriptRunner.cpp

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 something that makes it more
obvious it cancels the previous post.

[alex clarke (OOO till 29th), 2015/02/19 15:11:33]

I'd prefer to do this in a follow up patch.  I'll send that out today.

 }
 
 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 checks the execution order
doesn't change.

[Sami, 2015/02/19 13:40:00]

Do we have different behavior in this scenario?

1. soon += [1]
2. order += [2]
3. executeScript yields
4. soon += [3]
5. executeScript will do: [1, 3, 2]. Previously we'd have run [1, 2, 3].

I have no idea if this is a problem or not.

[alex clarke (OOO till 29th), 2015/02/19 15:11:33]

Yes that does appear to be the case, and I think it's OK.  From what I can tell,
the spec only guarantees the order of execution for the in-order scripts.

+            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();
     }
 }