Revert "Reland^3 "[blink] Control ScriptStreamer with ScriptResource""

This reverts commit 31bcf8aa.

Reason for revert: Canary crasher, Bug:902832

Original change's description:
> Reland^3 "[blink] Control ScriptStreamer with ScriptResource"
> 
> This is a reland of 5528336d, which
> was a reland of dd9df78c, which was
> a reland of 7bd91635.
> 
> VR tests have been properly deflaked in https://crrev.com/c/1319715, this
> is otherwise unchanged.
> 
> Original change's description:
> > Reland "Reland "[blink] Control ScriptStreamer with ScriptResource""
> >
> > This reverts commit c564235f.
> >
> > No changes from the original, VR flakes are caused by issues with
> > loadUrl and have been deflaked (see 894796 for details)
> >
> > Original change's description:
> > > Revert "Reland "[blink] Control ScriptStreamer with ScriptResource""
> > >
> > > This reverts commit dd9df78c.
> > >
> > > Reason for revert: Makes VR tests flaky (https://crbug.com/894796)
> > >
> > > Original change's description:
> > > > Reland "[blink] Control ScriptStreamer with ScriptResource"
> > > >
> > > > This is a reland of 7bd91635
> > > >
> > > > No changes from the original, revert reason is fixed with
> > > > https://chromium-review.googlesource.com/c/1283037
> > > >
> > > > Original change's description:
> > > > > [blink] Control ScriptStreamer with ScriptResource
> > > > >
> > > > > Move the ScriptStreamer control logic from ClassicPendingScript to
> > > > > ScriptResource. In the future, this will allow us to start streaming
> > > > > a ScriptResource before the ClassicPendingScript is available, e.g on
> > > > > preload.
> > > > >
> > > > > The ClassicPendingScript now only sends one "finished" callback, from
> > > > > ScriptResource. This callback is either when the streaming completes,
> > > > > or when there is no streaming and the ClassicPendingScript has told
> > > > > the ScriptResource that it doesn't intend to start streaming. In the
> > > > > long run, we want all scripts to go through the streaming interface,
> > > > > and we want ScriptResource to control when streaming starts, so the
> > > > > "no more streaming" call will disappear -- however it is necessary to
> > > > > maintain the current behaviour, and be able to control it with a
> > > > > runtime flag.
> > > > >
> > > > > Bug: chromium:865098
> > > > > Change-Id: I2c4cc16635a6538b7ff24a24463fe6f34efd8a2e
> > > > > Reviewed-on: https://chromium-review.googlesource.com/c/1181430
> > > > > Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> > > > > Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
> > > > > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> > > > > Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
> > > > > Cr-Commit-Position: refs/heads/master@{#599148}
> > > >
> > > > TBR=kouhei@chromium.org,kinuko@chromium.org
> > > >
> > > > Bug: chromium:865098
> > > > Change-Id: I0df324ef8e284f4d3d55a1721d71be5a66a3285c
> > > > Reviewed-on: https://chromium-review.googlesource.com/c/1286450
> > > > Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> > > > Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
> > > > Cr-Commit-Position: refs/heads/master@{#601143}
> > >
> > > TBR=kinuko@chromium.org,rmcilroy@chromium.org,haraken@chromium.org,hiroshige@chromium.org,kouhei@chromium.org,vogelheim@chromium.org,leszeks@chromium.org
> > >
> > > # Not skipping CQ checks because original CL landed > 1 day ago.
> > >
> > > Bug: chromium:865098
> > > Change-Id: Ibdbd7e80d0fcfacc84a73e83d165581c7f1d02d2
> > > Reviewed-on: https://chromium-review.googlesource.com/c/1297138
> > > Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> > > Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#602310}
> >
> > TBR=kinuko@chromium.org,rmcilroy@chromium.org,haraken@chromium.org,hiroshige@chromium.org,kouhei@chromium.org,vogelheim@chromium.org,leszeks@chromium.org
> >
> > # Not skipping CQ checks because original CL landed > 1 day ago.
> >
> > Bug: chromium:865098
> > Change-Id: I9794fe5366aa8cfc9291d53271ed680465b29a76
> > Reviewed-on: https://chromium-review.googlesource.com/c/1317566
> > Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> > Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#605295}
> 
> TBR=kinuko@chromium.org,rmcilroy@chromium.org,haraken@chromium.org,hiroshige@chromium.org,kouhei@chromium.org,vogelheim@chromium.org
> 
> Bug: chromium:865098
> Change-Id: I8d6101f51eb33b19ec332a06a310172b371b7fa4
> Reviewed-on: https://chromium-review.googlesource.com/c/1318923
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605815}

TBR=kinuko@chromium.org,rmcilroy@chromium.org,haraken@chromium.org,hiroshige@chromium.org,kouhei@chromium.org,vogelheim@chromium.org,leszeks@chromium.org

Change-Id: I9a9337344d5a11fcc6b16aab1d8ad600ffef4e10
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:865098
Reviewed-on: https://chromium-review.googlesource.com/c/1323655Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606095}

third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt

@@ -58,7 +58,7 @@ v8.compile Properties:
         columnNumber : 0
         consumedCacheSize : <number>
         lineNumber : 0
-        notStreamedReason : "already used streamed data"
+        notStreamedReason : "resource no longer alive"
         streamed : <boolean>
         url : .../devtools/resources/v8-cache-script.js
     }

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

@@ -14,7 +14,7 @@
 #include "third_party/blink/renderer/core/frame/settings.h"
 #include "third_party/blink/renderer/core/html/parser/text_resource_decoder.h"
 #include "third_party/blink/renderer/core/inspector/inspector_trace_events.h"
-#include "third_party/blink/renderer/core/loader/resource/script_resource.h"
+#include "third_party/blink/renderer/core/script/classic_pending_script.h"
 #include "third_party/blink/renderer/platform/bindings/v8_per_isolate_data.h"
 #include "third_party/blink/renderer/platform/cross_thread_functional.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
@@ -343,7 +343,7 @@ bool ScriptStreamer::HasEnoughDataForStreaming(size_t resource_buffer_size) {
   return resource_buffer_size >= small_script_threshold_;
 }
 
-void ScriptStreamer::NotifyAppendData() {
+void ScriptStreamer::NotifyAppendData(ScriptResource* resource) {
   DCHECK(IsMainThread());
   if (streaming_suppressed_)
     return;
@@ -351,8 +351,8 @@ void ScriptStreamer::NotifyAppendData() {
     // 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
     // to start the streaming.
-    DCHECK(script_resource_->ResourceBuffer());
-    if (!HasEnoughDataForStreaming(script_resource_->ResourceBuffer()->size()))
+    DCHECK(resource->ResourceBuffer());
+    if (!HasEnoughDataForStreaming(resource->ResourceBuffer()->size()))
       return;
     have_enough_data_for_streaming_ = true;
 
@@ -360,8 +360,8 @@ void ScriptStreamer::NotifyAppendData() {
       // Check for BOM (byte order marks), because that might change our
       // understanding of the data encoding.
       char maybe_bom[kMaximumLengthOfBOM] = {};
-      if (!script_resource_->ResourceBuffer()->GetBytes(maybe_bom,
-                                                        kMaximumLengthOfBOM)) {
+      if (!resource->ResourceBuffer()->GetBytes(maybe_bom,
+                                                kMaximumLengthOfBOM)) {
         NOTREACHED();
         return;
       }
@@ -369,7 +369,7 @@ void ScriptStreamer::NotifyAppendData() {
       std::unique_ptr<TextResourceDecoder> decoder(
           TextResourceDecoder::Create(TextResourceDecoderOptions(
               TextResourceDecoderOptions::kPlainTextContent,
-              WTF::TextEncoding(script_resource_->Encoding()))));
+              WTF::TextEncoding(resource->Encoding()))));
       decoder->CheckForBOM(maybe_bom, kMaximumLengthOfBOM);
 
       // The encoding may change when we see the BOM. Check for BOM now
@@ -441,7 +441,7 @@ void ScriptStreamer::NotifyAppendData() {
 
   }
   if (stream_)
-    stream_->DidReceiveData(script_resource_, this);
+    stream_->DidReceiveData(resource, this);
 }
 
 void ScriptStreamer::NotifyFinished() {
@@ -490,10 +490,10 @@ void ScriptStreamer::NotifyFinished() {
 }
 
 ScriptStreamer::ScriptStreamer(
-    ScriptResource* script_resource,
+    ClassicPendingScript* script,
     v8::ScriptCompiler::CompileOptions compile_options,
     scoped_refptr<base::SingleThreadTaskRunner> loading_task_runner)
-    : script_resource_(script_resource),
+    : pending_script_(script),
       detached_(false),
       stream_(nullptr),
       loading_finished_(false),
@@ -502,8 +502,8 @@ ScriptStreamer::ScriptStreamer(
       streaming_suppressed_(false),
       suppressed_reason_(kInvalid),
       compile_options_(compile_options),
-      script_url_string_(script_resource->Url().Copy().GetString()),
-      script_resource_identifier_(script_resource->Identifier()),
+      script_url_string_(script->GetResource()->Url().Copy().GetString()),
+      script_resource_identifier_(script->GetResource()->Identifier()),
       // Unfortunately there's no dummy encoding value in the enum; let's use
       // one we don't stream.
       encoding_(v8::ScriptCompiler::StreamedSource::TWO_BYTE),
@@ -517,7 +517,7 @@ void ScriptStreamer::Prefinalize() {
 }
 
 void ScriptStreamer::Trace(blink::Visitor* visitor) {
-  visitor->Trace(script_resource_);
+  visitor->Trace(pending_script_);
 }
 
 void ScriptStreamer::StreamingComplete() {
@@ -550,32 +550,64 @@ void ScriptStreamer::NotifyFinishedToClient() {
   if (!IsFinished())
     return;
 
-  script_resource_->StreamingFinished();
+  pending_script_->StreamingFinished();
 }
 
-ScriptStreamer* ScriptStreamer::Create(
-    ScriptResource* resource,
+void ScriptStreamer::StartStreaming(
+    ClassicPendingScript* script,
     scoped_refptr<base::SingleThreadTaskRunner> loading_task_runner,
     NotStreamingReason* not_streaming_reason) {
   DCHECK(IsMainThread());
   *not_streaming_reason = kInvalid;
+  ScriptResource* resource = ToScriptResource(script->GetResource());
   if (!resource->Url().ProtocolIsInHTTPFamily()) {
     *not_streaming_reason = kNotHTTP;
-    return nullptr;
+    return;
+  }
+  if (resource->IsCacheValidator()) {
+    // This happens e.g., during reloads. We're actually not going to load
+    // the current Resource of the ClassicPendingScript but switch to another
+    // Resource -> don't stream.
+    *not_streaming_reason = kReload;
+    return;
   }
   if (resource->IsLoaded() && !resource->ResourceBuffer()) {
     // This happens for already loaded resources, e.g. if resource
     // validation fails. In that case, the loading subsystem will discard
     // the resource buffer.
     *not_streaming_reason = kNoResourceBuffer;
-    return nullptr;
+    return;
   }
   // We cannot filter out short scripts, even if we wait for the HTTP headers
   // to arrive: the Content-Length HTTP header is not sent for chunked
   // downloads.
 
-  return new ScriptStreamer(resource, v8::ScriptCompiler::kNoCompileOptions,
-                            std::move(loading_task_runner));
+  ScriptStreamer* streamer =
+      new ScriptStreamer(script, v8::ScriptCompiler::kNoCompileOptions,
+                         std::move(loading_task_runner));
+
+  // If this script was ready when streaming began, no callbacks will be
+  // received to populate the data for the ScriptStreamer, so send them now.
+  // Note that this script may be processing an asynchronous cache hit, in
+  // which case ScriptResource::IsLoaded() will be true, but ready_state_ will
+  // not be kReadyStreaming. In that case, ScriptStreamer can listen to the
+  // async callbacks generated by the cache hit.
+  if (script->IsReady()) {
+    DCHECK(resource->IsLoaded());
+    streamer->NotifyAppendData(resource);
+    if (streamer->StreamingSuppressed()) {
+      *not_streaming_reason = streamer->StreamingSuppressedReason();
+      return;
+    }
+  }
+
+  // The Resource might go out of scope if the script is no longer needed.
+  // This makes ClassicPendingScript notify the ScriptStreamer when it is
+  // destroyed.
+  script->SetStreamer(streamer);
+
+  if (script->IsReady())
+    streamer->NotifyFinished();
 }
 
 }  // namespace blink

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

@@ -17,11 +17,12 @@
 
 namespace blink {
 
+class ClassicPendingScript;
 class ScriptResource;
 class SourceStream;
 
 // ScriptStreamer streams incomplete script data to V8 so that it can be parsed
-// while it's loaded. ScriptResource holds a reference to ScriptStreamer.
+// while it's loaded. ClassicPendingScript holds a reference to ScriptStreamer.
 // At the moment, ScriptStreamer is only used for parser blocking scripts; this
 // means that the Document stays stable and no other scripts are executing
 // while we're streaming. It is possible, though, that Document and the
@@ -39,21 +40,18 @@ class CORE_EXPORT ScriptStreamer final
   enum NotStreamingReason {
     kAlreadyLoaded,  // DEPRECATED
     kNotHTTP,
-    kRevalidate,
+    kReload,
     kContextNotValid,  // DEPRECATED
     kEncodingNotSupported,
-    // TODO(leszeks): Deprecate once scheduled streaming is on by default
     kThreadBusy,
     kV8CannotStream,
     kScriptTooSmall,
     kNoResourceBuffer,
     kHasCodeCache,
-    kStreamerNotReadyOnGetSource,  // DEPRECATED
+    kStreamerNotReadyOnGetSource,
     kInlineScript,
     kDidntTryToStartStreaming,
     kErrorOccurred,
-    kStreamingDisabled,
-    kSecondScriptResourceUse,
     kWorkerTopLevelScript,
 
     // Pseudo values that should never be seen in reported metrics
@@ -64,11 +62,11 @@ class CORE_EXPORT ScriptStreamer final
   ~ScriptStreamer();
   void Trace(blink::Visitor*);
 
-  // Create a script streamer which will stream the given ScriptResource into V8
-  // as it loads.
-  static ScriptStreamer* Create(ScriptResource*,
-                                scoped_refptr<base::SingleThreadTaskRunner>,
-                                NotStreamingReason* not_streaming_reason);
+  // Launches a task (on a background thread) which will stream the given
+  // ClassicPendingScript into V8 as it loads.
+  static void StartStreaming(ClassicPendingScript*,
+                             scoped_refptr<base::SingleThreadTaskRunner>,
+                             NotStreamingReason* not_streaming_reason);
 
   // Returns false if we cannot stream the given encoding.
   static bool ConvertEncoding(const char* encoding_name,
@@ -101,12 +99,8 @@ class CORE_EXPORT ScriptStreamer final
     return suppressed_reason_;
   }
 
-  // Called by ScriptResource when data arrives from the network.
-  void NotifyAppendData();
-  // Called by ScriptResource when loading has completed.
-  //
-  // Should not be called synchronously, as it can trigger script resource
-  // client callbacks.
+  // Called by ClassicPendingScript when data arrives from the network.
+  void NotifyAppendData(ScriptResource*);
   void NotifyFinished();
 
   // Called by ScriptStreamingTask when it has streamed all data to V8 and V8
@@ -129,21 +123,17 @@ class CORE_EXPORT ScriptStreamer final
   // Maximum size of the BOM marker.
   static constexpr size_t kMaximumLengthOfBOM = 4;
 
-  ScriptStreamer(ScriptResource*,
+  ScriptStreamer(ClassicPendingScript*,
                  v8::ScriptCompiler::CompileOptions,
                  scoped_refptr<base::SingleThreadTaskRunner>);
 
   void Prefinalize();
 
-  // Should not be called synchronously, as it can trigger script resource
-  // client callbacks.
   void StreamingComplete();
-  // Should not be called synchronously, as it can trigger script resource
-  // client callbacks.
   void NotifyFinishedToClient();
   bool HasEnoughDataForStreaming(size_t resource_buffer_size);
 
-  Member<ScriptResource> script_resource_;
+  Member<ClassicPendingScript> pending_script_;
   // Whether ScriptStreamer is detached from the Resource. In those cases, the
   // script data is not needed any more, and the client won't get notified
   // when the loading and streaming are done.

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

@@ -159,7 +159,10 @@ class TestPendingScriptClient final
 TEST_F(ScriptStreamingTest, CompilingStreamedScript) {
   // Test that we can successfully compile a streamed script.
   V8TestingScope scope;
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
 
@@ -196,7 +199,10 @@ TEST_F(ScriptStreamingTest, CompilingStreamedScriptWithParseError) {
   // the V8 side typically finished before loading finishes: make sure we
   // handle it gracefully.
   V8TestingScope scope;
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
   AppendData("function foo() {");
@@ -234,7 +240,10 @@ TEST_F(ScriptStreamingTest, CancellingStreaming) {
   // Test that the upper layers (PendingScript and up) can be ramped down
   // while streaming is ongoing, and ScriptStreamer handles it gracefully.
   V8TestingScope scope;
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
   AppendData("function foo() {");
@@ -255,11 +264,14 @@ TEST_F(ScriptStreamingTest, CancellingStreaming) {
   EXPECT_FALSE(client->Finished());
 }
 
-TEST_F(ScriptStreamingTest, DataAfterDisposingPendingScript) {
+TEST_F(ScriptStreamingTest, DataAfterCancellingStreaming) {
   // Test that the upper layers (PendingScript and up) can be ramped down
   // before streaming is started, and ScriptStreamer handles it gracefully.
   V8TestingScope scope;
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
 
@@ -279,7 +291,6 @@ TEST_F(ScriptStreamingTest, DataAfterDisposingPendingScript) {
                               // holding on to it here.
 
   // Make sure the streaming starts.
-  AppendData(resource, "function foo() {");
   AppendPadding(resource);
   resource.Clear();
 
@@ -295,7 +306,10 @@ TEST_F(ScriptStreamingTest, SuppressingStreaming) {
   // upper layer (ScriptResourceClient) should get a notification when the
   // script is loaded.
   V8TestingScope scope;
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
   AppendData("function foo() {");
@@ -324,7 +338,10 @@ TEST_F(ScriptStreamingTest, EmptyScripts) {
   // (ScriptResourceClient) should be notified when an empty script has been
   // loaded.
   V8TestingScope scope;
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
 
@@ -343,7 +360,10 @@ TEST_F(ScriptStreamingTest, SmallScripts) {
   V8TestingScope scope;
   ScriptStreamer::SetSmallScriptThresholdForTesting(100);
 
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
 
@@ -365,7 +385,10 @@ TEST_F(ScriptStreamingTest, ScriptsWithSmallFirstChunk) {
   V8TestingScope scope;
   ScriptStreamer::SetSmallScriptThresholdForTesting(100);
 
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
 
@@ -401,7 +424,10 @@ TEST_F(ScriptStreamingTest, EncodingChanges) {
   V8TestingScope scope;
   GetResource()->SetEncodingForTest("windows-1252");
 
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
 
@@ -439,7 +465,10 @@ TEST_F(ScriptStreamingTest, EncodingFromBOM) {
   // This encoding is wrong on purpose.
   GetResource()->SetEncodingForTest("windows-1252");
 
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
 
@@ -471,7 +500,10 @@ TEST_F(ScriptStreamingTest, EncodingFromBOM) {
 // A test for crbug.com/711703. Should not crash.
 TEST_F(ScriptStreamingTest, GarbageCollectDuringStreaming) {
   V8TestingScope scope;
-  GetResource()->StartStreaming(loading_task_runner_);
+  ScriptStreamer::NotStreamingReason reason;
+  ScriptStreamer::StartStreaming(GetPendingScript(), loading_task_runner_,
+                                 &reason);
+  GetPendingScript()->SetNotStreamingReasonForTest(reason);
 
   TestPendingScriptClient* client = new TestPendingScriptClient;
   GetPendingScript()->WatchForLoad(client);
@@ -483,32 +515,6 @@ TEST_F(ScriptStreamingTest, GarbageCollectDuringStreaming) {
       BlinkGC::kEagerSweeping, BlinkGC::GCReason::kForcedGC);
 }
 
-TEST_F(ScriptStreamingTest, ResourceSetRevalidatingRequest) {
-  V8TestingScope scope;
-  GetResource()->StartStreaming(loading_task_runner_);
-
-  TestPendingScriptClient* client = new TestPendingScriptClient;
-  GetPendingScript()->WatchForLoad(client);
-
-  // Kick the streaming off.
-  AppendData("function foo() {");
-  AppendPadding();
-  AppendData("}");
-  Finish();
-  ProcessTasksUntilStreamingComplete();
-
-  // Second start streaming should fail.
-  EXPECT_FALSE(GetResource()->StartStreaming(loading_task_runner_));
-
-  ResourceRequest request(GetResource()->Url());
-  GetResource()->SetRevalidatingRequest(request);
-
-  // The next streaming should still fail, but the reason should be
-  // "kRevalidate".
-  EXPECT_FALSE(GetResource()->StartStreaming(loading_task_runner_));
-  EXPECT_EQ(GetResource()->NoStreamerReason(), ScriptStreamer::kRevalidate);
-}
-
 }  // namespace
 
 }  // namespace blink

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

@@ -93,15 +93,11 @@ class V8ScriptRunnerTest : public testing::Test {
   }
 
   ScriptResource* CreateEmptyResource() {
-    ScriptResource* resource =
-        ScriptResource::CreateForTest(NullURL(), UTF8Encoding());
-    resource->SetClientIsWaitingForFinished();
-    return resource;
+    return ScriptResource::CreateForTest(NullURL(), UTF8Encoding());
   }
 
   ScriptResource* CreateResource(const WTF::TextEncoding& encoding) {
     ScriptResource* resource = ScriptResource::CreateForTest(Url(), encoding);
-    resource->SetClientIsWaitingForFinished();
     String code = Code();
     ResourceResponse response(Url());
     response.SetHTTPStatusCode(200);

third_party/blink/renderer/core/inspector/inspector_trace_events.cc

@@ -371,8 +371,8 @@ const char* NotStreamedReasonString(ScriptStreamer::NotStreamingReason reason) {
   switch (reason) {
     case ScriptStreamer::kNotHTTP:
       return "not http/https protocol";
-    case ScriptStreamer::kRevalidate:
-      return "revalidation event";
+    case ScriptStreamer::kReload:
+      return "reload event";
     case ScriptStreamer::kContextNotValid:
       return "script context not valid";
     case ScriptStreamer::kEncodingNotSupported:
@@ -395,10 +395,6 @@ const char* NotStreamedReasonString(ScriptStreamer::NotStreamingReason reason) {
       return "start streaming not called";
     case ScriptStreamer::kErrorOccurred:
       return "an error occurred";
-    case ScriptStreamer::kStreamingDisabled:
-      return "already disabled streaming";
-    case ScriptStreamer::kSecondScriptResourceUse:
-      return "already used streamed data";
     case ScriptStreamer::kWorkerTopLevelScript:
       return "worker top-level scripts are not streamable";
     case ScriptStreamer::kAlreadyLoaded:

third_party/blink/renderer/core/loader/document_loader.cc

@@ -266,8 +266,7 @@ Resource* DocumentLoader::StartPreload(ResourceType type,
       break;
     case ResourceType::kScript:
       params.SetRequestContext(mojom::RequestContextType::SCRIPT);
-      resource = ScriptResource::Fetch(params, Fetcher(), nullptr,
-                                       ScriptResource::kAllowStreaming);
+      resource = ScriptResource::Fetch(params, Fetcher(), nullptr);
       break;
     case ResourceType::kCSSStyleSheet:
       resource = CSSStyleSheetResource::Fetch(params, Fetcher(), nullptr);

third_party/blink/renderer/core/loader/modulescript/document_module_script_fetcher.cc

@@ -27,8 +27,7 @@ void DocumentModuleScriptFetcher::Fetch(FetchParameters& fetch_params,
   if (FetchIfLayeredAPI(fetch_params))
     return;
 
-  ScriptResource::Fetch(fetch_params, fetcher_, this,
-                        ScriptResource::kNoStreaming);
+  ScriptResource::Fetch(fetch_params, fetcher_, this);
 }
 
 void DocumentModuleScriptFetcher::NotifyFinished(Resource* resource) {

third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc

@@ -217,10 +217,8 @@ TEST_F(ModuleScriptLoaderTest, FetchDataURL) {
   TestModuleScriptLoaderClient* client = new TestModuleScriptLoaderClient;
   TestFetchDataURL(ModuleScriptCustomFetchType::kNone, client);
 
-  // TODO(leszeks): This should finish synchronously, but currently due
-  // to the script resource/script streamer interaction, it does not.
-  RunUntilIdle();
-  EXPECT_TRUE(client->WasNotifyFinished());
+  EXPECT_TRUE(client->WasNotifyFinished())
+      << "ModuleScriptLoader should finish synchronously.";
   ASSERT_TRUE(client->GetModuleScript());
   EXPECT_FALSE(client->GetModuleScript()->HasEmptyRecord());
   EXPECT_FALSE(client->GetModuleScript()->HasParseError());
@@ -274,11 +272,8 @@ TEST_F(ModuleScriptLoaderTest, InvalidSpecifier) {
   TestModuleScriptLoaderClient* client = new TestModuleScriptLoaderClient;
   TestInvalidSpecifier(ModuleScriptCustomFetchType::kNone, client);
 
-  // TODO(leszeks): This should finish synchronously, but currently due
-  // to the script resource/script streamer interaction, it does not.
-  RunUntilIdle();
-  EXPECT_TRUE(client->WasNotifyFinished());
-
+  EXPECT_TRUE(client->WasNotifyFinished())
+      << "ModuleScriptLoader should finish synchronously.";
   ASSERT_TRUE(client->GetModuleScript());
   EXPECT_TRUE(client->GetModuleScript()->HasEmptyRecord());
   EXPECT_TRUE(client->GetModuleScript()->HasParseError());
@@ -318,10 +313,8 @@ TEST_F(ModuleScriptLoaderTest, FetchInvalidURL) {
   TestModuleScriptLoaderClient* client = new TestModuleScriptLoaderClient;
   TestFetchInvalidURL(ModuleScriptCustomFetchType::kNone, client);
 
-  // TODO(leszeks): This should finish synchronously, but currently due
-  // to the script resource/script streamer interaction, it does not.
-  RunUntilIdle();
-  EXPECT_TRUE(client->WasNotifyFinished());
+  EXPECT_TRUE(client->WasNotifyFinished())
+      << "ModuleScriptLoader should finish synchronously.";
   EXPECT_FALSE(client->GetModuleScript());
 }
 
@@ -362,9 +355,6 @@ TEST_F(ModuleScriptLoaderTest, FetchURL) {
   EXPECT_FALSE(client->WasNotifyFinished())
       << "ModuleScriptLoader unexpectedly finished synchronously.";
   platform_->GetURLLoaderMockFactory()->ServeAsynchronousRequests();
-  // TODO(leszeks): This should finish synchronously, but currently due
-  // to the script resource/script streamer interaction, it does not.
-  RunUntilIdle();
 
   EXPECT_TRUE(client->WasNotifyFinished());
   EXPECT_TRUE(client->GetModuleScript());

third_party/blink/renderer/core/loader/modulescript/worker_module_script_fetcher.cc

@@ -33,8 +33,7 @@ void WorkerModuleScriptFetcher::Fetch(FetchParameters& fetch_params,
   // Step 13.2. "Fetch request, and asynchronously wait to run the remaining
   // steps as part of fetch's process response for the response response." [spec
   // text]
-  ScriptResource::Fetch(fetch_params, global_scope_->EnsureFetcher(), this,
-                        ScriptResource::kNoStreaming);
+  ScriptResource::Fetch(fetch_params, global_scope_->EnsureFetcher(), this);
 }
 
 void WorkerModuleScriptFetcher::Trace(blink::Visitor* visitor) {

third_party/blink/renderer/core/loader/modulescript/worklet_module_script_fetcher.cc

@@ -38,8 +38,7 @@ void WorkletModuleScriptFetcher::Fetch(FetchParameters& fetch_params,
   // need to handle that case, maybe by having a way to restart fetches in a
   // different global scope?
   url_ = fetch_params.Url();
-  ScriptResource::Fetch(fetch_params, fetcher_.Get(), this,
-                        ScriptResource::kNoStreaming);
+  ScriptResource::Fetch(fetch_params, fetcher_.Get(), this);
 }
 
 void WorkletModuleScriptFetcher::NotifyFinished(Resource* resource) {

third_party/blink/renderer/core/loader/resource/script_resource.h

@@ -28,7 +28,6 @@
 
 #include <memory>
 
-#include "third_party/blink/renderer/bindings/core/v8/script_streamer.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/loader/resource/text_resource.h"
 #include "third_party/blink/renderer/platform/bindings/parkable_string.h"
@@ -44,34 +43,11 @@ class FetchParameters;
 class KURL;
 class ResourceFetcher;
 
-// ScriptResource is a resource representing a JavaScript script. It is only
-// used for "classic" scripts, i.e. not modules.
-//
-// In addition to loading the script, a ScriptResource can optionally stream the
-// script to the JavaScript parser/compiler, using a ScriptStreamer. In this
-// case, clients of the ScriptResource will not receive the finished
-// notification until the streaming completes.
-//
-// See also:
-// https://docs.google.com/document/d/143GOPl_XVgLPFfO-31b_MdBcnjklLEX2OIg_6eN6fQ4
 class CORE_EXPORT ScriptResource final : public TextResource {
  public:
-  // For scripts fetched with kAllowStreaming, the ScriptResource expects users
-  // to call StartStreaming to start streaming the loaded data, and
-  // SetClientIsWaitingForFinished when they actually want the data to be
-  // available for execute. Note that StartStreaming can fail, so the client of
-  // an unfinished resource has to call SetClientIsWaitingForFinished to
-  // guarantee that it receives a finished callback.
-  //
-  // Scripts fetched with kNoStreaming will (asynchronously) call
-  // SetClientIsWaitingForFinished on the resource, so the user does not have to
-  // call it again. This is effectively the "legacy" behaviour.
-  enum StreamingAllowed { kNoStreaming, kAllowStreaming };
-
   static ScriptResource* Fetch(FetchParameters&,
                                ResourceFetcher*,
-                               ResourceClient*,
-                               StreamingAllowed);
+                               ResourceClient*);
 
   // Public for testing
   static ScriptResource* CreateForTest(const KURL& url,
@@ -87,45 +63,12 @@ class CORE_EXPORT ScriptResource final : public TextResource {
 
   ~ScriptResource() override;
 
-  void Trace(blink::Visitor*) override;
-
   void OnMemoryDump(WebMemoryDumpLevelOfDetail,
                     WebProcessMemoryDump*) const override;
 
-  void SetSerializedCachedMetadata(const char*, size_t) override;
+  void DestroyDecodedDataForFailedRevalidation() override;
 
-  // Returns true if streaming was successfully started (or if an active
-  // streamer is already running)
-  //
-  // TODO(leszeks): This value is only used for work stealing, so make this
-  // function return void if work stealing is removed.
-  bool StartStreaming(
-      scoped_refptr<base::SingleThreadTaskRunner> loading_task_runner);
-
-  // State that a client of the script resource will no longer try to start
-  // streaming, and is now waiting for the resource to call the client's finish
-  // callback (regardless of whether the resource is finished loading or
-  // finished streaming). Specifically, it causes the kCanStartStreaming to
-  // kStreamingNotAllowed transition. Streaming cannot be started after this is
-  // called.
-  //
-  // If the resource is already streaming, this will be a no-op, and the client
-  // will still only get the finished notification when the streaming completes.
-  //
-  // This function should never be called synchronously (except by
-  // NotifyFinished) as it can trigger all clients' finished callbacks, which in
-  // turn can invoke JavaScript execution.
-  //
-  // TODO(leszeks): Eventually Fetch (with streaming allowed) will be the only
-  // way of starting streaming, and SetClientIsWaitingForFinished will not be
-  // part of the public interface.
-  void SetClientIsWaitingForFinished();
-
-  // Called (only) by ScriptStreamer when streaming completes.
-  //
-  // This function should never be called synchronously as it can trigger all
-  // clients' finished callbacks, which in turn can invoke JavaScript execution.
-  void StreamingFinished();
+  void SetSerializedCachedMetadata(const char*, size_t) override;
 
   const ParkableString& SourceText();
 
@@ -133,50 +76,11 @@ class CORE_EXPORT ScriptResource final : public TextResource {
 
   SingleCachedMetadataHandler* CacheHandler();
 
-  // Gets the script streamer from the ScriptResource, clearing the resource's
-  // streamer so that it cannot be used twice.
-  ScriptStreamer* TakeStreamer();
-
-  ScriptStreamer::NotStreamingReason NoStreamerReason() const {
-    return not_streaming_reason_;
-  }
-
-  // Used in DCHECKs
-  bool HasStreamer() { return !!streamer_; }
-  bool HasFinishedStreamer() { return streamer_ && streamer_->IsFinished(); }
-
-  // Visible for tests.
-  void SetRevalidatingRequest(const ResourceRequest&) override;
-
  protected:
   CachedMetadataHandler* CreateCachedMetadataHandler(
       std::unique_ptr<CachedMetadataSender> send_callback) override;
 
-  void DestroyDecodedDataForFailedRevalidation() override;
-
-  void NotifyDataReceived(const char* data, size_t size) override;
-
-  // ScriptResources are considered finished when either:
-  //   1. Loading + streaming completes, or
-  //   2. Loading completes + streaming was never started + someone called
-  //      "SetClientIsWaitingForFinished" to block streaming from ever starting.
-  void NotifyFinished() override;
-  bool IsFinishedInternal() const override;
-
  private:
-  // Valid state transitions:
-  //
-  // kCanStartStreaming -> kStreaming -> kWaitingForStreamingToEnd
-  //                                                -> kFinishedNotificationSent
-  // kCanStartStreaming -> kStreamingNotAllowed -> kFinishedNotificationSent
-  enum class StreamingState {
-    kCanStartStreaming,         // Streaming can be started.
-    kStreamingNotAllowed,       // Streaming can no longer be started.
-    kStreaming,                 // Both loading the resource and streaming.
-    kWaitingForStreamingToEnd,  // Resource loaded but streaming not complete.
-    kFinishedNotificationSent   // Everything complete and finish sent.
-  };
-
   class ScriptResourceFactory : public ResourceFactory {
    public:
     ScriptResourceFactory()
@@ -197,16 +101,7 @@ class CORE_EXPORT ScriptResource final : public TextResource {
 
   bool CanUseCacheValidator() const override;
 
-  void AdvanceStreamingState(StreamingState new_state);
-
-  // Check that invariants for the state hold.
-  void CheckStreamingState() const;
-
   ParkableString source_text_;
-  Member<ScriptStreamer> streamer_;
-  ScriptStreamer::NotStreamingReason not_streaming_reason_ =
-      ScriptStreamer::kDidntTryToStartStreaming;
-  StreamingState streaming_state_ = StreamingState::kCanStartStreaming;
 };
 
 DEFINE_RESOURCE_TYPE_CASTS(Script);

third_party/blink/renderer/core/script/classic_pending_script.h

@@ -59,8 +59,6 @@ class CORE_EXPORT ClassicPendingScript final : public PendingScript,
     return mojom::ScriptType::kClassic;
   }
 
-  void WatchForLoad(PendingScriptClient*) override;
-
   ClassicScript* GetSource(const KURL& document_url) const override;
   bool IsReady() const override;
   bool IsExternal() const override { return is_external_; }
@@ -77,10 +75,12 @@ class CORE_EXPORT ClassicPendingScript final : public PendingScript,
  private:
   // See AdvanceReadyState implementation for valid state transitions.
   enum ReadyState {
-    // This state is considered "not ready".
+    // These states are considered "not ready".
     kWaitingForResource,
+    kWaitingForStreaming,
     // These states are considered "ready".
     kReady,
+    kReadyStreaming,
     kErrorOccurred,
   };
 
@@ -95,11 +95,16 @@ class CORE_EXPORT ClassicPendingScript final : public PendingScript,
   // appropriate.
   void AdvanceReadyState(ReadyState);
 
+  // Handle the end of streaming.
+  void FinishWaitingForStreaming();
+  void FinishReadyStreaming();
+  void CancelStreaming();
   void CheckState() const override;
 
   // ResourceClient
   void NotifyFinished(Resource*) override;
   String DebugName() const override { return "PendingScript"; }
+  void DataReceived(Resource*, const char*, size_t) override;
 
   static void RecordStreamingHistogram(
       ScriptSchedulingType type,
@@ -129,6 +134,7 @@ class CORE_EXPORT ClassicPendingScript final : public PendingScript,
   // The request is intervened by document.write() intervention.
   bool intervened_ = false;
 
+  Member<ScriptStreamer> streamer_;
   base::OnceClosure streamer_done_;
 
   // This flag tracks whether streamer_ is currently streaming. It is used

third_party/blink/renderer/core/script/document_write_intervention.cc

@@ -219,8 +219,7 @@ void PossiblyFetchBlockedDocWriteScript(
       resource->Url(), element_document.GetSecurityOrigin(), cross_origin,
       resource->Encoding(), FetchParameters::kIdleLoad);
   AddHeader(&params);
-  ScriptResource::Fetch(params, element_document.Fetcher(), nullptr,
-                        ScriptResource::kNoStreaming);
+  ScriptResource::Fetch(params, element_document.Fetcher(), nullptr);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/script/pending_script.h

@@ -77,7 +77,7 @@ class CORE_EXPORT PendingScript
     return parser_blocking_load_start_time_;
   }
 
-  virtual void WatchForLoad(PendingScriptClient*);
+  void WatchForLoad(PendingScriptClient*);
   void StopWatchingForLoad();
   void PendingScriptFinished();
 

third_party/blink/renderer/platform/loader/fetch/resource.cc

@@ -272,7 +272,7 @@ void Resource::CheckResourceIntegrity() {
 }
 
 void Resource::NotifyFinished() {
-  CHECK(IsFinishedInternal());
+  DCHECK(IsLoaded());
 
   ResourceClientWalker<ResourceClient> w(clients_);
   while (ResourceClient* c = w.Next()) {
@@ -299,10 +299,6 @@ void Resource::AppendData(const char* data, size_t length) {
       data_ = SharedBuffer::Create(data, length);
     SetEncodedSize(data_->size());
   }
-  NotifyDataReceived(data, length);
-}
-
-void Resource::NotifyDataReceived(const char* data, size_t length) {
   ResourceClientWalker<ResourceClient> w(Clients());
   while (ResourceClient* c = w.Next())
     c->DataReceived(this, data, length);
@@ -622,7 +618,7 @@ void Resource::DidAddClient(ResourceClient* c) {
   }
   if (!HasClient(c))
     return;
-  if (IsFinishedInternal()) {
+  if (IsLoaded()) {
     c->NotifyFinished(this);
     if (clients_.Contains(c)) {
       finished_clients_.insert(c);
@@ -694,12 +690,6 @@ void Resource::AddFinishObserver(ResourceFinishObserver* client,
 
   WillAddClientOrObserver();
   finish_observers_.insert(client);
-  // Despite these being "Finish" observers, what they actually care about is
-  // whether the resource is "Loaded", not "Finished" (e.g. link onload). Hence
-  // we check IsLoaded directly here, rather than IsFinishedInternal.
-  //
-  // TODO(leszeks): Either rename FinishObservers to LoadedObservers, or the
-  // NotifyFinished method of ResourceClient to NotifyProcessed (or similar).
   if (IsLoaded())
     TriggerNotificationForFinishObservers(task_runner);
 }

third_party/blink/renderer/platform/loader/fetch/resource.h

@@ -431,19 +431,6 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
  protected:
   Resource(const ResourceRequest&, ResourceType, const ResourceLoaderOptions&);
 
-  // Returns true if the resource has finished any processing it wanted to do
-  // after loading. Should only be used to decide whether to call
-  // NotifyFinished.
-  //
-  // By default this is the same as being loaded (i.e. no processing), but it is
-  // used by ScriptResource to signal that streaming JavaScript compilation
-  // completed. Note that classes overloading this method should also overload
-  // NotifyFinished to not call Resource::NotifyFinished until this value
-  // becomes true.
-  // TODO(hiroshige): Remove this when ScriptResourceContent is introduced.
-  virtual bool IsFinishedInternal() const { return IsLoaded(); }
-
-  virtual void NotifyDataReceived(const char* data, size_t size);
   virtual void NotifyFinished();
 
   void MarkClientFinished(ResourceClient*);

tools/metrics/histograms/enums.xml

@@ -36483,7 +36483,7 @@ Called by update_net_trust_anchors.py.-->
 <enum name="NotStreamingReason">
   <int value="0" label="DEPRECATED: Already loaded"/>
   <int value="1" label="Not HTTP"/>
-  <int value="2" label="Revalidation"/>
+  <int value="2" label="Reload"/>
   <int value="3" label="Context not valid"/>
   <int value="4" label="Encoding not supported"/>
   <int value="5" label="Thread busy"/>