Prerender: report a detailed kRendererProcessKilled status

2% prerenders are cancelled due to this reason and it is hard to understand what happened. This CL makes chromium track the termination status. Bug: 1407056 Change-Id: I252a605f635bb31473479a797153b4caa0ae23ab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4196677 Reviewed-by: Bo Liu <boliu@chromium.org> Commit-Queue: Lingqi Chi <lingqi@chromium.org> Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Cr-Commit-Position: refs/heads/main@{#1098442}
chromium · Jan 30, 2023 · 1961141 · 1961141
1 parent 185d33a
commit 1961141
Show file tree

Hide file tree

Showing 7 changed files with 152 additions and 11 deletions.
diff --git a/content/browser/preloading/prerender/prerender_browsertest.cc b/content/browser/preloading/prerender/prerender_browsertest.cc
@@ -6328,12 +6328,21 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, AbandonIfRendererProcessCrashes) {
     host_observer.WaitForDestroyed();
   }
 
-  ExpectFinalStatusForSpeculationRule(
 #if BUILDFLAG(IS_ANDROID)
+  ExpectFinalStatusForSpeculationRule(
       PrerenderFinalStatus::kRendererProcessKilled);
+  histogram_tester().ExpectUniqueSample(
+      "Prerender.Experimental.KilledPrerenderProcessTerminationStatus."
+      "SpeculationRule",
+      PrerenderProcessTerminationStatus::kOomProtected, 1);
 #else
+  ExpectFinalStatusForSpeculationRule(
       PrerenderFinalStatus::kRendererProcessCrashed);
-#endif  // BUILDFLAG(IS_ANDROID)
+  histogram_tester().ExpectTotalCount(
+      "Prerender.Experimental.KilledPrerenderProcessTerminationStatus."
+      "SpeculationRule",
+      0);
+#endif
 }
 
 // Test if the host is abandoned when the renderer page is killed.
@@ -6359,6 +6368,18 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, AbandonIfRendererProcessIsKilled) {
 
   ExpectFinalStatusForSpeculationRule(
       PrerenderFinalStatus::kRendererProcessKilled);
+  PrerenderProcessTerminationStatus expected_termination_status =
+#if BUILDFLAG(IS_ANDROID)
+      PrerenderProcessTerminationStatus::kOomProtected;
+#elif BUILDFLAG(IS_WIN)
+      PrerenderProcessTerminationStatus::kNormalTermination;
+#else
+      PrerenderProcessTerminationStatus::kProcessWasKilled;
+#endif
+  histogram_tester().ExpectUniqueSample(
+      "Prerender.Experimental.KilledPrerenderProcessTerminationStatus."
+      "SpeculationRule",
+      expected_termination_status, 1);
 }
 
 // Test if the host is abandoned when the primary main page that triggers a

diff --git a/content/browser/preloading/prerender/prerender_metrics.cc b/content/browser/preloading/prerender/prerender_metrics.cc
@@ -7,8 +7,11 @@
 #include "base/containers/flat_map.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/metrics_hashes.h"
+#include "base/notreached.h"
+#include "base/process/kill.h"
 #include "base/strings/string_util.h"
 #include "content/browser/devtools/devtools_instrumentation.h"
+#include "content/browser/preloading/prerender/prerender_final_status.h"
 #include "content/browser/renderer_host/frame_tree_node.h"
 #include "content/public/browser/prerender_trigger_type.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
@@ -28,6 +31,43 @@ enum HeaderMismatchType : uint32_t {
   kMaxValue = kValueMismatch
 };
 
+PrerenderProcessTerminationStatus TranslateToPrerenderUmaTerminationStatus(
+    base::TerminationStatus status) {
+  switch (status) {
+    case base::TerminationStatus::TERMINATION_STATUS_NORMAL_TERMINATION:
+      return PrerenderProcessTerminationStatus::kNormalTermination;
+    case base::TerminationStatus::TERMINATION_STATUS_ABNORMAL_TERMINATION:
+      return PrerenderProcessTerminationStatus::kAbnormalTermination;
+    case base::TerminationStatus::TERMINATION_STATUS_PROCESS_WAS_KILLED:
+      return PrerenderProcessTerminationStatus::kProcessWasKilled;
+    case base::TerminationStatus::TERMINATION_STATUS_PROCESS_CRASHED:
+      return PrerenderProcessTerminationStatus::kProcessCrashed;
+    case base::TerminationStatus::TERMINATION_STATUS_STILL_RUNNING:
+      return PrerenderProcessTerminationStatus::kStillRunning;
+#if BUILDFLAG(IS_CHROMEOS)
+    case base::TerminationStatus::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM:
+      return PrerenderProcessTerminationStatus::kProcessWasKilledByOom;
+#endif  // BUILDFLAG(IS_CHROMEOS)
+#if BUILDFLAG(IS_ANDROID)
+    case base::TerminationStatus::TERMINATION_STATUS_OOM_PROTECTED:
+      return PrerenderProcessTerminationStatus::kOomProtected;
+#endif  // BUILDFLAG(IS_ANDROID)
+    case base::TerminationStatus::TERMINATION_STATUS_LAUNCH_FAILED:
+      return PrerenderProcessTerminationStatus::kLaunchFailed;
+    case base::TerminationStatus::TERMINATION_STATUS_OOM:
+      return PrerenderProcessTerminationStatus::kOom;
+#if BUILDFLAG(IS_WIN)
+    case base::TerminationStatus::TERMINATION_STATUS_INTEGRITY_FAILURE:
+      return PrerenderProcessTerminationStatus::kIntegrityFailure;
+#endif  //  BUILDFLAG(IS_WIN)
+    case base::TerminationStatus::TERMINATION_STATUS_MAX_ENUM:
+      NOTREACHED();
+      return PrerenderProcessTerminationStatus::kInvalid;
+  }
+  NOTREACHED();
+  return PrerenderProcessTerminationStatus::kInvalid;
+}
+
 PrerenderCancelledInterface GetCancelledInterfaceType(
     const std::string& interface_name) {
   if (interface_name == "device.mojom.GamepadHapticsManager")
@@ -103,6 +143,17 @@ void RecordPrerenderCancelledInterface(
   }
 }
 
+void RecordRendererProcessKilledTerminationStatus(
+    base::TerminationStatus status_code,
+    PrerenderTriggerType trigger_type,
+    const std::string& embedder_histogram_suffix) {
+  base::UmaHistogramEnumeration(
+      GenerateHistogramName(
+          "Prerender.Experimental.KilledPrerenderProcessTerminationStatus",
+          trigger_type, embedder_histogram_suffix),
+      TranslateToPrerenderUmaTerminationStatus(status_code));
+}
+
 void RecordPrerenderFinalStatusUma(
     PrerenderFinalStatus final_status,
     PrerenderTriggerType trigger_type,
@@ -132,6 +183,18 @@ PrerenderCancellationReason::BuildForMojoBinderPolicy(
                                      interface_name);
 }
 
+// static
+PrerenderCancellationReason
+PrerenderCancellationReason::BuildForRendererProcessGone(
+    base::TerminationStatus status_code) {
+  if (status_code == base::TERMINATION_STATUS_PROCESS_CRASHED) {
+    return PrerenderCancellationReason(
+        PrerenderFinalStatus::kRendererProcessCrashed);
+  }
+  return PrerenderCancellationReason(
+      PrerenderFinalStatus::kRendererProcessKilled, status_code);
+}
+
 PrerenderCancellationReason::PrerenderCancellationReason(
     PrerenderFinalStatus final_status)
     : PrerenderCancellationReason(final_status, DetailedReasonVariant()) {}
@@ -165,6 +228,12 @@ void PrerenderCancellationReason::ReportMetrics(
                                         trigger_type,
                                         embedder_histogram_suffix);
       break;
+    case PrerenderFinalStatus::kRendererProcessKilled:
+      DCHECK(absl::holds_alternative<base::TerminationStatus>(explanation_));
+      RecordRendererProcessKilledTerminationStatus(
+          absl::get<base::TerminationStatus>(explanation_), trigger_type,
+          embedder_histogram_suffix);
+      break;
     default:
       DCHECK(absl::holds_alternative<absl::monostate>(explanation_));
       // Other types need not to report.
@@ -180,6 +249,11 @@ std::string PrerenderCancellationReason::ToDevtoolReasonString() const {
       // And devtool has to handle it based on the enum.xml, as the content
       // layer cannot know about the enums added by the embedder layer.
       return "";
+    case PrerenderFinalStatus::kRendererProcessKilled:
+      DCHECK(absl::holds_alternative<base::TerminationStatus>(explanation_));
+      // We do not have a plan to send the detailed crash reason to devtools
+      // yet.
+      return "";
     case PrerenderFinalStatus::kMojoBinderPolicy:
       DCHECK(absl::holds_alternative<std::string>(explanation_));
       return absl::get<std::string>(explanation_);

diff --git a/content/browser/preloading/prerender/prerender_metrics.h b/content/browser/preloading/prerender/prerender_metrics.h
@@ -47,20 +47,42 @@ enum class PrerenderCrossOriginRedirectionMismatch {
   kMaxValue = kSchemeHostPortMismatch
 };
 
+// Use this enum instead of base::TerminationStatus so that all numbers can be
+// mapped to their dedicated labels, regardless of the platform.
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class PrerenderProcessTerminationStatus {
+  kNormalTermination = 0,
+  kAbnormalTermination = 1,
+  kProcessWasKilled = 2,
+  kProcessCrashed = 3,
+  kStillRunning = 4,
+  kProcessWasKilledByOom = 5,
+  kOomProtected = 6,
+  kLaunchFailed = 7,
+  kOom = 8,
+  kIntegrityFailure = 9,
+  kInvalid = 10,
+  kMaxValue = kInvalid,
+};
+
 // Assembles PrerenderHostFinalStatus with a detailed explanation if applicable.
 // Some FinalStatus enums cover multiple sub cases. To explain them in detail,
 // some explanations can be attached to the status.
 class PrerenderCancellationReason {
  public:
-  using DetailedReasonVariant =
-      absl::variant<absl::monostate, uint64_t, std::string>;
+  using DetailedReasonVariant = absl::
+      variant<absl::monostate, uint64_t, base::TerminationStatus, std::string>;
 
   static PrerenderCancellationReason BuildForDisallowActivationState(
       uint64_t disallow_activation_reason);
 
   static PrerenderCancellationReason BuildForMojoBinderPolicy(
       const std::string& interface_name);
 
+  static PrerenderCancellationReason BuildForRendererProcessGone(
+      base::TerminationStatus status_code);
+
   explicit PrerenderCancellationReason(PrerenderFinalStatus final_status);
   ~PrerenderCancellationReason();
 

diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -3051,10 +3051,8 @@ void RenderFrameHostImpl::RenderProcessGone(
                   kRendererProcessKilled);
   }
 
-  CancelPrerendering(PrerenderCancellationReason(
-      info.status == base::TERMINATION_STATUS_PROCESS_CRASHED
-          ? PrerenderFinalStatus::kRendererProcessCrashed
-          : PrerenderFinalStatus::kRendererProcessKilled));
+  CancelPrerendering(
+      PrerenderCancellationReason::BuildForRendererProcessGone(info.status));
 
   if (owned_render_widget_host_)
     owned_render_widget_host_->RendererExited();

diff --git a/content/browser/screen_orientation/screen_orientation_browsertest.cc b/content/browser/screen_orientation/screen_orientation_browsertest.cc
@@ -533,9 +533,7 @@ IN_PROC_BROWSER_TEST_F(ScreenOrientationLockForPrerenderBrowserTest,
 
   // Shut down the prerendered page. It shouldn't trigger orientation unlock.
   test::PrerenderHostObserver prerender_observer(*web_contents(), host_id);
-  PrerenderHostRegistry* registry =
-      static_cast<WebContentsImpl*>(web_contents())->GetPrerenderHostRegistry();
-  registry->CancelHost(host_id, PrerenderFinalStatus::kRendererProcessKilled);
+  prerender_helper_.CancelPrerenderedPage(host_id);
   prerender_observer.WaitForDestroyed();
 
   // Delegate should not apply unlock.

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -83788,6 +83788,20 @@ chromeos/ash/components/peripheral_notification/peripheral_notification_manager.
   <int value="3" label="Prerender hit finished"/>
 </enum>
 
+<enum name="PrerenderProcessTerminationStatus">
+  <int value="0" label="kNormalTermination"/>
+  <int value="1" label="kAbnormalTermination"/>
+  <int value="2" label="kProcessWasKilled"/>
+  <int value="3" label="kProcessCrashed"/>
+  <int value="4" label="kStillRunning"/>
+  <int value="5" label="kProcessWasKilledByOom"/>
+  <int value="6" label="kOomProtected"/>
+  <int value="7" label="kLaunchFailed"/>
+  <int value="8" label="kOom"/>
+  <int value="9" label="kIntegrityFailure"/>
+  <int value="10" label="kInvalid"/>
+</enum>
+
 <enum name="PrerenderRelTypes">
   <int value="0" label="PRERENDER_REL_TYPE_NONE"/>
   <int value="1" label="PRERENDER_REL_TYPE_PRERENDER"/>

diff --git a/tools/metrics/histograms/metadata/navigation/histograms.xml b/tools/metrics/histograms/metadata/navigation/histograms.xml
@@ -1653,6 +1653,20 @@ Also used in tools/metrics/histograms/metadata/page/histograms.xml.
   </summary>
 </histogram>
 
+<histogram
+    name="Prerender.Experimental.KilledPrerenderProcessTerminationStatus{PrerenderTriggerType}"
+    enum="PrerenderProcessTerminationStatus" expires_after="2023-06-05">
+  <owner>lingqi@chromium.org</owner>
+  <owner>nhiroki@chromium.org</owner>
+  <summary>
+    This is a breakdown metric for
+    PrerenderHostFinalStatus::kRendererProcessKilled. Recorded by
+    PrerenderCancellationReason to track the detailed termination status when a
+    prerender is cancelled with this final status.
+  </summary>
+  <token key="PrerenderTriggerType" variants="PrerenderTriggerType"/>
+</histogram>
+
 <histogram name="Prerender.Experimental.PredictionStatus.DefaultSearchEngine"
     enum="PrerenderPredictionStatus" expires_after="2023-06-18">
   <owner>asamidoi@chromium.org</owner>