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

Unified Diff: Source/bindings/core/v8/ScriptStreamer.cpp

Issue 680493002: Revert "Script streaming: Add an option to make the main thread block (wait for parsing)" (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Created 6 years, 2 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
« no previous file with comments | « Source/bindings/core/v8/ScriptStreamer.h ('k') | Source/bindings/core/v8/ScriptStreamerTest.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/bindings/core/v8/ScriptStreamer.cpp
diff --git a/Source/bindings/core/v8/ScriptStreamer.cpp b/Source/bindings/core/v8/ScriptStreamer.cpp
index 517dcb7013ff47540e799d9094ff68a1e905c7bc..efbe8a5cba741a17b9c1f5df61ce4d09c83f8a4a 100644
--- a/Source/bindings/core/v8/ScriptStreamer.cpp
+++ b/Source/bindings/core/v8/ScriptStreamer.cpp
@@ -13,7 +13,6 @@
#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"
@@ -225,20 +224,28 @@ void ScriptStreamer::startStreaming(PendingScript& script, Settings* settings, S
blink::Platform::current()->histogramEnumeration(startedStreamingHistogramName(scriptType), 0, 2);
}
-void ScriptStreamer::streamingCompleteOnBackgroundThread()
+void ScriptStreamer::streamingComplete()
{
- ASSERT(!isMainThread());
- MutexLocker locker(m_mutex);
- m_parsingFinished = true;
- if (shouldBlockMainThread()) {
- // Normally, the main thread is 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.
- m_parsingFinishedCondition.signal();
- } else {
- callOnMainThread(WTF::bind(&ScriptStreamer::streamingComplete, this));
+ 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.
+ m_parsingFinished = true;
+
+ notifyFinishedToClient();
+
+ // The background thread no longer holds an implicit reference.
+ deref();
}
void ScriptStreamer::cancel()
@@ -255,7 +262,6 @@ void ScriptStreamer::cancel()
void ScriptStreamer::suppressStreaming()
{
- MutexLocker locker(m_mutex);
ASSERT(!m_parsingFinished);
ASSERT(!m_loadingFinished);
m_streamingSuppressed = true;
@@ -288,8 +294,7 @@ 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 or in
- // notifyFinished.
+ // the corresponding deref() is in streamingComplete.
ref();
ScriptStreamingTask* task = new ScriptStreamingTask(m_task.release(), this);
ScriptStreamerThread::shared()->postTask(task);
@@ -313,33 +318,10 @@ 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);
- if (!isFinished()) {
- m_parsingFinishedCondition.wait(m_mutex);
- }
- ASSERT(isFinished());
- }
-
notifyFinishedToClient();
-
- if (shouldBlockMainThread() && m_parsingFinished) {
- // streamingComplete won't be called, so do the ramp-down work
- // here. Since m_parsingFinished is true, we know that there was a
- // background task and we need to deref().
- deref();
- }
}
-ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::StreamedSource::Encoding encoding, PendingScript::Type scriptType, ScriptStreamingMode mode)
+ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::StreamedSource::Encoding encoding, PendingScript::Type scriptType)
: m_resource(resource)
, m_detached(false)
, m_stream(new SourceStream(this))
@@ -350,37 +332,9 @@ ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::Str
, m_haveEnoughDataForStreaming(false)
, m_streamingSuppressed(false)
, m_scriptType(scriptType)
- , m_scriptStreamingMode(mode)
{
}
-void ScriptStreamer::streamingComplete()
-{
- // The background task is completed; do the necessary ramp-down in the main
- // thread.
- ASSERT(isMainThread());
- // In the blocking mode, the ramp-down is done in notifyFinished.
- ASSERT(!shouldBlockMainThread());
-
- // 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()
{
ASSERT(isMainThread());
@@ -392,7 +346,6 @@ 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.
- MutexLocker locker(m_mutex);
if (isFinished() && m_client)
m_client->notifyFinished(m_resource);
}
@@ -421,10 +374,6 @@ 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())
@@ -466,7 +415,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, settings->v8ScriptStreamingMode()));
+ RefPtr<ScriptStreamer> streamer = adoptRef(new ScriptStreamer(resource, encoding, scriptType));
// Decide what kind of cached data we should produce while streaming. By
// default, we generate the parser cache for streamed scripts, to emulate
« no previous file with comments | « Source/bindings/core/v8/ScriptStreamer.h ('k') | Source/bindings/core/v8/ScriptStreamerTest.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698