Reland: Script streaming: Add an option to make the main thread block (wait for parsing)

This makes the main thread block and wait for the parser thread when all the
script data has arrived. The goal is to ensure that the script (which can now be
compiled) will get the main thread's attention as soon as possible.

This feature is temporary: The usefulness of blocking will be evaluated with
Finch, and this option will be removed (blocking will be always enabled or never
enabled, based on which option turns out to be better).

Previous version: https://codereview.chromium.org/651163002/

R=haraken@chromium.org
BUG=

Review URL: https://codereview.chromium.org/674133002

git-svn-id: svn://svn.chromium.org/blink/trunk@184361 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp

@@ -13,6 +13,7 @@
 #include "core/fetch/ScriptResource.h"
 #include "core/frame/Settings.h"
 #include "platform/SharedBuffer.h"
+#include "platform/TraceEvent.h"
 #include "public/platform/Platform.h"
 #include "wtf/MainThread.h"
 #include "wtf/text/TextEncodingRegistry.h"
@@ -224,28 +225,23 @@ void ScriptStreamer::startStreaming(PendingScript& script, Settings* settings, S
         blink::Platform::current()->histogramEnumeration(startedStreamingHistogramName(scriptType), 0, 2);
 }
 
-void ScriptStreamer::streamingComplete()
+void ScriptStreamer::streamingCompleteOnBackgroundThread()
 {
-    ASSERT(isMainThread());
-    // It's possible that the corresponding Resource was deleted before V8
-    // finished streaming. In that case, the data or the notification is not
-    // needed. In addition, if the streaming is suppressed, the non-streaming
-    // code path will resume after the resource has loaded, before the
-    // background task finishes.
-    if (m_detached || m_streamingSuppressed) {
-        deref();
-        return;
-    }
-
-    // We have now streamed the whole script to V8 and it has parsed the
-    // script. We're ready for the next step: compiling and executing the
-    // script.
+    ASSERT(!isMainThread());
+    MutexLocker locker(m_mutex);
     m_parsingFinished = true;
-
-    notifyFinishedToClient();
-
-    // The background thread no longer holds an implicit reference.
-    deref();
+    // In the blocking case, the main thread is normally waiting at this
+    // point, but it can also happen that the load is not yet finished
+    // (e.g., a parse error). In that case, notifyFinished will be called
+    // eventually and it will not wait on m_parsingFinishedCondition.
+
+    // In the non-blocking case, notifyFinished might already be called, or it
+    // might be called in the future. In any case, do the cleanup here.
+    if (m_mainThreadWaitingForParserThread) {
+        m_parsingFinishedCondition.signal();
+    } else {
+        callOnMainThread(WTF::bind(&ScriptStreamer::streamingComplete, this));
+    }
 }
 
 void ScriptStreamer::cancel()
@@ -262,8 +258,10 @@ void ScriptStreamer::cancel()
 
 void ScriptStreamer::suppressStreaming()
 {
-    ASSERT(!m_parsingFinished);
+    MutexLocker locker(m_mutex);
     ASSERT(!m_loadingFinished);
+    // It can be that the parsing task has already finished (e.g., if there was
+    // a parse error).
     m_streamingSuppressed = true;
 }
 
@@ -271,8 +269,11 @@ void ScriptStreamer::notifyAppendData(ScriptResource* resource)
 {
     ASSERT(isMainThread());
     ASSERT(m_resource == resource);
-    if (m_streamingSuppressed)
-        return;
+    {
+        MutexLocker locker(m_mutex);
+        if (m_streamingSuppressed)
+            return;
+    }
     if (!m_haveEnoughDataForStreaming) {
         // Even if the first data chunk is small, the script can still be big
         // enough - wait until the next data chunk comes before deciding whether
@@ -294,7 +295,8 @@ void ScriptStreamer::notifyAppendData(ScriptResource* resource)
         ASSERT(m_task);
         // ScriptStreamer needs to stay alive as long as the background task is
         // running. This is taken care of with a manual ref() & deref() pair;
-        // the corresponding deref() is in streamingComplete.
+        // the corresponding deref() is in streamingComplete or in
+        // notifyFinished.
         ref();
         ScriptStreamingTask* task = new ScriptStreamingTask(m_task.release(), this);
         ScriptStreamerThread::shared()->postTask(task);
@@ -318,10 +320,40 @@ void ScriptStreamer::notifyFinished(Resource* resource)
     }
     m_stream->didFinishLoading();
     m_loadingFinished = true;
+
+    if (shouldBlockMainThread()) {
+        // Make the main thead wait until the streaming is complete, to make
+        // sure that the script gets the main thread's attention as early as
+        // possible (for possible compiling, if the client wants to do it
+        // right away). Note that blocking here is not any worse than the
+        // non-streaming code path where the main thread eventually blocks
+        // to parse the script.
+        TRACE_EVENT0("v8", "v8.mainThreadWaitingForParserThread");
+        MutexLocker locker(m_mutex);
+        while (!isFinished()) {
+            ASSERT(!m_parsingFinished);
+            ASSERT(!m_streamingSuppressed);
+            m_mainThreadWaitingForParserThread = true;
+            m_parsingFinishedCondition.wait(m_mutex);
+        }
+    }
+
+    // Calling notifyFinishedToClient can result into the upper layers dropping
+    // references to ScriptStreamer. Keep it alive until this function ends.
+    RefPtr<ScriptStreamer> protect(this);
+
     notifyFinishedToClient();
+
+    if (m_mainThreadWaitingForParserThread) {
+        ASSERT(m_parsingFinished);
+        ASSERT(!m_streamingSuppressed);
+        // streamingComplete won't be called, so do the ramp-down work
+        // here.
+        deref();
+    }
 }
 
-ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::StreamedSource::Encoding encoding, PendingScript::Type scriptType)
+ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::StreamedSource::Encoding encoding, PendingScript::Type scriptType, ScriptStreamingMode mode)
     : m_resource(resource)
     , m_detached(false)
     , m_stream(new SourceStream(this))
@@ -332,7 +364,34 @@ ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::Str
     , m_haveEnoughDataForStreaming(false)
     , m_streamingSuppressed(false)
     , m_scriptType(scriptType)
+    , m_scriptStreamingMode(mode)
+    , m_mainThreadWaitingForParserThread(false)
+{
+}
+
+void ScriptStreamer::streamingComplete()
 {
+    // The background task is completed; do the necessary ramp-down in the main
+    // thread.
+    ASSERT(isMainThread());
+
+    // It's possible that the corresponding Resource was deleted before V8
+    // finished streaming. In that case, the data or the notification is not
+    // needed. In addition, if the streaming is suppressed, the non-streaming
+    // code path will resume after the resource has loaded, before the
+    // background task finishes.
+    if (m_detached || m_streamingSuppressed) {
+        deref();
+        return;
+    }
+
+    // We have now streamed the whole script to V8 and it has parsed the
+    // script. We're ready for the next step: compiling and executing the
+    // script.
+    notifyFinishedToClient();
+
+    // The background thread no longer holds an implicit reference.
+    deref();
 }
 
 void ScriptStreamer::notifyFinishedToClient()
@@ -346,7 +405,12 @@ void ScriptStreamer::notifyFinishedToClient()
     // function calling notifyFinishedToClient was already scheduled in the task
     // queue and the upper layer decided that it's not interested in the script
     // and called removeClient.
-    if (isFinished() && m_client)
+    {
+        MutexLocker locker(m_mutex);
+        if (!isFinished())
+            return;
+    }
+    if (m_client)
         m_client->notifyFinished(m_resource);
 }
 
@@ -374,6 +438,10 @@ bool ScriptStreamer::startStreamingInternal(PendingScript& script, Settings* set
     ASSERT(isMainThread());
     if (!settings || !settings->v8ScriptStreamingEnabled())
         return false;
+    if (settings->v8ScriptStreamingMode() == ScriptStreamingModeOnlyAsyncAndDefer
+        && scriptType == PendingScript::ParsingBlocking)
+        return false;
+
     ScriptResource* resource = script.resource();
     ASSERT(!resource->isLoaded());
     if (!resource->url().protocolIsInHTTPFamily())
@@ -415,7 +483,7 @@ bool ScriptStreamer::startStreamingInternal(PendingScript& script, Settings* set
     // The Resource might go out of scope if the script is no longer needed. We
     // will soon call PendingScript::setStreamer, which makes the PendingScript
     // notify the ScriptStreamer when it is destroyed.
-    RefPtr<ScriptStreamer> streamer = adoptRef(new ScriptStreamer(resource, encoding, scriptType));
+    RefPtr<ScriptStreamer> streamer = adoptRef(new ScriptStreamer(resource, encoding, scriptType, settings->v8ScriptStreamingMode()));
 
     // Decide what kind of cached data we should produce while streaming. By
     // default, we generate the parser cache for streamed scripts, to emulate

third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.h

@@ -5,6 +5,7 @@
 #ifndef ScriptStreamer_h
 #define ScriptStreamer_h
 
+#include "bindings/core/v8/ScriptStreamingMode.h"
 #include "core/dom/PendingScript.h"
 #include "wtf/RefCounted.h"
 
@@ -82,7 +83,7 @@ public:
 
     // Called by ScriptStreamingTask when it has streamed all data to V8 and V8
     // has processed it.
-    void streamingComplete();
+    void streamingCompleteOnBackgroundThread();
 
     static void setSmallScriptThresholdForTesting(size_t threshold)
     {
@@ -96,10 +97,16 @@ private:
     // streamed. Non-const for testing.
     static size_t kSmallScriptThreshold;
 
-    ScriptStreamer(ScriptResource*, v8::ScriptCompiler::StreamedSource::Encoding, PendingScript::Type);
+    ScriptStreamer(ScriptResource*, v8::ScriptCompiler::StreamedSource::Encoding, PendingScript::Type, ScriptStreamingMode);
 
+    void streamingComplete();
     void notifyFinishedToClient();
 
+    bool shouldBlockMainThread() const
+    {
+        return m_scriptStreamingMode == ScriptStreamingModeAllPlusBlockParsingBlocking && m_scriptType == PendingScript::ParsingBlocking;
+    }
+
     static const char* startedStreamingHistogramName(PendingScript::Type);
 
     static bool startStreamingInternal(PendingScript&, Settings*, ScriptState*, PendingScript::Type);
@@ -118,12 +125,14 @@ private:
     ScriptResourceClient* m_client;
     WTF::OwnPtr<v8::ScriptCompiler::ScriptStreamingTask> m_task;
     bool m_loadingFinished; // Whether loading from the network is done.
-    bool m_parsingFinished; // Whether the V8 side processing is done.
+    // Whether the V8 side processing is done. Will be used by the main thread
+    // and the streamer thread; guarded by m_mutex.
+    bool m_parsingFinished;
     // Whether we have received enough data to start the streaming.
     bool m_haveEnoughDataForStreaming;
 
     // Whether the script source code should be retrieved from the Resource
-    // instead of the ScriptStreamer.
+    // instead of the ScriptStreamer; guarded by m_mutex.
     bool m_streamingSuppressed;
 
     // What kind of cached data V8 produces during streaming.
@@ -131,6 +140,18 @@ private:
 
     // For recording metrics for different types of scripts separately.
     PendingScript::Type m_scriptType;
+
+    // Streaming mode defines whether the main thread should block and wait for
+    // the parsing to complete after the load has finished. See
+    // ScriptStreamer::notifyFinished for more information.
+    ScriptStreamingMode m_scriptStreamingMode;
+    Mutex m_mutex;
+    ThreadCondition m_parsingFinishedCondition;
+    // Whether the main thread is currently waiting on the parser thread in
+    // notifyFinished(). This also defines which thread should do the cleanup of
+    // the parsing task: if the main thread is waiting, the main thread should
+    // do it, otherwise the parser thread should do it. Guarded by m_mutex.
+    bool m_mainThreadWaitingForParserThread;
 };
 
 } // namespace blink

third_party/WebKit/Source/bindings/core/v8/ScriptStreamerTest.cpp

@@ -8,6 +8,7 @@
 
 #include "bindings/core/v8/ScriptSourceCode.h"
 #include "bindings/core/v8/ScriptStreamerThread.h"
+#include "bindings/core/v8/ScriptStreamingMode.h"
 #include "bindings/core/v8/V8Binding.h"
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "core/dom/PendingScript.h"
@@ -58,7 +59,9 @@ private:
     PendingScript m_pendingScript;
 };
 
-class ScriptStreamingTest : public testing::Test {
+// The bool param for ScriptStreamingTest controls whether to make the main
+// thread block and wait for parsing.
+class ScriptStreamingTest : public testing::TestWithParam<bool> {
 public:
     ScriptStreamingTest()
         : m_scope(v8::Isolate::GetCurrent())
@@ -68,6 +71,8 @@ public:
         , m_pendingScript(PendingScriptWrapper::create(0, m_resource)) // Takes ownership of m_resource.
     {
         m_settings->setV8ScriptStreamingEnabled(true);
+        if (GetParam())
+            m_settings->setV8ScriptStreamingMode(ScriptStreamingModeAllPlusBlockParsingBlocking);
         m_resource->setLoading(true);
         ScriptStreamer::setSmallScriptThresholdForTesting(0);
     }
@@ -140,7 +145,7 @@ private:
     bool m_finished;
 };
 
-TEST_F(ScriptStreamingTest, CompilingStreamedScript)
+TEST_P(ScriptStreamingTest, CompilingStreamedScript)
 {
     // Test that we can successfully compile a streamed script.
     ScriptStreamer::startStreaming(pendingScript(), m_settings.get(), m_scope.scriptState(), PendingScript::ParsingBlocking);
@@ -169,7 +174,7 @@ TEST_F(ScriptStreamingTest, CompilingStreamedScript)
     EXPECT_FALSE(tryCatch.HasCaught());
 }
 
-TEST_F(ScriptStreamingTest, CompilingStreamedScriptWithParseError)
+TEST_P(ScriptStreamingTest, CompilingStreamedScriptWithParseError)
 {
     // Test that scripts with parse errors are handled properly. In those cases,
     // the V8 side typically finished before loading finishes: make sure we
@@ -202,7 +207,7 @@ TEST_F(ScriptStreamingTest, CompilingStreamedScriptWithParseError)
     EXPECT_TRUE(tryCatch.HasCaught());
 }
 
-TEST_F(ScriptStreamingTest, CancellingStreaming)
+TEST_P(ScriptStreamingTest, CancellingStreaming)
 {
     // Test that the upper layers (PendingScript and up) can be ramped down
     // while streaming is ongoing, and ScriptStreamer handles it gracefully.
@@ -229,7 +234,7 @@ TEST_F(ScriptStreamingTest, CancellingStreaming)
     EXPECT_FALSE(client.finished());
 }
 
-TEST_F(ScriptStreamingTest, SuppressingStreaming)
+TEST_P(ScriptStreamingTest, SuppressingStreaming)
 {
     // If we notice during streaming that there is a code cache, streaming
     // is suppressed (V8 doesn't parse while the script is loading), and the
@@ -257,7 +262,7 @@ TEST_F(ScriptStreamingTest, SuppressingStreaming)
     EXPECT_FALSE(sourceCode.streamer());
 }
 
-TEST_F(ScriptStreamingTest, EmptyScripts)
+TEST_P(ScriptStreamingTest, EmptyScripts)
 {
     // Empty scripts should also be streamed properly, that is, the upper layer
     // (ScriptResourceClient) should be notified when an empty script has been
@@ -278,7 +283,7 @@ TEST_F(ScriptStreamingTest, EmptyScripts)
     EXPECT_FALSE(sourceCode.streamer());
 }
 
-TEST_F(ScriptStreamingTest, SmallScripts)
+TEST_P(ScriptStreamingTest, SmallScripts)
 {
     // Small scripts shouldn't be streamed.
     ScriptStreamer::setSmallScriptThresholdForTesting(100);
@@ -301,7 +306,7 @@ TEST_F(ScriptStreamingTest, SmallScripts)
     EXPECT_FALSE(sourceCode.streamer());
 }
 
-TEST_F(ScriptStreamingTest, ScriptsWithSmallFirstChunk)
+TEST_P(ScriptStreamingTest, ScriptsWithSmallFirstChunk)
 {
     // If a script is long enough, if should be streamed, even if the first data
     // chunk is small.
@@ -331,6 +336,8 @@ TEST_F(ScriptStreamingTest, ScriptsWithSmallFirstChunk)
     EXPECT_FALSE(tryCatch.HasCaught());
 }
 
+INSTANTIATE_TEST_CASE_P(ScriptStreamingInstantiation, ScriptStreamingTest, ::testing::Values(false, true));
+
 } // namespace
 
 } // namespace blink

third_party/WebKit/Source/bindings/core/v8/ScriptStreamerThread.cpp

@@ -67,9 +67,7 @@ void ScriptStreamingTask::run()
     // Running the task can and will block: SourceStream::GetSomeData will get
     // called and it will block and wait for data from the network.
     m_v8Task->Run();
-    // Post a task to the main thread to signal that V8 has completed the
-    // streaming.
-    callOnMainThread(WTF::bind(&ScriptStreamer::streamingComplete, m_streamer));
+    m_streamer->streamingCompleteOnBackgroundThread();
     ScriptStreamerThread::shared()->taskDone();
 }
 

third_party/WebKit/Source/bindings/core/v8/ScriptStreamingMode.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ScriptStreamingMode_h
+#define ScriptStreamingMode_h
+
+// ScriptStreamingModes are used for evaluating different heuristics for when we
+// should start streaming.
+
+namespace blink {
+
+enum ScriptStreamingMode {
+    // Stream all scripts
+    ScriptStreamingModeAll,
+    // Stream only async and deferred scripts
+    ScriptStreamingModeOnlyAsyncAndDefer,
+    // Stream all scripts, block the main thread after loading when streaming
+    // parser blocking scripts.
+    ScriptStreamingModeAllPlusBlockParsingBlocking
+};
+
+} // namespace blink
+
+#endif // ScriptStreamingMode_h

third_party/WebKit/Source/bindings/core/v8/v8.gypi

@@ -90,6 +90,7 @@
             'ScriptStreamer.h',
             'ScriptStreamerThread.cpp',
             'ScriptStreamerThread.h',
+            'ScriptStreamingMode.h',
             'ScriptString.cpp',
             'ScriptString.h',
             'ScriptValue.cpp',

third_party/WebKit/Source/core/frame/Settings.h

@@ -27,6 +27,7 @@
 #ifndef Settings_h
 #define Settings_h
 
+#include "bindings/core/v8/ScriptStreamingMode.h"
 #include "bindings/core/v8/V8CacheOptions.h"
 #include "core/SettingsMacros.h"
 #include "core/css/PointerProperties.h"

third_party/WebKit/Source/core/frame/Settings.in

@@ -281,6 +281,7 @@ fullscreenSupported initial=true
 v8CacheOptions type=V8CacheOptions, initial=V8CacheOptionsOff
 
 v8ScriptStreamingEnabled initial=false
+v8ScriptStreamingMode type=ScriptStreamingMode, initial=ScriptStreamingModeAll
 
 # These values are bit fields for the properties of available pointing devices
 # and may take on multiple values (e.g. laptop with touchpad and touchscreen