Implement Crash Handle API

android_webview/browser/aw_browser_terminator.cc

 // Copyright 2016 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 "android_webview/browser/aw_browser_terminator.h"
 
 #include <unistd.h>
 
+#include "android_webview/browser/aw_render_process_gone_delegate.h"
 #include "android_webview/common/aw_descriptors.h"
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/sync_socket.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host.h"
+#include "content/public/browser/render_widget_host_iterator.h"
+#include "content/public/browser/web_contents.h"
 
 using content::BrowserThread;
 
 namespace android_webview {
 
+namespace {
+
+void GetAwRenderProcessGoneDelegatesForRenderProcess(
+    int render_process_id,
+    std::vector<AwRenderProcessGoneDelegate*>* delegates) {
+  content::RenderProcessHost* rph =
+      content::RenderProcessHost::FromID(render_process_id);
+  if (!rph)
+    return;
+
+  std::unique_ptr<content::RenderWidgetHostIterator> widgets(
+      content::RenderWidgetHost::GetRenderWidgetHosts());
+  while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
+    content::RenderViewHost* view = content::RenderViewHost::From(widget);
+    if (view && rph == view->GetProcess()) {
+      content::WebContents* wc = content::WebContents::FromRenderViewHost(view);
+      if (wc) {
+        AwRenderProcessGoneDelegate* delegate =
+            AwRenderProcessGoneDelegate::FromWebContents(wc);
+        if (delegate)
+          delegates->push_back(delegate);
+      }
+    }
+  }
+}
+
+void OnRenderProcessGone(int child_process_id) {

[Tobias Sargeant, 2017/01/13 10:48:38]

I think it would be good to name this and the below in a less similar manner to
make it clear that this is providing a notification that the delegates will soon
receive the termination reason. Otherwise (to me, at least) it looks like these
are either/or methods.

Also, what guarantee do we have that the list of delegates that we get at this
point is identical to the list below? What if an AwContents is destroyed in the
meantime (or, more confusingly, created. will the new AwContents cause the
creation of a new renderer)?

[michaelbai, 2017/01/13 21:01:38]

Changed to OnRenderProcessGoneDetail(), and rewrote the comments.
No, there is no guarantee, change the description of Delegate class, basically,
AwContents is destroyed, then delegate is gone, there is no meaning to notify it
with detail information, new AwContents has new renderer,  it seems there is no
confusing here.

+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  std::vector<AwRenderProcessGoneDelegate*> delegates;
+  GetAwRenderProcessGoneDelegatesForRenderProcess(child_process_id, &delegates);
+  for (auto delegate : delegates)
+    delegate->OnRenderProcessGone(child_process_id);
+}
+
+void OnRenderProcessGoneWithDetail(int child_process_id, bool crashed) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  std::vector<AwRenderProcessGoneDelegate*> delegates;
+  GetAwRenderProcessGoneDelegatesForRenderProcess(child_process_id, &delegates);
+  for (auto delegate : delegates) {
+    if (!delegate->OnRenderProcessGoneWithDetail(child_process_id, crashed)) {
+      // Keeps this log unchanged, CTS test uses it to detect crash.
+      LOG(ERROR) << "Render process's abnormal termination wasn't handled by"
+                 << " all associated webviews, triggering application crash";
+      // For some reason, FATAL message didn't show up in adb logcat.

[Torne, 2017/01/13 13:24:08]

This normally works, I think; we should probably look into why, rather than just
doing this?

[michaelbai, 2017/01/13 21:01:38]

We shouldn't try to fix LOG issue in this patch, I filed a bug to track it.

[Torne, 2017/01/16 12:20:32]

We use LOG(FATAL) in other places and I'm pretty sure it works in general; I
think you should investigate this specific case and work out what's up rather
than just assuming this is broken.

[michaelbai, 2017/01/17 17:09:42]

Would you like point me where it still work? it will help me to figure out how
it is broken for this specific case. Thanks!

[michaelbai, 2017/01/18 03:01:26]

It seems Android O issue, the message showed up now.

+      LOG(FATAL) << "Crash";
+    }
+  }
+}
+
+}  // namespace
+
 AwBrowserTerminator::AwBrowserTerminator() {}
 
 AwBrowserTerminator::~AwBrowserTerminator() {}
 
 void AwBrowserTerminator::OnChildStart(int child_process_id,
                                        content::FileDescriptorInfo* mappings) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::PROCESS_LAUNCHER);
 
   base::AutoLock auto_lock(child_process_id_to_pipe_lock_);
   DCHECK(!ContainsKey(child_process_id_to_pipe_, child_process_id));
 
   auto local_pipe = base::MakeUnique<base::SyncSocket>();
   auto child_pipe = base::MakeUnique<base::SyncSocket>();
   if (base::SyncSocket::CreatePair(local_pipe.get(), child_pipe.get())) {
     child_process_id_to_pipe_[child_process_id] = std::move(local_pipe);
     mappings->Transfer(kAndroidWebViewCrashSignalDescriptor,
                        base::ScopedFD(dup(child_pipe->handle())));
   }
 }
 
 void AwBrowserTerminator::ProcessTerminationStatus(
+    int child_process_id,
     std::unique_ptr<base::SyncSocket> pipe) {
+  bool crashed = false;
+
+  // If the child process hasn't written anything into the pipe. This implies
+  // that it was terminated via SIGKILL by the low memory killer.
   if (pipe->Peek() >= sizeof(int)) {
     int exit_code;
     pipe->Receive(&exit_code, sizeof(exit_code));
-    LOG(FATAL) << "Renderer process crash detected (code " << exit_code
+    LOG(ERROR) << "Renderer process crash detected (code " << exit_code
                << "). Terminating browser.";

[Torne, 2017/01/13 13:24:08]

We're not terminating the browser necessarily any more, can we update this log
message?

[michaelbai, 2017/01/13 21:01:38]

Done.

-  } else {
-    // The child process hasn't written anything into the pipe. This implies
-    // that it was terminated via SIGKILL by the low memory killer, and thus we
-    // need to perform a clean exit.
-    exit(0);
+    crashed = true;
   }
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&OnRenderProcessGoneWithDetail, child_process_id, crashed));
 }
 
 void AwBrowserTerminator::OnChildExit(
     int child_process_id,
     base::ProcessHandle pid,
     content::ProcessType process_type,
     base::TerminationStatus termination_status,
     base::android::ApplicationState app_state) {
+  OnRenderProcessGone(child_process_id);

[Tobias Sargeant, 2017/01/13 10:48:38]

Don't do this here; OnChildExit can be called more than once per child
(unavoidable because of the way the chrome child process notification system
works) so you need to move it to after the early-out when we don't find a pipe.

[michaelbai, 2017/01/13 21:01:38]

Thanks, it was added for experiment, I didn't think through it afterwards. 

   std::unique_ptr<base::SyncSocket> pipe;
 
   {
     base::AutoLock auto_lock(child_process_id_to_pipe_lock_);
     const auto& iter = child_process_id_to_pipe_.find(child_process_id);
     if (iter == child_process_id_to_pipe_.end()) {
       // We might get a NOTIFICATION_RENDERER_PROCESS_TERMINATED and a
       // NOTIFICATION_RENDERER_PROCESS_CLOSED.
       return;
     }
     pipe = std::move(iter->second);
     child_process_id_to_pipe_.erase(iter);
   }
   if (termination_status == base::TERMINATION_STATUS_NORMAL_TERMINATION)
     return;

[Tobias Sargeant, 2017/01/13 10:48:38]

You may need to organise for the *WithDetail delegate method to be called in
this case, although I think there's actually no such thing as a webview renderer
terminating normally.

[michaelbai, 2017/01/13 21:01:38]

Here, we don't know whether render process crashed or was killed.

   DCHECK(pipe->handle() != base::SyncSocket::kInvalidHandle);
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&AwBrowserTerminator::ProcessTerminationStatus,
+                 child_process_id,
                  base::Passed(std::move(pipe))));
 }
 
 }  // namespace breakpad