android: Fix inputs into CrashDumpObserver

Overall goal is to switch Crash related classes on Android to use
content::ChildProcessTerminationInfo, which has more relevant
termination information than is currently available.

This is a set of related changes / fixes in order to make intentions and
behaviors on Android clearer.

* Change BrowserChildProcessHostImpl::OnChildDisconnected to always call
  BrowserChildProcessKilled (and BrowserChildProcessHostDelegate
  OnProcessCrashed), and explicitly document this behavior. This is
  because Android cannot distinguish between crashes and kills, and
  basing this decision on whether there was oom protection makes very
  little sense. Generally base::TerminationStatus does not convey much
  information.
  One concrete behavior change is that BrowserChildProcessKilled (and
  OnProcessCrashed) will now be called even when chrome app is in the
  background.
* Corresponding to above, have CrashDumpObserver listen to
  BrowserChildProcessKilled instead of BrowserChildProcessCrashed.
  Crashed was never called anyway. Listening to Killed allows populating
  additional termination info.
* Stop using base::TerminationStatus in crash, and use bool
  was_oom_protected_status instead, which is the only useful bit of
  information conveyed by TerminationStatus on Android.
  This also forces code to stop abusing other values of
  TerminationStatus for other purposes, ie added an explicit bool
  normal_exit instead of relying on NORMAL_EXIT.
* FastShutdownStarted case is treated as a normal exit. Previously it
  was treated as a "normal crash", although with other fields
  unpopulated.

components/data_reduction_proxy TBR
TBR=tbansal@chromium.org

Bug: 693484
Change-Id: I887dcff000ccbc38bbb9e4159c1c5bb27a0c8876
Reviewed-on: https://chromium-review.googlesource.com/1025042
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554581}

android_webview/browser/aw_browser_terminator.cc

@@ -129,21 +129,15 @@ void AwBrowserTerminator::OnChildStart(
 }
 
 void AwBrowserTerminator::OnChildExitAsync(
-    int process_host_id,
-    base::ProcessHandle pid,
-    content::ProcessType process_type,
-    base::TerminationStatus termination_status,
-    base::android::ApplicationState app_state,
+    const breakpad::CrashDumpObserver::TerminationInfo& info,
     base::FilePath crash_dump_dir,
     std::unique_ptr<base::SyncSocket> pipe) {
   if (crash_reporter::IsCrashReporterEnabled()) {
     breakpad::CrashDumpManager::GetInstance()->ProcessMinidumpFileFromChild(
-        crash_dump_dir, process_host_id, process_type, termination_status,
-        app_state);
+        crash_dump_dir, info);
   }
 
-  if (!pipe.get() ||
-      termination_status == base::TERMINATION_STATUS_NORMAL_TERMINATION)
+  if (!pipe.get() || info.normal_termination)
     return;
 
   bool crashed = false;
@@ -153,22 +147,19 @@ void AwBrowserTerminator::OnChildExitAsync(
   if (pipe->Peek() >= sizeof(int)) {
     int exit_code;
     pipe->Receive(&exit_code, sizeof(exit_code));
-    LOG(ERROR) << "Renderer process (" << pid << ") crash detected (code "
+    LOG(ERROR) << "Renderer process (" << info.pid << ") crash detected (code "
                << exit_code << ").";
     crashed = true;
   }
 
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                          base::BindOnce(&OnRenderProcessGoneDetail,
-                                         process_host_id, pid, crashed));
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&OnRenderProcessGoneDetail, info.process_host_id, info.pid,
+                     crashed));
 }
 
 void AwBrowserTerminator::OnChildExit(
-    int process_host_id,
-    base::ProcessHandle pid,
-    content::ProcessType process_type,
-    base::TerminationStatus termination_status,
-    base::android::ApplicationState app_state) {
+    const breakpad::CrashDumpObserver::TerminationInfo& info) {
   std::unique_ptr<base::SyncSocket> pipe;
 
   {
@@ -176,7 +167,7 @@ void AwBrowserTerminator::OnChildExit(
     // We might get a NOTIFICATION_RENDERER_PROCESS_TERMINATED and a
     // NOTIFICATION_RENDERER_PROCESS_CLOSED. In that case we only want
     // to process the first notification.
-    const auto& iter = process_host_id_to_pipe_.find(process_host_id);
+    const auto& iter = process_host_id_to_pipe_.find(info.process_host_id);
     if (iter != process_host_id_to_pipe_.end()) {
       pipe = std::move(iter->second);
       DCHECK(pipe->handle() != base::SyncSocket::kInvalidHandle);
@@ -184,15 +175,14 @@ void AwBrowserTerminator::OnChildExit(
     }
   }
   if (pipe.get()) {
-    OnRenderProcessGone(process_host_id);
+    OnRenderProcessGone(info.process_host_id);
   }
 
   base::PostTaskWithTraits(
       FROM_HERE,
       {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
        base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
-      base::BindOnce(&AwBrowserTerminator::OnChildExitAsync, process_host_id,
-                     pid, process_type, termination_status, app_state,
+      base::BindOnce(&AwBrowserTerminator::OnChildExitAsync, info,
                      crash_dump_dir_, std::move(pipe)));
 }
 

android_webview/browser/aw_browser_terminator.h

@@ -33,20 +33,14 @@ class AwBrowserTerminator : public breakpad::CrashDumpObserver::Client {
   // breakpad::CrashDumpObserver::Client implementation.
   void OnChildStart(int process_host_id,
                     content::PosixFileDescriptorInfo* mappings) override;
-  void OnChildExit(int process_host_id,
-                   base::ProcessHandle pid,
-                   content::ProcessType process_type,
-                   base::TerminationStatus termination_status,
-                   base::android::ApplicationState app_state) override;
+  void OnChildExit(
+      const breakpad::CrashDumpObserver::TerminationInfo& info) override;
 
  private:
-  static void OnChildExitAsync(int process_host_id,
-                               base::ProcessHandle pid,
-                               content::ProcessType process_type,
-                               base::TerminationStatus termination_status,
-                               base::android::ApplicationState app_state,
-                               base::FilePath crash_dump_dir,
-                               std::unique_ptr<base::SyncSocket> pipe);
+  static void OnChildExitAsync(
+      const breakpad::CrashDumpObserver::TerminationInfo& info,
+      base::FilePath crash_dump_dir,
+      std::unique_ptr<base::SyncSocket> pipe);
 
   base::FilePath crash_dump_dir_;
 

chrome/browser/android/compositor/compositor_view.cc

@@ -249,9 +249,10 @@ void CompositorView::SetNeedsComposite(JNIEnv* env,
   compositor_->SetNeedsComposite();
 }
 
-void CompositorView::BrowserChildProcessHostDisconnected(
-    const content::ChildProcessData& data) {
-  LOG(WARNING) << "Child process disconnected (type=" << data.process_type
+void CompositorView::BrowserChildProcessKilled(
+    const content::ChildProcessData& data,
+    const content::ChildProcessTerminationInfo& info) {
+  LOG(WARNING) << "Child process died (type=" << data.process_type
                << ") pid=" << data.handle << ")";
   if (base::android::BuildInfo::GetInstance()->sdk_int() <=
           base::android::SDK_VERSION_JELLY_BEAN_MR2 &&

chrome/browser/android/compositor/compositor_view.h

@@ -96,8 +96,9 @@ class CompositorView : public content::CompositorClient,
   ~CompositorView() override;
 
   // content::BrowserChildProcessObserver implementation:
-  void BrowserChildProcessHostDisconnected(
-      const content::ChildProcessData& data) override;
+  void BrowserChildProcessKilled(
+      const content::ChildProcessData& data,
+      const content::ChildProcessTerminationInfo& info) override;
 
   void SetBackground(bool visible, SkColor color);
 

chrome/browser/metrics/chrome_stability_metrics_provider.cc

@@ -147,7 +147,7 @@ void ChromeStabilityMetricsProvider::OnCrashDumpProcessed(
   if (details.status ==
           breakpad::CrashDumpManager::CrashDumpStatus::kValidDump &&
       details.process_type == content::PROCESS_TYPE_RENDERER &&
-      details.termination_status == base::TERMINATION_STATUS_OOM_PROTECTED &&
+      details.was_oom_protected_status &&
       (details.app_state ==
            base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES ||
        details.app_state ==

chrome/browser/metrics/oom/out_of_memory_reporter_unittest.cc

@@ -144,9 +144,9 @@ class OutOfMemoryReporterTest : public ChromeRenderViewHostTestHarness,
         crash_waiter.Wait();
     EXPECT_EQ(expect_oom, breakpad::CrashDumpManager::IsForegroundOom(details))
         << "process_type: " << details.process_type
-        << " termination_status: " << details.termination_status
         << " file_size: " << details.file_size
-        << " app_state: " << details.app_state;
+        << " app_state: " << details.app_state
+        << " was_oom_protected_status: " << details.was_oom_protected_status;
 
     // Since the observer list is not ordered, it isn't guaranteed that the
     // OutOfMemoryReporter will be notified at this point. Flush the current

components/crash/content/browser/child_process_crash_observer_android.cc

@@ -35,11 +35,7 @@ void ChildProcessCrashObserver::OnChildStart(
 }
 
 void ChildProcessCrashObserver::OnChildExit(
-    int process_host_id,
-    base::ProcessHandle pid,
-    content::ProcessType process_type,
-    base::TerminationStatus termination_status,
-    base::android::ApplicationState app_state) {
+    const CrashDumpObserver::TerminationInfo& info) {
   // This might be called twice for a given child process, with a
   // NOTIFICATION_RENDERER_PROCESS_TERMINATED and then with
   // NOTIFICATION_RENDERER_PROCESS_CLOSED.
@@ -48,8 +44,7 @@ void ChildProcessCrashObserver::OnChildExit(
       FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
       base::Bind(&CrashDumpManager::ProcessMinidumpFileFromChild,
                  base::Unretained(CrashDumpManager::GetInstance()),
-                 crash_dump_dir_, process_host_id, process_type,
-                 termination_status, app_state));
+                 crash_dump_dir_, info));
 }
 
 }  // namespace breakpad

components/crash/content/browser/child_process_crash_observer_android.h

@@ -19,11 +19,7 @@ class ChildProcessCrashObserver : public breakpad::CrashDumpObserver::Client {
   // breakpad::CrashDumpObserver::Client implementation:
   void OnChildStart(int process_host_id,
                     content::PosixFileDescriptorInfo* mappings) override;
-  void OnChildExit(int process_host_id,
-                   base::ProcessHandle pid,
-                   content::ProcessType process_type,
-                   base::TerminationStatus termination_status,
-                   base::android::ApplicationState app_state) override;
+  void OnChildExit(const CrashDumpObserver::TerminationInfo& info) override;
 
  private:
   base::FilePath crash_dump_dir_;

components/crash/content/browser/crash_dump_manager_android.cc

@@ -34,11 +34,11 @@ base::LazyInstance<CrashDumpManager>::Leaky g_instance =
 CrashDumpManager::CrashDumpDetails::CrashDumpDetails(
     int process_host_id,
     content::ProcessType process_type,
-    base::TerminationStatus termination_status,
+    bool was_oom_protected_status,
     base::android::ApplicationState app_state)
     : process_host_id(process_host_id),
       process_type(process_type),
-      termination_status(termination_status),
+      was_oom_protected_status(was_oom_protected_status),
       app_state(app_state) {}
 
 CrashDumpManager::CrashDumpDetails::CrashDumpDetails() {}
@@ -48,7 +48,7 @@ CrashDumpManager::CrashDumpDetails::CrashDumpDetails(
     const CrashDumpManager::CrashDumpDetails& other)
     : CrashDumpDetails(other.process_host_id,
                        other.process_type,
-                       other.termination_status,
+                       other.was_oom_protected_status,
                        other.app_state) {
   file_size = other.file_size;
   status = other.status;
@@ -63,8 +63,7 @@ CrashDumpManager* CrashDumpManager::GetInstance() {
 bool CrashDumpManager::IsForegroundOom(const CrashDumpDetails& details) {
   // If the crash is size 0, it is an OOM.
   return details.process_type == content::PROCESS_TYPE_RENDERER &&
-         details.termination_status == base::TERMINATION_STATUS_OOM_PROTECTED &&
-         details.file_size == 0 &&
+         details.was_oom_protected_status && details.file_size == 0 &&
          details.app_state ==
              base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES;
 }
@@ -109,17 +108,14 @@ base::ScopedFD CrashDumpManager::CreateMinidumpFileForChild(
 
 void CrashDumpManager::ProcessMinidumpFileFromChild(
     base::FilePath crash_dump_dir,
-    int process_host_id,
-    content::ProcessType process_type,
-    base::TerminationStatus termination_status,
-    base::android::ApplicationState app_state) {
+    const CrashDumpObserver::TerminationInfo& info) {
   base::AssertBlockingAllowed();
-  CrashDumpDetails details(process_host_id, process_type, termination_status,
-                           app_state);
+  CrashDumpDetails details(info.process_host_id, info.process_type,
+                           info.was_oom_protected_status, info.app_state);
   base::FilePath minidump_path;
   // If the minidump for a given child process has already been
   // processed, then there is no more work to do.
-  if (!GetMinidumpPath(process_host_id, &minidump_path)) {
+  if (!GetMinidumpPath(info.process_host_id, &minidump_path)) {
     NotifyObservers(details);
     return;
   }
@@ -132,20 +128,19 @@ void CrashDumpManager::ProcessMinidumpFileFromChild(
   }
 
   int r = base::GetFileSize(minidump_path, &file_size);
-  DCHECK(r) << "Failed to retrieve size for minidump "
-            << minidump_path.value();
+  DCHECK(r) << "Failed to retrieve size for minidump " << minidump_path.value();
 
   // TODO(wnwen): If these numbers match up to TabWebContentsObserver's
   //     TabRendererCrashStatus histogram, then remove that one as this is more
   //     accurate with more detail.
-  if ((process_type == content::PROCESS_TYPE_RENDERER ||
-       process_type == content::PROCESS_TYPE_GPU) &&
-      app_state != base::android::APPLICATION_STATE_UNKNOWN) {
+  if ((info.process_type == content::PROCESS_TYPE_RENDERER ||
+       info.process_type == content::PROCESS_TYPE_GPU) &&
+      info.app_state != base::android::APPLICATION_STATE_UNKNOWN) {
     ExitStatus exit_status;
-    bool is_running =
-        (app_state == base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES);
-    bool is_paused =
-        (app_state == base::android::APPLICATION_STATE_HAS_PAUSED_ACTIVITIES);
+    bool is_running = (info.app_state ==
+                       base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES);
+    bool is_paused = (info.app_state ==
+                      base::android::APPLICATION_STATE_HAS_PAUSED_ACTIVITIES);
     if (file_size == 0) {
       if (is_running) {
         exit_status = EMPTY_MINIDUMP_WHILE_RUNNING;
@@ -163,17 +158,16 @@ void CrashDumpManager::ProcessMinidumpFileFromChild(
         exit_status = VALID_MINIDUMP_WHILE_BACKGROUND;
       }
     }
-    if (process_type == content::PROCESS_TYPE_RENDERER) {
-      if (termination_status == base::TERMINATION_STATUS_OOM_PROTECTED) {
-        UMA_HISTOGRAM_ENUMERATION("Tab.RendererDetailedExitStatus",
-                                  exit_status,
+    if (info.process_type == content::PROCESS_TYPE_RENDERER) {
+      if (info.was_oom_protected_status) {
+        UMA_HISTOGRAM_ENUMERATION("Tab.RendererDetailedExitStatus", exit_status,
                                   ExitStatus::MINIDUMP_STATUS_COUNT);
       } else {
         UMA_HISTOGRAM_ENUMERATION("Tab.RendererDetailedExitStatusUnbound",
                                   exit_status,
                                   ExitStatus::MINIDUMP_STATUS_COUNT);
       }
-    } else if (process_type == content::PROCESS_TYPE_GPU) {
+    } else if (info.process_type == content::PROCESS_TYPE_GPU) {
       UMA_HISTOGRAM_ENUMERATION("GPU.GPUProcessDetailedExitStatus", exit_status,
                                 ExitStatus::MINIDUMP_STATUS_COUNT);
     }
@@ -196,8 +190,9 @@ void CrashDumpManager::ProcessMinidumpFileFromChild(
     return;
   }
   const uint64_t rand = base::RandUint64();
-  const std::string filename = base::StringPrintf(
-      "chromium-renderer-minidump-%016" PRIx64 ".dmp%d", rand, process_host_id);
+  const std::string filename =
+      base::StringPrintf("chromium-renderer-minidump-%016" PRIx64 ".dmp%d",
+                         rand, info.process_host_id);
   base::FilePath dest_path = crash_dump_dir.Append(filename);
   r = base::Move(minidump_path, dest_path);
   if (!r) {

components/crash/content/browser/crash_dump_manager_android.h

@@ -13,8 +13,8 @@
 #include "base/lazy_instance.h"
 #include "base/memory/ref_counted.h"
 #include "base/observer_list_threadsafe.h"
-#include "base/process/kill.h"
 #include "base/synchronization/lock.h"
+#include "components/crash/content/browser/crash_dump_observer_android.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/process_type.h"
 
@@ -58,7 +58,7 @@ class CrashDumpManager {
   struct CrashDumpDetails {
     CrashDumpDetails(int process_host_id,
                      content::ProcessType process_type,
-                     base::TerminationStatus termination_status,
+                     bool was_oom_protected_status,
                      base::android::ApplicationState app_state);
     CrashDumpDetails();
     ~CrashDumpDetails();
@@ -67,7 +67,7 @@ class CrashDumpManager {
     int process_host_id = content::ChildProcessHost::kInvalidUniqueID;
 
     content::ProcessType process_type = content::PROCESS_TYPE_UNKNOWN;
-    base::TerminationStatus termination_status;
+    bool was_oom_protected_status = false;
     base::android::ApplicationState app_state;
     int64_t file_size = 0;
     CrashDumpStatus status = CrashDumpStatus::kNoDump;
@@ -91,11 +91,9 @@ class CrashDumpManager {
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  void ProcessMinidumpFileFromChild(base::FilePath crash_dump_dir,
-                                    int process_host_id,
-                                    content::ProcessType process_type,
-                                    base::TerminationStatus termination_status,
-                                    base::android::ApplicationState app_state);
+  void ProcessMinidumpFileFromChild(
+      base::FilePath crash_dump_dir,
+      const CrashDumpObserver::TerminationInfo& info);
 
   base::ScopedFD CreateMinidumpFileForChild(int process_host_id);
 

components/crash/content/browser/crash_dump_manager_android_unittest.cc

@@ -39,18 +39,15 @@ class CrashDumpManagerTest : public testing::Test {
   // TODO(csharrison): This test harness is not robust enough. Namely, it does
   // not support actually processing a non-empty crash dump, due to JNI calls.
   static void CreateAndProcessEmptyMinidump(
-      int process_host_id,
-      content::ProcessType process_type,
-      base::TerminationStatus status,
-      base::android::ApplicationState app_state) {
+      const CrashDumpObserver::TerminationInfo& info) {
     base::ScopedFD fd =
         CrashDumpManager::GetInstance()->CreateMinidumpFileForChild(
-            process_host_id);
+            info.process_host_id);
     EXPECT_TRUE(fd.is_valid());
     base::ScopedTempDir dump_dir;
     EXPECT_TRUE(dump_dir.CreateUniqueTempDir());
     CrashDumpManager::GetInstance()->ProcessMinidumpFileFromChild(
-        dump_dir.GetPath(), process_host_id, process_type, status, app_state);
+        dump_dir.GetPath(), info);
   }
 
  private:
@@ -96,19 +93,27 @@ TEST_F(CrashDumpManagerTest, SimpleOOM) {
   manager->AddObserver(&crash_dump_observer);
 
   int process_host_id = 1;
+  CrashDumpObserver::TerminationInfo termination_info{
+      process_host_id,
+      base::kNullProcessHandle,
+      content::PROCESS_TYPE_RENDERER,
+      base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES,
+      false /* normal_termination */,
+      true /* has_oom_protection_bindings */,
+      false /* was_killed_intentionally_by_browser */,
+      true /* was_oom_protected_status */
+  };
   base::PostTaskWithTraits(
       FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
       base::Bind(&CrashDumpManagerTest::CreateAndProcessEmptyMinidump,
-                 process_host_id, content::PROCESS_TYPE_RENDERER,
-                 base::TERMINATION_STATUS_OOM_PROTECTED,
-                 base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES));
+                 termination_info));
   crash_dump_observer.WaitForProcessed();
 
   const CrashDumpManager::CrashDumpDetails& details =
       crash_dump_observer.last_details();
   EXPECT_EQ(process_host_id, details.process_host_id);
   EXPECT_EQ(content::PROCESS_TYPE_RENDERER, details.process_type);
-  EXPECT_EQ(base::TERMINATION_STATUS_OOM_PROTECTED, details.termination_status);
+  EXPECT_TRUE(details.was_oom_protected_status);
   EXPECT_EQ(base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES,
             details.app_state);
   EXPECT_EQ(0, details.file_size);
@@ -127,20 +132,28 @@ TEST_F(CrashDumpManagerTest, NoDumpCreated) {
   manager->AddObserver(&crash_dump_observer);
 
   int process_host_id = 1;
+  CrashDumpObserver::TerminationInfo termination_info{
+      process_host_id,
+      base::kNullProcessHandle,
+      content::PROCESS_TYPE_RENDERER,
+      base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES,
+      false /* normal_termination */,
+      true /* has_oom_protection_bindings */,
+      false /* was_killed_intentionally_by_browser */,
+      true /* was_oom_protected_status */
+  };
   base::PostTaskWithTraits(
       FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
       base::Bind(&CrashDumpManager::ProcessMinidumpFileFromChild,
-                 base::Unretained(manager), base::FilePath(), process_host_id,
-                 content::PROCESS_TYPE_RENDERER,
-                 base::TERMINATION_STATUS_OOM_PROTECTED,
-                 base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES));
+                 base::Unretained(manager), base::FilePath(),
+                 termination_info));
   crash_dump_observer.WaitForProcessed();
 
   const CrashDumpManager::CrashDumpDetails& details =
       crash_dump_observer.last_details();
   EXPECT_EQ(process_host_id, details.process_host_id);
   EXPECT_EQ(content::PROCESS_TYPE_RENDERER, details.process_type);
-  EXPECT_EQ(base::TERMINATION_STATUS_OOM_PROTECTED, details.termination_status);
+  EXPECT_TRUE(details.was_oom_protected_status);
   EXPECT_EQ(base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES,
             details.app_state);
   EXPECT_EQ(0, details.file_size);

components/crash/content/browser/crash_dump_observer_android.cc

@@ -21,10 +21,22 @@ using content::BrowserThread;
 namespace breakpad {
 
 namespace {
+
 base::LazyInstance<CrashDumpObserver>::DestructorAtExit g_instance =
     LAZY_INSTANCE_INITIALIZER;
+
+void PopulateTerminationInfo(
+    const content::ChildProcessTerminationInfo& content_info,
+    CrashDumpObserver::TerminationInfo* info) {
+  info->has_oom_protection_bindings = content_info.has_oom_protection_bindings;
+  info->was_killed_intentionally_by_browser =
+      content_info.was_killed_intentionally_by_browser;
+  info->was_oom_protected_status =
+      content_info.status == base::TERMINATION_STATUS_OOM_PROTECTED;
 }
 
+}  // namespace
+
 // static
 void CrashDumpObserver::Create() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -64,11 +76,7 @@ void CrashDumpObserver::RegisterClient(std::unique_ptr<Client> client) {
   registered_clients_.push_back(std::move(client));
 }
 
-void CrashDumpObserver::OnChildExit(int process_host_id,
-                                    base::ProcessHandle pid,
-                                    content::ProcessType process_type,
-                                    base::TerminationStatus termination_status,
-                                    base::android::ApplicationState app_state) {
+void CrashDumpObserver::OnChildExit(const TerminationInfo& info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   std::vector<Client*> registered_clients_copy;
   {
@@ -77,8 +85,7 @@ void CrashDumpObserver::OnChildExit(int process_host_id,
       registered_clients_copy.push_back(client.get());
   }
   for (auto* client : registered_clients_copy) {
-    client->OnChildExit(process_host_id, pid, process_type, termination_status,
-                        app_state);
+    client->OnChildExit(info);
   }
 }
 
@@ -99,20 +106,31 @@ void CrashDumpObserver::BrowserChildProcessStarted(
 void CrashDumpObserver::BrowserChildProcessHostDisconnected(
     const content::ChildProcessData& data) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  OnChildExit(data.id, data.handle,
-              static_cast<content::ProcessType>(data.process_type),
-              base::TERMINATION_STATUS_MAX_ENUM,
-              base::android::ApplicationStatusListener::GetState());
+  TerminationInfo info;
+  auto it = browser_child_process_info_.find(data.id);
+  if (it != browser_child_process_info_.end()) {
+    info = it->second;
+    browser_child_process_info_.erase(it);
+  } else {
+    info = TerminationInfo{data.id, data.handle,
+                           static_cast<content::ProcessType>(data.process_type),
+                           base::android::ApplicationStatusListener::GetState(),
+                           true /* normal_termination */};
+  }
+  OnChildExit(info);
 }
 
-void CrashDumpObserver::BrowserChildProcessCrashed(
+void CrashDumpObserver::BrowserChildProcessKilled(
     const content::ChildProcessData& data,
-    const content::ChildProcessTerminationInfo& info) {
+    const content::ChildProcessTerminationInfo& content_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  OnChildExit(data.id, data.handle,
-              static_cast<content::ProcessType>(data.process_type),
-              base::TERMINATION_STATUS_ABNORMAL_TERMINATION,
-              base::android::APPLICATION_STATE_UNKNOWN);
+  DCHECK(!base::ContainsKey(browser_child_process_info_, data.id));
+  TerminationInfo info{data.id, data.handle,
+                       static_cast<content::ProcessType>(data.process_type),
+                       base::android::ApplicationStatusListener::GetState()};
+  PopulateTerminationInfo(content_info, &info);
+  browser_child_process_info_.emplace(data.id, info);
+  // Subsequent BrowserChildProcessHostDisconnected will call OnChildExit.
 }
 
 void CrashDumpObserver::Observe(int type,
@@ -121,26 +139,25 @@ void CrashDumpObserver::Observe(int type,
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   content::RenderProcessHost* rph =
       content::Source<content::RenderProcessHost>(source).ptr();
-  base::TerminationStatus term_status = base::TERMINATION_STATUS_MAX_ENUM;
-  base::android::ApplicationState app_state =
-      base::android::APPLICATION_STATE_UNKNOWN;
+  TerminationInfo info{rph->GetID(), rph->GetProcess().Handle(),
+                       content::PROCESS_TYPE_RENDERER,
+                       base::android::APPLICATION_STATE_UNKNOWN};
   switch (type) {
     case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED: {
       // NOTIFICATION_RENDERER_PROCESS_TERMINATED is sent when the renderer
       // process is cleanly shutdown.
-      term_status = base::TERMINATION_STATUS_NORMAL_TERMINATION;
+      info.normal_termination = true;
       break;
     }
     case content::NOTIFICATION_RENDERER_PROCESS_CLOSED: {
       // We do not care about android fast shutdowns as it is a known case where
       // the renderer is intentionally killed when we are done with it.
-      if (rph->FastShutdownStarted()) {
-        break;
-      }
-      content::ChildProcessTerminationInfo* termination_info =
-          content::Details<content::ChildProcessTerminationInfo>(details).ptr();
-      term_status = termination_info->status;
-      app_state = base::android::ApplicationStatusListener::GetState();
+      info.normal_termination = rph->FastShutdownStarted();
+      info.app_state = base::android::ApplicationStatusListener::GetState();
+      const auto& content_info =
+          *content::Details<content::ChildProcessTerminationInfo>(details)
+               .ptr();
+      PopulateTerminationInfo(content_info, &info);
       break;
     }
     case content::NOTIFICATION_RENDERER_PROCESS_CREATED: {
@@ -153,16 +170,14 @@ void CrashDumpObserver::Observe(int type,
       NOTREACHED();
       return;
   }
-  base::ProcessHandle pid = rph->GetProcess().Handle();
   const auto& iter = process_host_id_to_pid_.find(rph->GetID());
   if (iter != process_host_id_to_pid_.end()) {
-    if (pid == base::kNullProcessHandle) {
-      pid = iter->second;
+    if (info.pid == base::kNullProcessHandle) {
+      info.pid = iter->second;
     }
     process_host_id_to_pid_.erase(iter);
   }
-  OnChildExit(rph->GetID(), pid, content::PROCESS_TYPE_RENDERER, term_status,
-              app_state);
+  OnChildExit(info);
 }
 
 }  // namespace breakpad

components/crash/content/browser/crash_dump_observer_android.h

@@ -11,14 +11,18 @@
 #include "base/android/application_status_listener.h"
 #include "base/lazy_instance.h"
 #include "base/memory/ref_counted.h"
-#include "base/process/kill.h"
 #include "base/process/process.h"
 #include "content/public/browser/browser_child_process_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/posix_file_descriptor_info.h"
+#include "content/public/common/child_process_host.h"
 #include "content/public/common/process_type.h"
 
+namespace content {
+struct ChildProcessTerminationInfo;
+}
+
 namespace breakpad {
 
 // This class centralises the observation of child processes for the
@@ -27,6 +31,31 @@ namespace breakpad {
 class CrashDumpObserver : public content::BrowserChildProcessObserver,
                           public content::NotificationObserver {
  public:
+  struct TerminationInfo {
+    int process_host_id = content::ChildProcessHost::kInvalidUniqueID;
+    base::ProcessHandle pid = base::kNullProcessHandle;
+    content::ProcessType process_type = content::PROCESS_TYPE_UNKNOWN;
+    base::android::ApplicationState app_state =
+        base::android::APPLICATION_STATE_UNKNOWN;
+
+    // True if this is intentional shutdown of the child process, e.g. when a
+    // tab is closed. Some fields below may not be populated if this is true.
+    bool normal_termination = false;
+
+    // Values from ChildProcessTerminationInfo.
+    // Note base::TerminationStatus and exit_code are missing intentionally
+    // because those fields hold no useful information on Android.
+    bool has_oom_protection_bindings = false;
+    bool was_killed_intentionally_by_browser = false;
+
+    // Note this is slightly different |has_oom_protection_bindings|.
+    // This is equivalent to status == TERMINATION_STATUS_NORMAL_TERMINATION,
+    // which historically also checked whether app is in foreground, using
+    // a slightly different implementation than
+    // ApplicationStatusListener::GetState.
+    bool was_oom_protected_status = false;
+  };
+
   // CrashDumpObserver client interface.
   // Client methods will be called synchronously in the order in which
   // clients were registered. It is the implementer's responsibility
@@ -44,13 +73,8 @@ class CrashDumpObserver : public content::BrowserChildProcessObserver,
     virtual void OnChildStart(int process_host_id,
                               content::PosixFileDescriptorInfo* mappings) = 0;
     // OnChildExit is called on the UI thread.
-    // OnChildExit may be called twice (once for the child process
-    // termination, and once for the IPC channel disconnection).
-    virtual void OnChildExit(int process_host_id,
-                             base::ProcessHandle pid,
-                             content::ProcessType process_type,
-                             base::TerminationStatus termination_status,
-                             base::android::ApplicationState app_state) = 0;
+    // OnChildExit may be called twice for the same process.
+    virtual void OnChildExit(const TerminationInfo& info) = 0;
 
     virtual ~Client() {}
   };
@@ -84,12 +108,9 @@ class CrashDumpObserver : public content::BrowserChildProcessObserver,
   // content::BrowserChildProcessObserver implementation:
   void BrowserChildProcessHostDisconnected(
       const content::ChildProcessData& data) override;
-  void BrowserChildProcessCrashed(
+  void BrowserChildProcessKilled(
       const content::ChildProcessData& data,
       const content::ChildProcessTerminationInfo& info) override;
-  // On Android we will never observe BrowserChildProcessCrashed
-  // because we do not receive exit codes from zygote spawned
-  // processes.
 
   // NotificationObserver implementation:
   void Observe(int type,
@@ -97,11 +118,7 @@ class CrashDumpObserver : public content::BrowserChildProcessObserver,
                const content::NotificationDetails& details) override;
 
   // Called on child process exit (including crash).
-  void OnChildExit(int process_host_id,
-                   base::ProcessHandle pid,
-                   content::ProcessType process_type,
-                   base::TerminationStatus termination_status,
-                   base::android::ApplicationState app_state);
+  void OnChildExit(const TerminationInfo& info);
 
   content::NotificationRegistrar notification_registrar_;
 
@@ -111,6 +128,9 @@ class CrashDumpObserver : public content::BrowserChildProcessObserver,
   // process_host_id to process id.
   std::map<int, base::ProcessHandle> process_host_id_to_pid_;
 
+  // Key is process_host_id. Only used for BrowserChildProcessHost.
+  std::map<int, TerminationInfo> browser_child_process_info_;
+
   DISALLOW_COPY_AND_ASSIGN(CrashDumpObserver);
 };
 

components/data_reduction_proxy/content/browser/data_reduction_proxy_pingback_client_impl_unittest.cc

@@ -159,9 +159,7 @@ class DataReductionProxyPingbackClientImplTest : public testing::Test {
   void ReportCrash(bool oom) {
 #if defined(OS_ANDROID)
     breakpad::CrashDumpManager::CrashDumpDetails details = {
-        kCrashProcessId, content::PROCESS_TYPE_RENDERER,
-        oom ? base::TERMINATION_STATUS_OOM_PROTECTED
-            : base::TERMINATION_STATUS_ABNORMAL_TERMINATION,
+        kCrashProcessId, content::PROCESS_TYPE_RENDERER, oom,
         base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES};
     details.file_size = oom ? 0 : 1;
     static_cast<breakpad::CrashDumpManager::Observer*>(pingback_client_.get())

content/browser/browser_child_process_host_impl.cc

@@ -82,11 +82,13 @@ void NotifyProcessHostDisconnected(const ChildProcessData& data) {
     observer.BrowserChildProcessHostDisconnected(data);
 }
 
+#if !defined(OS_ANDROID)
 void NotifyProcessCrashed(const ChildProcessData& data,
                           const ChildProcessTerminationInfo& info) {
   for (auto& observer : g_browser_child_process_observers.Get())
     observer.BrowserChildProcessCrashed(data, info);
 }
+#endif
 
 void NotifyProcessKilled(const ChildProcessData& data,
                          const ChildProcessTerminationInfo& info) {
@@ -431,6 +433,11 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
   if (child_process_.get() || data_.handle) {
     ChildProcessTerminationInfo info =
         GetTerminationInfo(true /* known_dead */);
+#if defined(OS_ANDROID)
+    delegate_->OnProcessCrashed(info.exit_code);
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::BindOnce(&NotifyProcessKilled, data_, info));
+#else  // OS_ANDROID
     switch (info.status) {
       case base::TERMINATION_STATUS_PROCESS_CRASHED:
       case base::TERMINATION_STATUS_ABNORMAL_TERMINATION: {
@@ -443,9 +450,6 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
                                   PROCESS_TYPE_MAX);
         break;
       }
-#if defined(OS_ANDROID)
-      case base::TERMINATION_STATUS_OOM_PROTECTED:
-#endif
 #if defined(OS_CHROMEOS)
       case base::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM:
 #endif
@@ -469,6 +473,7 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
       default:
         break;
     }
+#endif  // OS_ANDROID
     UMA_HISTOGRAM_ENUMERATION("ChildProcess.Disconnected2",
                               static_cast<ProcessType>(data_.process_type),
                               PROCESS_TYPE_MAX);

content/public/browser/browser_child_process_observer.h

@@ -42,6 +42,11 @@ class CONTENT_EXPORT BrowserChildProcessObserver {
       const ChildProcessData& data,
       const ChildProcessTerminationInfo& info) {}
 
+  // Note for Android. There is no way to reliably distinguish between Crash
+  // and Kill. Arbitrarily choose all abnormal terminations on Android to call
+  // BrowserChildProcessKilled, which means BrowserChildProcessCrashed will
+  // never be called on Android.
+
  protected:
   // The observer can be destroyed on any thread.
   virtual ~BrowserChildProcessObserver() {}