Teach ScriptRunner how to yield and post on loading task queue

Source/core/dom/ScriptRunner.cpp

 /*
  * Copyright (C) 2010 Google, Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 12 matching lines @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "core/dom/ScriptRunner.h"
 
 #include "core/dom/Document.h"
 #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);
 }
 
 ScriptRunner::~ScriptRunner()
 {
 #if !ENABLE(OILPAN)
     // Make sure that ScriptLoaders don't keep their PendingScripts alive.
     for (ScriptLoader* scriptLoader : m_scriptsToExecuteInOrder)
         scriptLoader->detach();
@@ skipping 21 matching lines @@
 
     case IN_ORDER_EXECUTION:
         m_document->incrementLoadEventDelayCount();
         m_scriptsToExecuteInOrder.append(scriptLoader);
         break;
     }
 }
 
 void ScriptRunner::suspend()
 {
-    m_timer.stop();
+    m_executeScriptsTaskFactory.cancel();
 }
 
 void ScriptRunner::resume()
 {
     if (hasPendingScripts())
-        m_timer.startOneShot(0, FROM_HERE);
+        postTaskIfOneIsNotAlreadyInFlight();
 }
 
 void ScriptRunner::notifyScriptReady(ScriptLoader* scriptLoader, ExecutionType executionType)
 {
     switch (executionType) {
     case ASYNC_EXECUTION:
         ASSERT(m_pendingAsyncScripts.contains(scriptLoader));
         m_scriptsToExecuteSoon.append(scriptLoader);
         m_pendingAsyncScripts.remove(scriptLoader);
         break;
 
     case IN_ORDER_EXECUTION:
         ASSERT(!m_scriptsToExecuteInOrder.isEmpty());
         break;
     }
-    m_timer.startOneShot(0, FROM_HERE);
+    postTaskIfOneIsNotAlreadyInFlight();
 }
 
 void ScriptRunner::notifyScriptLoadError(ScriptLoader* scriptLoader, ExecutionType executionType)
 {
     switch (executionType) {
     case ASYNC_EXECUTION:
         ASSERT(m_pendingAsyncScripts.contains(scriptLoader));
         m_pendingAsyncScripts.remove(scriptLoader);
         scriptLoader->detach();
         m_document->decrementLoadEventDelayCount();
@@ skipping 33 matching lines @@
 
 void ScriptRunner::movePendingAsyncScript(ScriptRunner* newRunner, ScriptLoader* scriptLoader)
 {
     if (m_pendingAsyncScripts.contains(scriptLoader)) {
         newRunner->addPendingAsyncScript(scriptLoader);
         m_pendingAsyncScripts.remove(scriptLoader);
         m_document->decrementLoadEventDelayCount();
     }
 }
 
-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);
-
-    size_t size = scriptLoaders.size();
-    for (size_t i = 0; i < size; ++i) {
-        scriptLoaders[i]->execute();
+    // New scripts are always appended to m_scriptsToExecuteSoon and m_scriptsToExecuteInOrder (never prepended)
+    // 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.
+    // NOTE a yield followed by a notifyScriptReady(... ASYNC_EXECUTION) will result in that script executing
+    // before any pre-existing ScriptsToExecuteInOrder.
+    size_t numScriptsToExecuteSoon = m_scriptsToExecuteSoon.size();
+    size_t numScriptsToExecuteInOrder = m_scriptsToExecuteInOrder.size();
+    for (size_t i = 0; i < numScriptsToExecuteSoon; i++) {
+        if (shouldYieldForHighPriorityWork())

[marja, 2015/02/20 12:06:52]

Another behavioral consideration: when this executeScripts() is called, should
we expect it to execute at least 1 script before yielding? Why / why not? (I can
think of reasons both ways...)

[alex clarke (OOO till 29th), 2015/02/20 13:40:17]

The Scheduler shouldn't run a task from the LoadingTaskQueue when
shouldYieldForHighPriorityWork is true. So we might as well move the yield to
after the execute.  It's clearer that something will be done.

+            return;
+        ASSERT(!m_scriptsToExecuteSoon.isEmpty());
+        m_scriptsToExecuteSoon.takeFirst()->execute();
         m_document->decrementLoadEventDelayCount();
     }
+
+    for (size_t i = 0; i < numScriptsToExecuteInOrder; i++) {
+        ASSERT(!m_scriptsToExecuteInOrder.isEmpty());
+        if (!m_scriptsToExecuteInOrder.first()->isReady() || shouldYieldForHighPriorityWork())
+            return;
+        m_scriptsToExecuteInOrder.takeFirst()->execute();
+        m_document->decrementLoadEventDelayCount();
+    }
+}
+
+bool ScriptRunner::shouldYieldForHighPriorityWork()

[marja, 2015/02/20 12:06:52]

Nit: this function name is confusing, I'd just expect a function called
shouldFoo() just return boolean and *not* do any other work such as scheduling a
task.

Idk what would be a better name, yieldForHighPriorityWork maybe? (Though, that
function cannot just yield on behalf of the caller, so it's not perfect either.)

[alex clarke (OOO till 29th), 2015/02/20 13:40:17]

Lets go with yieldForHighPriorityWork.

+{
+    if (!Scheduler::shared()->shouldYieldForHighPriorityWork())
+        return false;
+
+    postTaskIfOneIsNotAlreadyInFlight();
+    return true;
+}
+
+void ScriptRunner::postTaskIfOneIsNotAlreadyInFlight()
+{
+    if (m_executeScriptsTaskFactory.isPending())
+        return;
+
+    // FIXME: Rename task() so that it's obvious it cancels any pending task.
+    Scheduler::shared()->postLoadingTask(FROM_HERE, m_executeScriptsTaskFactory.task());
 }
 
 void ScriptRunner::trace(Visitor* visitor)
 {
 #if ENABLE(OILPAN)
     visitor->trace(m_document);
     visitor->trace(m_scriptsToExecuteInOrder);
     visitor->trace(m_scriptsToExecuteSoon);
     visitor->trace(m_pendingAsyncScripts);
 #endif
 }
 
 }