Stream scripts to V8 as they load - Blink side.

Source/core/html/parser/HTMLScriptRunner.cpp

Index: Source/core/html/parser/HTMLScriptRunner.cpp
diff --git a/Source/core/html/parser/HTMLScriptRunner.cpp b/Source/core/html/parser/HTMLScriptRunner.cpp
index bc4742749b294cc9a3a902c80e1b23e3fd7e815e..1e5a1b980a5f3e1fa9f8b6d0b7ca848c40c7c8a4 100644
--- a/Source/core/html/parser/HTMLScriptRunner.cpp
+++ b/Source/core/html/parser/HTMLScriptRunner.cpp
@@ -73,6 +73,9 @@ void HTMLScriptRunner::detach()
 
     if (m_parserBlockingScript.resource() && m_parserBlockingScript.watchingForLoad())
         stopWatchingForLoad(m_parserBlockingScript);
+    // If the script is streamed, watchingForLoad is false. In that case, we
+    // don't need to do anything (PendingScript will take care of cancelling the
+    // streaming).

[haraken, 2014/08/17 16:05:28]

It's a bit weird that the lifetime of not-streamed loading is taken care of by
the HTMLScriptRunner whereas the lifetime of streamed loading is taken care of
by PendingScripts. Can we refactor the code so that all loadings are taken care
of by PendingScripts?

(I guess you'll anyway need to make the change when you support
not-parser-blocking scripts.)

[haraken, 2014/08/17 16:05:28]

Move this comment to above line 74.

[marja, 2014/08/20 11:45:57]

I don't think it's accurate to say "lifetime of not-streamed loading is taken
care of by the HTMLScriptRunner whereas the lifetime of streamed loading is
taken care of by PendingScripts".

However, HTMLScriptRunner is *watching* non-streamed loads of parsing blocking
scripts, but not watching streamed loads, and not watching the load of
non-parsing-blocking scripts. (Thus functions like
HTMLScriptRunner::watchForLoad and stopWatchingForLoad.) So it's not about
managing the loading itself, but about watching it.

I can unify this though: ScriptStreamer can imitate a Resource so that the
HTMLScriptRunner doesn't know whether it's watching a Resource load or a
ScriptStreamer load. It'll just delegate the "watch" and "stopWatching" to
PendingScript, which will know what to do. I implemented that version and it
turned out pretty nice. Wdyt? I can also do this as a separate CL before the
actual streaming.

[haraken, 2014/08/20 15:00:19]

The new version looks nicer to me!

 
     while (!m_scriptsToExecuteAfterParsing.isEmpty()) {
         PendingScript pendingScript = m_scriptsToExecuteAfterParsing.takeFirst();
@@ -102,13 +105,17 @@ inline PassRefPtrWillBeRawPtr<Event> createScriptLoadEvent()
     return Event::create(EventTypeNames::load);
 }
 
-ScriptSourceCode HTMLScriptRunner::sourceFromPendingScript(const PendingScript& script, bool& errorOccurred) const
+ScriptSourceCode HTMLScriptRunner::sourceFromPendingScript(PendingScript& script, bool& errorOccurred) const
 {
     if (script.resource()) {
         errorOccurred = script.resource()->errorOccurred();
         ASSERT(script.resource()->isLoaded());
+        if (script.streamer()) {
+            return ScriptSourceCode(script.releaseStreamer(), script.resource());
+        }
         return ScriptSourceCode(script.resource());
     }
+    ASSERT(!script.isStreaming());

[haraken, 2014/08/17 16:05:28]

Help me understand: Why is it guaranteed that script.isStreaming() is false at
this point?

[marja, 2014/08/20 11:45:57]

Because this function is only called when we want to compile the script, so,
streaming must have finished before that. (This can only be called after
HTMLScriptRunner has gotten the "finished" notification.)

(We cannot extract the source code from a resource which is still loading.)

     errorOccurred = false;
     return ScriptSourceCode(script.element()->textContent(), documentURLForScriptExecution(m_document), script.startingPosition());
 }
@@ -120,6 +127,8 @@ bool HTMLScriptRunner::isPendingScriptReady(const PendingScript& script)
         return false;
     if (script.resource() && !script.resource()->isLoaded())
         return false;
+    if (script.isStreaming())
+        return false;
     return true;
 }
 
@@ -189,6 +198,11 @@ void HTMLScriptRunner::stopWatchingForLoad(PendingScript& pendingScript)
 
 void HTMLScriptRunner::notifyFinished(Resource* cachedResource)
 {
+    // This function should not be called for the parser blocking script, if
+    // we've finished loading but we're still streaming it. It will be called
+    // when the streaming has completed too.

[haraken, 2014/08/17 16:05:28]

This comment is a bit misleading. Slightly better:

// For a parser-blocking script, this function should be called only when the
streaming is complete.

[marja, 2014/08/20 11:45:57]

Done.

+    ASSERT(!(m_parserBlockingScript.resource() == cachedResource
+        && m_parserBlockingScript.isStreaming()));
     m_host->notifyScriptLoaded(cachedResource);
 }
 
@@ -275,8 +289,11 @@ void HTMLScriptRunner::requestParsingBlockingScript(Element* element)
     // We only care about a load callback if resource is not already
     // in the cache. Callers will attempt to run the m_parserBlockingScript
     // if possible before returning control to the parser.
-    if (!m_parserBlockingScript.resource()->isLoaded())
-        watchForLoad(m_parserBlockingScript);
+    if (!m_parserBlockingScript.resource()->isLoaded()) {
+        if (!V8ScriptStreamer::startStreaming(&m_parserBlockingScript, this)) {
+            watchForLoad(m_parserBlockingScript);
+        }
+    }
 }
 
 void HTMLScriptRunner::requestDeferredScript(Element* element)
@@ -334,6 +351,8 @@ void HTMLScriptRunner::runScript(Element* script, const TextPosition& scriptStar
             return;
 
         if (scriptLoader->willExecuteWhenDocumentFinishedParsing()) {
+            // TODO(marja): in addition to parser blocking scripts, stream also

[haraken, 2014/08/17 16:05:28]

Nit: TODO(marja) => FIXME

[marja, 2014/08/20 11:45:57]

Done.

+            // scripts which are not parser blocking.

[haraken, 2014/08/17 16:05:28]

I wonder if there is any particular reason you cannot enable the streaming for
not-parser-blocking scripts. I'm fine with leaving this as a FIXME for a
separate CL, but I'm curious if there is any particular reason you cannot do it
easily.

[marja, 2014/08/20 11:45:57]

(See the other comment.)

             requestDeferredScript(script);
         } else if (scriptLoader->readyToBeParserExecuted()) {
             if (m_scriptNestingLevel == 1) {