android: Add reverse rank uma

Record the reverse rank of android killed child processes.

Bug: 946216
Change-Id: I844bed1b8219b711a301952a808160f61ffab1ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1603056
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659781}

chrome/browser/metrics/chrome_stability_metrics_provider_unittest.cc

@@ -63,8 +63,10 @@ TEST_F(ChromeStabilityMetricsProviderTest, BrowserChildProcessObserverGpu) {
   child_process_data.metrics_name = kTestGpuProcessName;
 
   provider.BrowserChildProcessLaunchedAndConnected(child_process_data);
-  content::ChildProcessTerminationInfo abnormal_termination_info{
-      base::TERMINATION_STATUS_ABNORMAL_TERMINATION, 1};
+  content::ChildProcessTerminationInfo abnormal_termination_info;
+  abnormal_termination_info.status =
+      base::TERMINATION_STATUS_ABNORMAL_TERMINATION;
+  abnormal_termination_info.exit_code = 1;
   provider.BrowserChildProcessCrashed(child_process_data,
                                       abnormal_termination_info);
   provider.BrowserChildProcessCrashed(child_process_data,
@@ -94,8 +96,10 @@ TEST_F(ChromeStabilityMetricsProviderTest, BrowserChildProcessObserverUtility) {
 
   provider.BrowserChildProcessLaunchedAndConnected(child_process_data);
   const int kExitCode = 1;
-  content::ChildProcessTerminationInfo abnormal_termination_info{
-      base::TERMINATION_STATUS_ABNORMAL_TERMINATION, kExitCode};
+  content::ChildProcessTerminationInfo abnormal_termination_info;
+  abnormal_termination_info.status =
+      base::TERMINATION_STATUS_ABNORMAL_TERMINATION;
+  abnormal_termination_info.exit_code = kExitCode;
   provider.BrowserChildProcessCrashed(child_process_data,
                                       abnormal_termination_info);
   provider.BrowserChildProcessCrashed(child_process_data,
@@ -146,31 +150,35 @@ TEST_F(ChromeStabilityMetricsProviderTest, NotificationObserver) {
       rph_factory->CreateRenderProcessHost(profile, site_instance.get()));
 
   // Crash and abnormal termination should increment renderer crash count.
-  content::ChildProcessTerminationInfo crash_details{
-      base::TERMINATION_STATUS_PROCESS_CRASHED, 1};
+  content::ChildProcessTerminationInfo crash_details;
+  crash_details.status = base::TERMINATION_STATUS_PROCESS_CRASHED;
+  crash_details.exit_code = 1;
   provider.Observe(
       content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
       content::Source<content::RenderProcessHost>(host),
       content::Details<content::ChildProcessTerminationInfo>(&crash_details));
 
-  content::ChildProcessTerminationInfo term_details{
-      base::TERMINATION_STATUS_ABNORMAL_TERMINATION, 1};
+  content::ChildProcessTerminationInfo term_details;
+  term_details.status = base::TERMINATION_STATUS_ABNORMAL_TERMINATION;
+  term_details.exit_code = 1;
   provider.Observe(
       content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
       content::Source<content::RenderProcessHost>(host),
       content::Details<content::ChildProcessTerminationInfo>(&term_details));
 
   // Kill does not increment renderer crash count.
-  content::ChildProcessTerminationInfo kill_details{
-      base::TERMINATION_STATUS_PROCESS_WAS_KILLED, 1};
+  content::ChildProcessTerminationInfo kill_details;
+  kill_details.status = base::TERMINATION_STATUS_PROCESS_WAS_KILLED;
+  kill_details.exit_code = 1;
   provider.Observe(
       content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
       content::Source<content::RenderProcessHost>(host),
       content::Details<content::ChildProcessTerminationInfo>(&kill_details));
 
   // Failed launch increments failed launch count.
-  content::ChildProcessTerminationInfo failed_launch_details{
-      base::TERMINATION_STATUS_LAUNCH_FAILED, 1};
+  content::ChildProcessTerminationInfo failed_launch_details;
+  failed_launch_details.status = base::TERMINATION_STATUS_LAUNCH_FAILED;
+  failed_launch_details.exit_code = 1;
   provider.Observe(content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
                    content::Source<content::RenderProcessHost>(host),
                    content::Details<content::ChildProcessTerminationInfo>(

chrome/browser/metrics/plugin_metrics_provider_unittest.cc

@@ -168,8 +168,10 @@ TEST_F(PluginMetricsProviderTest, ProvideStabilityMetricsWhenPendingTask) {
 
   // Increase number of process launches which should also start a delayed
   // task.
-  content::ChildProcessTerminationInfo abnormal_termination_info{
-      base::TERMINATION_STATUS_ABNORMAL_TERMINATION, 1};
+  content::ChildProcessTerminationInfo abnormal_termination_info;
+  abnormal_termination_info.status =
+      base::TERMINATION_STATUS_ABNORMAL_TERMINATION;
+  abnormal_termination_info.exit_code = 1;
   content::ChildProcessData child_process_data1(
       content::PROCESS_TYPE_PPAPI_PLUGIN);
   child_process_data1.name = base::UTF8ToUTF16("p1");

components/crash/content/browser/child_exit_observer_android.cc

@@ -38,6 +38,7 @@ void PopulateTerminationInfo(
       content_info.remaining_process_with_moderate_binding;
   info->remaining_process_with_waived_binding =
       content_info.remaining_process_with_waived_binding;
+  info->best_effort_reverse_rank = content_info.best_effort_reverse_rank;
   info->was_oom_protected_status =
       content_info.status == base::TERMINATION_STATUS_OOM_PROTECTED;
 }

components/crash/content/browser/child_exit_observer_android.h

@@ -70,6 +70,7 @@ class ChildExitObserver : public content::BrowserChildProcessObserver,
     int remaining_process_with_strong_binding = 0;
     int remaining_process_with_moderate_binding = 0;
     int remaining_process_with_waived_binding = 0;
+    int best_effort_reverse_rank = -1;
 
     // Note this is slightly different |has_oom_protection_bindings|.
     // This is equivalent to status == TERMINATION_STATUS_NORMAL_TERMINATION,

components/crash/content/browser/crash_metrics_reporter_android.cc

@@ -287,6 +287,17 @@ void CrashMetricsReporter::ChildProcessExited(
         info.remaining_process_with_strong_binding, 20);
   }
 
+  if (android_oom_kill) {
+    if (info.best_effort_reverse_rank >= 0) {
+      UMA_HISTOGRAM_EXACT_LINEAR("Stability.Android.OomKillReverseRank",
+                                 info.best_effort_reverse_rank, 50);
+    }
+    if (info.best_effort_reverse_rank != -2) {
+      UMA_HISTOGRAM_BOOLEAN("Stability.Android.OomKillReverseRankSuccess",
+                            info.best_effort_reverse_rank != -1);
+    }
+  }
+
   ReportLegacyCrashUma(info, crashed);
   NotifyObservers(info.process_host_id, reported_counts);
 }

content/browser/child_process_launcher_helper_android.cc

@@ -190,7 +190,8 @@ static void JNI_ChildProcessLauncherHelperImpl_SetTerminationInfo(
     jboolean clean_exit,
     jint remaining_process_with_strong_binding,
     jint remaining_process_with_moderate_binding,
-    jint remaining_process_with_waived_binding) {
+    jint remaining_process_with_waived_binding,
+    jint reverse_rank) {
   ChildProcessTerminationInfo* info =
       reinterpret_cast<ChildProcessTerminationInfo*>(termination_info_ptr);
   info->binding_state =
@@ -203,6 +204,7 @@ static void JNI_ChildProcessLauncherHelperImpl_SetTerminationInfo(
       remaining_process_with_moderate_binding;
   info->remaining_process_with_waived_binding =
       remaining_process_with_waived_binding;
+  info->best_effort_reverse_rank = reverse_rank;
 }
 
 // static

content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelperImpl.java

@@ -168,6 +168,7 @@ public final class ChildProcessLauncherHelperImpl {
                     sLauncherByPid.remove(connection.getPid());
                     if (mBindingManager != null) mBindingManager.removeConnection(connection);
                     if (mRanking != null) {
+                        setReverseRankWhenConnectionLost(mRanking.getReverseRank(connection));
                         mRanking.removeConnection(connection);
                         if (mBindingManager != null) mBindingManager.rankingChanged();
                     }
@@ -183,6 +184,10 @@ public final class ChildProcessLauncherHelperImpl {
     private @ChildProcessImportance int mEffectiveImportance = ChildProcessImportance.MODERATE;
     private boolean mVisible;
 
+    // Protects fields below that are accessed on client thread as well.
+    private final Object mLock = new Object();
+    private int mReverseRankWhenConnectionLost;
+
     @CalledByNative
     private static FileDescriptorInfo makeFdInfo(
             int id, int fd, boolean autoClose, long offset, long size) {
@@ -423,9 +428,12 @@ public final class ChildProcessLauncherHelperImpl {
         if (sandboxed) {
             mRanking = sSandboxedChildConnectionRanking;
             mBindingManager = sBindingManager;
+            mReverseRankWhenConnectionLost = -1;
         } else {
             mRanking = null;
             mBindingManager = null;
+            // -2 means not applicable.
+            mReverseRankWhenConnectionLost = -2;
         }
     }
 
@@ -441,6 +449,18 @@ public final class ChildProcessLauncherHelperImpl {
         return TextUtils.isEmpty(mProcessType) ? "" : mProcessType;
     }
 
+    private void setReverseRankWhenConnectionLost(int reverseRank) {
+        synchronized (mLock) {
+            mReverseRankWhenConnectionLost = reverseRank;
+        }
+    }
+
+    private int getReverseRankWhenConnectionLost() {
+        synchronized (mLock) {
+            return mReverseRankWhenConnectionLost;
+        }
+    }
+
     // Called on client (UI or IO) thread.
     @CalledByNative
     private void getTerminationInfoAndStop(long terminationInfoPtr) {
@@ -450,11 +470,14 @@ public final class ChildProcessLauncherHelperImpl {
         // access it afterwards.
         if (connection == null) return;
 
+        // Note there is no guarantee that connection lost has happened. However ChildProcessRanking
+        // is not thread safe, so this is the best we can do.
+        int reverseRank = getReverseRankWhenConnectionLost();
         int bindingCounts[] = connection.remainingBindingStateCountsCurrentOrWhenDied();
         nativeSetTerminationInfo(terminationInfoPtr, connection.bindingStateCurrentOrWhenDied(),
                 connection.isKilledByUs(), connection.hasCleanExit(),
                 bindingCounts[ChildBindingState.STRONG], bindingCounts[ChildBindingState.MODERATE],
-                bindingCounts[ChildBindingState.WAIVED]);
+                bindingCounts[ChildBindingState.WAIVED], reverseRank);
         LauncherThread.post(() -> mLauncher.stop());
     }
 
@@ -700,5 +723,5 @@ public final class ChildProcessLauncherHelperImpl {
 
     private static native void nativeSetTerminationInfo(long termiantionInfoPtr,
             @ChildBindingState int bindingState, boolean killedByUs, boolean cleanExit,
-            int remainingStrong, int remainingModerate, int remainingWaived);
+            int remainingStrong, int remainingModerate, int remainingWaived, int reverseRank);
 }

content/public/android/java/src/org/chromium/content/browser/ChildProcessRanking.java

@@ -241,6 +241,17 @@ public class ChildProcessRanking implements Iterable<ChildProcessConnection> {
         return mRankings.get(mRankings.size() - 1).connection;
     }
 
+    /**
+     * @return reverse rank. Eg lowest ranked connection will have value 0.
+     */
+    public int getReverseRank(ChildProcessConnection connection) {
+        assert connection != null;
+        assert mRankings.size() > 0;
+        int i = indexOf(connection);
+        assert i != -1;
+        return mRankings.size() - 1 - i;
+    }
+
     private int indexOf(ChildProcessConnection connection) {
         for (int i = 0; i < mRankings.size(); ++i) {
             if (mRankings.get(i).connection == connection) return i;

content/public/browser/BUILD.gn

@@ -101,6 +101,7 @@ jumbo_source_set("browser_sources") {
     "child_process_data.h",
     "child_process_launcher_utils.h",
     "child_process_security_policy.h",
+    "child_process_termination_info.cc",
     "child_process_termination_info.h",
     "clear_site_data_utils.h",
     "client_certificate_delegate.h",

content/public/browser/child_process_termination_info.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/public/browser/child_process_termination_info.h"
+
+namespace content {
+
+ChildProcessTerminationInfo::ChildProcessTerminationInfo() = default;
+ChildProcessTerminationInfo::ChildProcessTerminationInfo(
+    const ChildProcessTerminationInfo& other) = default;
+ChildProcessTerminationInfo::~ChildProcessTerminationInfo() = default;
+
+}  // namespace content

content/public/browser/child_process_termination_info.h

@@ -8,6 +8,7 @@
 #include "base/process/kill.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "content/common/content_export.h"
 #include "content/public/common/result_codes.h"
 
 #if defined(OS_ANDROID)
@@ -16,7 +17,11 @@
 
 namespace content {
 
-struct ChildProcessTerminationInfo {
+struct CONTENT_EXPORT ChildProcessTerminationInfo {
+  ChildProcessTerminationInfo();
+  ChildProcessTerminationInfo(const ChildProcessTerminationInfo& other);
+  ~ChildProcessTerminationInfo();
+
   base::TerminationStatus status = base::TERMINATION_STATUS_NORMAL_TERMINATION;
 
   // If |status| is TERMINATION_STATUS_LAUNCH_FAILED then |exit_code| will
@@ -44,6 +49,12 @@ struct ChildProcessTerminationInfo {
   int remaining_process_with_strong_binding = 0;
   int remaining_process_with_moderate_binding = 0;
   int remaining_process_with_waived_binding = 0;
+
+  // Eg lowest ranked process at time of death should have value 0.
+  // Valid values are non-negative.
+  // -1 means could not be obtained due to threading restrictions.
+  // -2 means not applicable because process is not ranked.
+  int best_effort_reverse_rank = -1;
 #endif
 };
 

content/public/test/mock_render_process_host.cc

@@ -93,7 +93,9 @@ void MockRenderProcessHost::SimulateRenderProcessExit(
     base::TerminationStatus status,
     int exit_code) {
   has_connection_ = false;
-  ChildProcessTerminationInfo termination_info{status, exit_code};
+  ChildProcessTerminationInfo termination_info;
+  termination_info.status = status;
+  termination_info.exit_code = exit_code;
   NotificationService::current()->Notify(
       NOTIFICATION_RENDERER_PROCESS_CLOSED, Source<RenderProcessHost>(this),
       Details<ChildProcessTerminationInfo>(&termination_info));

tools/metrics/histograms/histograms.xml

@@ -121640,6 +121640,30 @@ should be kept until we use this API.  -->
   </summary>
 </histogram>
 
+<histogram name="Stability.Android.OomKillReverseRank" units="rank"
+    expires_after="2020-04-10">
+  <owner>boliu@chromium.org</owner>
+  <owner>ssid@chromium.org</owner>
+  <summary>
+    Records the reverse rank of a child process when it is killed by android if
+    applicable. Chrome on Android ranks some child processes and provides hints
+    to android that it should kill from lowest to highest ranked. The lowest
+    ranked process has reverse rank 0. This is a measure how good the hints to
+    android are; if hints were perfect, then all android kills should have
+    reverse rank 0.
+  </summary>
+</histogram>
+
+<histogram name="Stability.Android.OomKillReverseRankSuccess"
+    enum="BooleanSuccess" expires_after="2020-04-10">
+  <owner>boliu@chromium.org</owner>
+  <owner>ssid@chromium.org</owner>
+  <summary>
+    Getting value for OomKillReverseRank may fail. Recorded when an applicable
+    child process is killed by android.
+  </summary>
+</histogram>
+
 <histogram name="Stability.Android.PendingMinidumpsOnStartup" units="minidumps"
     expires_after="2017-05-16">
   <obsolete>