Revert "[v8] Re-post non-blocking ScriptStreamer tasks"

This reverts commit 5912b768.

Reason for revert: Seems to cause Scanner crashes

Original change's description:
> [v8] Re-post non-blocking ScriptStreamer tasks
> 
> Posted blocking ScriptStreamer tasks can end up in a queue, and have
> their Resource complete loading before the task even starts. When this
> happens, these tasks will no longer block on network input, but they
> will still be posted to the blocking task runner, which does not spin up
> new threads if the running task is not blocked. When there are a lot of
> small functions posted for streaming, this results in a slightly
> paradoxical starvation, where the blocking task runner is processing
> lots of no-longer-blocking tasks all in one thread, as it has no reason
> to spin up new threads.
> 
> To avoid this, if a streaming task isn't started before the resource
> finishes loading, we can cancel this posted task, and instead post a new
> non-blocking task to the non-blocking thread pool. This frees up the
> blocking task runner to process only blocking tasks (and thus allows it
> to spin up new threads for them).
> 
> Bug: chromium:865098
> Bug: chromium:866868
> Change-Id: Iceb282582781109ade233366d1009c3be50db3a5
> Reviewed-on: https://chromium-review.googlesource.com/1174380
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Alexander Timin <altimin@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583649}

TBR=skyostil@google.com,gab@chromium.org,rmcilroy@chromium.org,haraken@chromium.org,kouhei@chromium.org,skyostil@chromium.org,altimin@chromium.org,leszeks@chromium.org

Change-Id: Iffbab7a8994d12ffce53c6b5b76d19f4db79df0b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:865098, chromium:866868, chromium:875162
Reviewed-on: https://chromium-review.googlesource.com/1179641Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583995}

third_party/blink/renderer/bindings/core/v8/script_streamer.cc

@@ -308,9 +308,7 @@ void ScriptStreamer::SuppressStreaming(NotStreamingReason reason) {
   suppressed_reason_ = reason;
 }
 
-namespace {
-
-void RunScriptStreamingTask(
+static void RunScriptStreamingTask(
     std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask> task,
     ScriptStreamer* streamer) {
   TRACE_EVENT1(
@@ -323,23 +321,6 @@ void RunScriptStreamingTask(
   streamer->StreamingCompleteOnBackgroundThread();
 }
 
-void RunBlockingScriptStreamingTask(
-    std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask> task,
-    ScriptStreamer* streamer,
-    std::atomic_flag* blocking_task_started_or_cancelled) {
-  if (!blocking_task_started_or_cancelled->test_and_set())
-    return;
-  RunScriptStreamingTask(std::move(task), streamer);
-}
-
-void RunNonBlockingScriptStreamingTask(
-    std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask> task,
-    ScriptStreamer* streamer) {
-  RunScriptStreamingTask(std::move(task), streamer);
-}
-
-}  // namespace
-
 bool ScriptStreamer::HasEnoughDataForStreaming(size_t resource_buffer_size) {
   if (RuntimeEnabledFeatures::ScheduledScriptStreamingEnabled()) {
     // Enable streaming for small scripts, but we must still check the BOM
@@ -427,24 +408,17 @@ void ScriptStreamer::NotifyAppendData(ScriptResource* resource) {
     }
 
     if (RuntimeEnabledFeatures::ScheduledScriptStreamingEnabled()) {
-      // Script streaming tasks are high priority, as they can block the parser,
-      // and they can (and probably will) block during their own execution as
-      // they wait for more input.
-      //
-      // Pass through the atomic cancellation token which is set to true by the
-      // task when it is started, or set to true by the streamer if it wants to
-      // cancel the task.
+      // Script streaming tasks are high priority, as they can block the
+      // parser, and they can (and probably will) block during their own
+      // execution as they wait for more input.
       //
       // TODO(leszeks): Decrease the priority of these tasks where possible.
       BackgroundScheduler::PostOnBackgroundThreadWithTraits(
           FROM_HERE, {base::TaskPriority::USER_BLOCKING, base::MayBlock()},
-          CrossThreadBind(RunBlockingScriptStreamingTask,
+          CrossThreadBind(RunScriptStreamingTask,
                           WTF::Passed(std::move(script_streaming_task)),
-                          WrapCrossThreadPersistent(this),
-                          WTF::CrossThreadUnretained(
-                              &blocking_task_started_or_cancelled_)));
+                          WrapCrossThreadPersistent(this)));
     } else {
-      blocking_task_started_or_cancelled_.test_and_set();
       ScriptStreamerThread::Shared()->PostTask(
           CrossThreadBind(&ScriptStreamerThread::RunScriptStreamingTask,
                           WTF::Passed(std::move(script_streaming_task)),
@@ -466,34 +440,8 @@ void ScriptStreamer::NotifyFinished() {
     SuppressStreaming(kScriptTooSmall);
   }
 
-  if (stream_) {
-    // If the corresponding blocking task hasn't started yet, cancel it and post
-    // a non-blocking task, since we know now that all the data is received and
-    // we will no longer block.
-    //
-    // TODO(874080): Once we have mutable task traits, simply unmark the
-    // existing task as no longer MayBlock.
-    if (RuntimeEnabledFeatures::ScheduledScriptStreamingEnabled() &&
-        !blocking_task_started_or_cancelled_.test_and_set()) {
-      ScriptState::Scope scope(script_state_);
-      std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask>
-          script_streaming_task(
-              base::WrapUnique(v8::ScriptCompiler::StartStreamingScript(
-                  script_state_->GetIsolate(), source_.get(),
-                  compile_options_)));
-
-      // The task creation shouldn't fail, since it didn't fail before during
-      // NotifyAppendData.
-      CHECK(script_streaming_task);
-      BackgroundScheduler::PostOnBackgroundThreadWithTraits(
-          FROM_HERE, {base::TaskPriority::USER_BLOCKING},
-          CrossThreadBind(RunNonBlockingScriptStreamingTask,
-                          WTF::Passed(std::move(script_streaming_task)),
-                          WrapCrossThreadPersistent(this)));
-    }
-
+  if (stream_)
     stream_->DidFinishLoading();
-  }
   loading_finished_ = true;
 
   NotifyFinishedToClient();

third_party/blink/renderer/bindings/core/v8/script_streamer.h

@@ -5,7 +5,6 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_SCRIPT_STREAMER_H_
 #define THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_SCRIPT_STREAMER_H_
 
-#include <atomic>
 #include <memory>
 
 #include "base/single_thread_task_runner.h"
@@ -147,11 +146,6 @@ class CORE_EXPORT ScriptStreamer final
   // Whether we have received enough data to start the streaming.
   bool have_enough_data_for_streaming_;
 
-  // Flag used to allow atomic cancelling and reposting of the streaming task
-  // when the load completes without the task yet starting.
-  // TODO(874080): Remove once we can mutate task traits.
-  std::atomic_flag blocking_task_started_or_cancelled_;
-
   // Whether the script source code should be retrieved from the Resource
   // instead of the ScriptStreamer.
   bool streaming_suppressed_;