Reland "Add PerfectHeuristics UKM event, and metric for DelayAsyncScriptExecution"

This reverts commit f97fae25.

Reason for revert: Fixing the null dereference of Document's loader
which caused the crash in the first place. This crash was similar to
what we saw in crbug.com/931330 and fixed with crrev.com/c/1470543.

Original change's description:
> Revert "Add PerfectHeuristics UKM event, and metric for DelayAsyncScriptExecution"
> 
> This reverts commit 647d93e9.
> 
> Reason for revert: crbug/1116821 - causing crashes
> TBR=ryansturm@chromium.org,falken@chromium.org,rkaplow@chromium.org
> 
> Original change's description:
> > Add PerfectHeuristics UKM event, and metric for DelayAsyncScriptExecution
> >
> > This CL introduces the PerfectHeuristics UKM event, along with a boolean
> > metric for when a blink::ScriptRunner comes across an async script
> > before the script element's node document has finished parsing. The
> > PerfectHeuristics UKM event is logged in the UkmPageLoadMetricsObserver.
> >
> > See the Google-internal UKM privacy review:
> > https://docs.google.com/document/d/1UqkpGXc4LEh5jzzVR8fCVVTuCuG2mwvhQTuCy36zT3g/edit#
> >
> > See the Perfect Heuristics [1] project document (currently
> > Google-internal).
> >
> > [1]: https://docs.google.com/document/d/1Y7uF76Gq0VlJZky2j7x-lbD1_awH64eW0WRg1bNLuFs/edit
> >
> > R=​falken@chromium.org, nhiroki@chromium.org, ryansturm@chromium.org, sophiechang@chromium.org
> >
> > Bug: 1086227
> > Change-Id: I71954491a1a559c2e1244a543092f1b8f38b8136
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340845
> > Commit-Queue: Dominic Farolino <dom@chromium.org>
> > Reviewed-by: Sophie Chang <sophiechang@chromium.org>
> > Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> > Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> > Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
> > Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#798393}
> 
> TBR=falken@chromium.org,rkaplow@chromium.org,nhiroki@chromium.org,sophiechang@chromium.org,ryansturm@chromium.org,dom@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 1086227,1116821
> Change-Id: I0131884d123e2122fa674097381293174795fecf
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359573
> Commit-Queue: Sophie Chang <sophiechang@chromium.org>
> Reviewed-by: Sophie Chang <sophiechang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#798738}

TBR=falken@chromium.org,rkaplow@chromium.org,nhiroki@chromium.org,sophiechang@chromium.org,ryansturm@chromium.org,dom@chromium.org

# Not skipping CQ checks because this is a reland.

Bug: 1086227
Bug: 1116821
Change-Id: I7722da2c17dc34952ea9155d72840708b4eaa5b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359742Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798832}

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc

@@ -319,6 +319,7 @@ void UkmPageLoadMetricsObserver::OnComplete(
     RecordInputTimingMetrics();
   }
   ReportLayoutStability();
+  ReportPerfectHeuristicsMetrics();
   ReportAbortMetrics(timing, current_time);
 }
 
@@ -877,6 +878,15 @@ void UkmPageLoadMetricsObserver::ReportLayoutStability() {
   }
 }
 
+void UkmPageLoadMetricsObserver::ReportPerfectHeuristicsMetrics() {
+  ukm::builders::PerfectHeuristics builder(GetDelegate().GetPageUkmSourceId());
+  if (!delay_async_script_execution_before_finished_parsing_seen_)
+    return;
+
+  builder.Setdelay_async_script_execution_before_finished_parsing(1).Record(
+      ukm::UkmRecorder::Get());
+}
+
 void UkmPageLoadMetricsObserver::ReportAbortMetrics(
     const page_load_metrics::mojom::PageLoadTiming& timing,
     base::TimeTicks page_end_time) {
@@ -1144,4 +1154,10 @@ void UkmPageLoadMetricsObserver::OnLoadingBehaviorObserved(
                           kLoadingBehaviorFontPreloadStartedBeforeRendering) {
     font_preload_started_before_rendering_observed_ = true;
   }
+
+  if (behavior_flag &
+      blink::LoadingBehaviorFlag::
+          kLoadingBehaviorAsyncScriptReadyBeforeDocumentFinishedParsing) {
+    delay_async_script_execution_before_finished_parsing_seen_ = true;
+  }
 }

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.h

@@ -142,6 +142,8 @@ class UkmPageLoadMetricsObserver
 
   void ReportLayoutStability();
 
+  void ReportPerfectHeuristicsMetrics();
+
   void ReportAbortMetrics(
       const page_load_metrics::mojom::PageLoadTiming& timing,
       base::TimeTicks page_end_time);
@@ -248,7 +250,9 @@ class UkmPageLoadMetricsObserver
   // Unique across the lifetime of the browser process.
   int main_document_sequence_number_ = -1;
 
+  // These are to capture observed LoadingBehaviorFlags.
   bool font_preload_started_before_rendering_observed_ = false;
+  bool delay_async_script_execution_before_finished_parsing_seen_ = false;
 
   bool currently_in_foreground_ = false;
   // The last time the page became foregrounded, or navigation start if the page

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer_unittest.cc

@@ -1461,6 +1461,28 @@ TEST_F(UkmPageLoadMetricsObserverTest,
               testing::ElementsAre(base::Bucket(25, 1)));
 }
 
+TEST_F(UkmPageLoadMetricsObserverTest,
+       PerfectHeuristicsDelayaAsyncScriptExecution) {
+  NavigateAndCommit(GURL(kTestUrl1));
+
+  page_load_metrics::mojom::FrameMetadata metadata;
+  metadata.behavior_flags |= blink::LoadingBehaviorFlag::
+      kLoadingBehaviorAsyncScriptReadyBeforeDocumentFinishedParsing;
+  tester()->SimulateMetadataUpdate(metadata, web_contents()->GetMainFrame());
+
+  // Simulate closing the tab.
+  DeleteContents();
+
+  auto entries = tester()->test_ukm_recorder().GetEntriesByName(
+      ukm::builders::PerfectHeuristics::kEntryName);
+  EXPECT_EQ(1ul, entries.size());
+  tester()->test_ukm_recorder().ExpectEntryMetric(
+      entries.front(),
+      ukm::builders::PerfectHeuristics::
+          kdelay_async_script_execution_before_finished_parsingName,
+      1);
+}
+
 TEST_F(UkmPageLoadMetricsObserverTest, MHTMLNotTracked) {
   auto navigation = content::NavigationSimulator::CreateBrowserInitiated(
       GURL(kTestUrl1), web_contents());

third_party/blink/public/common/loader/loading_behavior_flag.h

@@ -44,6 +44,9 @@ enum LoadingBehaviorFlag {
   // Indicates that the page uses the Next.js JavaScript framework (via a
   // window variable)
   kLoadingBehaviorNextJSFrameworkUsed = 1 << 9,
+  // Indicates that an async script was ready to execute before the script
+  // element's node document has finished parsing.
+  kLoadingBehaviorAsyncScriptReadyBeforeDocumentFinishedParsing = 1 << 10,
 };
 
 }  // namespace blink

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

@@ -32,6 +32,7 @@
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
+#include "third_party/blink/renderer/core/loader/document_loader.h"
 #include "third_party/blink/renderer/core/script/script_loader.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
@@ -147,6 +148,12 @@ bool ScriptRunner::CanDelayAsyncScripts() {
           DelayAsyncScriptExecutionUntilFinishedParsingEnabled() ||
       RuntimeEnabledFeatures::
           DelayAsyncScriptExecutionUntilFirstPaintOrFinishedParsingEnabled();
+  // The document's loader can be null here, e.g., if the frame is being
+  // detached.
+  if (!document_->Parsing() && document_->Loader()) {
+    document_->Loader()->DidObserveLoadingBehavior(
+        kLoadingBehaviorAsyncScriptReadyBeforeDocumentFinishedParsing);
+  }
   return !delay_async_script_milestone_reached_ && flags_enabled;
 }
 

tools/metrics/ukm/ukm.xml

@@ -9606,6 +9606,21 @@ be describing additional metrics about the same event.
   </summary>
 </event>
 
+<event name="PerfectHeuristics" singular="True">
+  <owner>dom@chromium.org</owner>
+  <owner>chrome-loading@google.com</owner>
+  <summary>
+    Metrics recorded for the various Perfect Heuristics experiments being run.
+  </summary>
+  <metric name="delay_async_script_execution_before_finished_parsing"
+      enum="Boolean">
+    <summary>
+      Set to 1 when blink::ScriptRunner sees an async script before the
+      associated document has finished parsing.
+    </summary>
+  </metric>
+</event>
+
 <event name="PeriodicBackgroundSyncEventCompleted">
   <owner>nator@chromium.org</owner>
   <owner>rayankans@chromium.org</owner>