[ScriptStreamer] Propagate NotStreamingReason to v8.compile trace event.

Enables tracking of why a particular script was not parsed / compiled by the ScriptStreamer. BUG=865098 Change-Id: I1a1b033689d0f12df4f28ff5c4bb21243cbec197 Reviewed-on: https://chromium-review.googlesource.com/1143762 Commit-Queue: Ross McIlroy <rmcilroy@chromium.org> Reviewed-by: Marja Hölttä <marja@chromium.org> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#577808}
qweri0p · Jul 25, 2018 · 0d2aca0 · 0d2aca0
1 parent 4ea122b
commit 0d2aca0
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 15 deletions.
diff --git a/...bKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt b/...bKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt
@@ -6,6 +6,7 @@ v8.compile Properties:
     data : {
         columnNumber : 0
         lineNumber : 0
+        notStreamedReason : "inline script"
         streamed : <boolean>
         url : .../devtools/service-workers/resources/v8-cache-worker.js
     }
@@ -20,6 +21,7 @@ v8.compile Properties:
         cacheProduceOptions : "full code"
         columnNumber : 0
         lineNumber : 0
+        notStreamedReason : "script has code-cache available"
         producedCacheSize : <number>
         streamed : <boolean>
         url : .../devtools/resources/v8-cache-script.js
@@ -39,6 +41,7 @@ v8.compile Properties:
         columnNumber : 0
         consumedCacheSize : <number>
         lineNumber : 0
+        notStreamedReason : "script too small"
         streamed : <boolean>
         url : .../devtools/resources/v8-cache-script.js
     }
@@ -55,6 +58,7 @@ v8.compile Properties:
         columnNumber : 0
         consumedCacheSize : <number>
         lineNumber : 0
+        notStreamedReason : "resource no longer alive"
         streamed : <boolean>
         url : .../devtools/resources/v8-cache-script.js
     }

diff --git a/...ty/WebKit/LayoutTests/http/tests/devtools/tracing/timeline-js/compile-script-expected.txt b/...ty/WebKit/LayoutTests/http/tests/devtools/tracing/timeline-js/compile-script-expected.txt
@@ -5,6 +5,7 @@ v8.compile Properties:
     data : {
         columnNumber : 0
         lineNumber : 0
+        notStreamedReason : "inline script"
         streamed : <boolean>
         url : 
     }
@@ -18,6 +19,7 @@ v8.compile Properties:
     data : {
         columnNumber : 0
         lineNumber : 0
+        notStreamedReason : "script too small"
         streamed : <boolean>
         url : .../devtools/tracing/timeline-js/resources/timeline-script-tag-2.js
     }

diff --git a/third_party/blink/renderer/bindings/core/v8/script_source_code.cc b/third_party/blink/renderer/bindings/core/v8/script_source_code.cc
@@ -56,6 +56,7 @@ ScriptSourceCode::ScriptSourceCode(
     const TextPosition& start_position)
     : source_(TreatNullSourceAsEmpty(source)),
       cache_handler_(cache_handler),
+      not_streaming_reason_(ScriptStreamer::kInlineScript),
       url_(StripFragmentIdentifier(url)),
       start_position_(start_position),
       source_location_type_(source_location_type) {
@@ -76,14 +77,18 @@ ScriptSourceCode::ScriptSourceCode(
                        start_position) {}
 
 ScriptSourceCode::ScriptSourceCode(ScriptStreamer* streamer,
-                                   ScriptResource* resource)
+                                   ScriptResource* resource,
+                                   ScriptStreamer::NotStreamingReason reason)
     : source_(TreatNullSourceAsEmpty(resource->SourceText())),
       cache_handler_(resource->CacheHandler()),
       streamer_(streamer),
+      not_streaming_reason_(reason),
       url_(StripFragmentIdentifier(resource->GetResponse().Url())),
       source_map_url_(SourceMapUrlFromResponse(resource->GetResponse())),
       start_position_(TextPosition::MinimumPosition()),
-      source_location_type_(ScriptSourceLocationType::kExternalFile) {}
+      source_location_type_(ScriptSourceLocationType::kExternalFile) {
+  DCHECK_EQ(!streamer, reason != ScriptStreamer::NotStreamingReason::kInvalid);
+}
 
 ScriptSourceCode::~ScriptSourceCode() = default;
 

diff --git a/third_party/blink/renderer/bindings/core/v8/script_source_code.h b/third_party/blink/renderer/bindings/core/v8/script_source_code.h
@@ -68,7 +68,9 @@ class CORE_EXPORT ScriptSourceCode final {
   //
   // We lose the encoding information from ScriptResource.
   // Not sure if that matters.
-  ScriptSourceCode(ScriptStreamer*, ScriptResource*);
+  ScriptSourceCode(ScriptStreamer*,
+                   ScriptResource*,
+                   ScriptStreamer::NotStreamingReason);
 
   ~ScriptSourceCode();
   void Trace(blink::Visitor*);
@@ -83,11 +85,15 @@ class CORE_EXPORT ScriptSourceCode final {
   const String& SourceMapUrl() const { return source_map_url_; }
 
   ScriptStreamer* Streamer() const { return streamer_; }
+  ScriptStreamer::NotStreamingReason NotStreamingReason() const {
+    return not_streaming_reason_;
+  }
 
  private:
   const MovableString source_;
   Member<SingleCachedMetadataHandler> cache_handler_;
   Member<ScriptStreamer> streamer_;
+  ScriptStreamer::NotStreamingReason not_streaming_reason_;
 
   // The URL of the source code, which is primarily intended for DevTools
   // javascript debugger.

diff --git a/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc b/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc
@@ -225,7 +225,7 @@ void V8CodeCache::ProduceCache(
                       compile_options, cached_data ? cached_data->length : 0),
                   base::Optional<InspectorCompileScriptEvent::V8CacheResult::
                                      ConsumeResult>()),
-              source.Streamer()));
+              source.Streamer(), source.NotStreamingReason()));
       break;
     }
     case ProduceCacheOptions::kNoProduceCache:
@@ -314,7 +314,7 @@ scoped_refptr<CachedMetadata> V8CodeCache::GenerateFullCodeCache(
                   cached_data ? cached_data->length : 0),
               base::Optional<
                   InspectorCompileScriptEvent::V8CacheResult::ConsumeResult>()),
-          false));
+          false, ScriptStreamer::kHasCodeCache));
 
   return cached_metadata;
 }

diff --git a/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc b/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc
@@ -215,10 +215,10 @@ v8::MaybeLocal<v8::Script> V8ScriptRunner::CompileScript(
   v8::MaybeLocal<v8::Script> script =
       CompileScriptInternal(isolate, execution_context, source, origin,
                             compile_options, no_cache_reason, &cache_result);
-  TRACE_EVENT_END1(
-      kTraceEventCategoryGroup, "v8.compile", "data",
-      InspectorCompileScriptEvent::Data(file_name, script_start_position,
-                                        cache_result, source.Streamer()));
+  TRACE_EVENT_END1(kTraceEventCategoryGroup, "v8.compile", "data",
+                   InspectorCompileScriptEvent::Data(
+                       file_name, script_start_position, cache_result,
+                       source.Streamer(), source.NotStreamingReason()));
   return script;
 }
 

diff --git a/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc b/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc
@@ -130,7 +130,8 @@ TEST_F(V8ScriptRunnerTest, emptyResourceDoesNotHaveCacheHandler) {
 
 TEST_F(V8ScriptRunnerTest, codeOption) {
   V8TestingScope scope;
-  ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()));
+  ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()),
+                               ScriptStreamer::kScriptTooSmall);
   SingleCachedMetadataHandler* cache_handler = source_code.CacheHandler();
   SetCacheTimeStamp(cache_handler);
 
@@ -147,7 +148,8 @@ TEST_F(V8ScriptRunnerTest, codeOption) {
 
 TEST_F(V8ScriptRunnerTest, consumeCodeOption) {
   V8TestingScope scope;
-  ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()));
+  ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()),
+                               ScriptStreamer::kScriptTooSmall);
   // Set timestamp to simulate a warm run.
   SingleCachedMetadataHandler* cache_handler = source_code.CacheHandler();
   SetCacheTimeStamp(cache_handler);
@@ -177,7 +179,8 @@ TEST_F(V8ScriptRunnerTest, consumeCodeOption) {
 
 TEST_F(V8ScriptRunnerTest, produceAndConsumeCodeOption) {
   V8TestingScope scope;
-  ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()));
+  ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()),
+                               ScriptStreamer::kScriptTooSmall);
   SingleCachedMetadataHandler* cache_handler = source_code.CacheHandler();
 
   // Cold run - should set the timestamp

diff --git a/third_party/blink/renderer/core/inspector/inspector_trace_events.cc b/third_party/blink/renderer/core/inspector/inspector_trace_events.cc
@@ -358,6 +358,41 @@ const char* CompileOptionsString(v8::ScriptCompiler::CompileOptions options) {
   return "";
 }
 
+const char* NotStreamedReasonString(ScriptStreamer::NotStreamingReason reason) {
+  switch (reason) {
+    case ScriptStreamer::kNotHTTP:
+      return "not http/https protocol";
+    case ScriptStreamer::kReload:
+      return "reload event";
+    case ScriptStreamer::kContextNotValid:
+      return "script context not valid";
+    case ScriptStreamer::kEncodingNotSupported:
+      return "encoding not supported";
+    case ScriptStreamer::kThreadBusy:
+      return "script streamer thread busy";
+    case ScriptStreamer::kV8CannotStream:
+      return "V8 cannot stream script";
+    case ScriptStreamer::kScriptTooSmall:
+      return "script too small";
+    case ScriptStreamer::kNoResourceBuffer:
+      return "resource no longer alive";
+    case ScriptStreamer::kHasCodeCache:
+      return "script has code-cache available";
+    case ScriptStreamer::kStreamerNotReadyOnGetSource:
+      return "streamer not ready";
+    case ScriptStreamer::kInlineScript:
+      return "inline script";
+    case ScriptStreamer::kDidntTryToStartStreaming:
+      return "start streaming not called";
+    case ScriptStreamer::kAlreadyLoaded:
+    case ScriptStreamer::kCount:
+    case ScriptStreamer::kInvalid:
+      NOTREACHED();
+  }
+  NOTREACHED();
+  return "";
+}
+
 }  // namespace
 
 namespace InspectorScheduleStyleInvalidationTrackingEvent {
@@ -1105,7 +1140,8 @@ std::unique_ptr<TracedValue> InspectorCompileScriptEvent::Data(
     const String& url,
     const TextPosition& text_position,
     const V8CacheResult& cache_result,
-    bool streamed) {
+    bool streamed,
+    ScriptStreamer::NotStreamingReason not_streaming_reason) {
   std::unique_ptr<TracedValue> value = FillLocation(url, text_position);
 
   if (cache_result.produce_result) {
@@ -1125,6 +1161,10 @@ std::unique_ptr<TracedValue> InspectorCompileScriptEvent::Data(
     value->SetBoolean("cacheRejected", cache_result.consume_result->rejected);
   }
   value->SetBoolean("streamed", streamed);
+  if (!streamed) {
+    value->SetString("notStreamedReason",
+                     NotStreamedReasonString(not_streaming_reason));
+  }
   return value;
 }
 

diff --git a/third_party/blink/renderer/core/inspector/inspector_trace_events.h b/third_party/blink/renderer/core/inspector/inspector_trace_events.h
@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
+#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/css/css_selector.h"
 #include "third_party/blink/renderer/core/loader/frame_loader_types.h"
@@ -425,7 +426,8 @@ struct V8CacheResult {
 std::unique_ptr<TracedValue> Data(const String& url,
                                   const WTF::TextPosition&,
                                   const V8CacheResult&,
-                                  bool streamed);
+                                  bool streamed,
+                                  ScriptStreamer::NotStreamingReason);
 }  // namespace InspectorCompileScriptEvent
 
 namespace InspectorFunctionCallEvent {

diff --git a/third_party/blink/renderer/core/script/classic_pending_script.cc b/third_party/blink/renderer/core/script/classic_pending_script.cc
@@ -391,7 +391,8 @@ ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url,
   RecordStreamingHistogram(GetSchedulingType(), streamer_ready,
                            not_streamed_reason);
 
-  ScriptSourceCode source_code(streamer_ready ? streamer_ : nullptr, resource);
+  ScriptSourceCode source_code(streamer_ready ? streamer_ : nullptr, resource,
+                               not_streamed_reason);
   // The base URL for external classic script is
   // "the URL from which the script was obtained" [spec text]
   // https://html.spec.whatwg.org/multipage/webappapis.html#concept-script-base-url