Fix re-entrancy caused by RenderFrameHostImpl::DispatchBeforeUnload.

The method, on certain conditions, would simulate receiving a
OnBeforeUnloadACK() IPC. This would immediately call back into the unload
handler, which had just called RenderFrameHostImpl::DispatchBeforeUnload.

The problematic series of events is as follows:
  1) TabStripModel::CloseWebContentses tries to close many tabs.
  2) This calls RunUnloadListenerBeforeClosing() on each tab.
  3) UnloadListener will call RenderFrameHostImpl::DispatchBeforeUnload, which
     on certain conditions will synchronously call OnBeforeUnloadACK().
  4) This calls back into TabStripModel, mutating internal state.
  5) The implementation of TabStripModel::CloseWebContentses relies on the
     assumption that there is no re-entrancy. This manifests itself as a buffer
     overrun when (1) calls SendDetachWebContentsNotifications, which then
     passes inconsistent data to the observers.

Bug: 851400
Change-Id: I2080875f5d8d118c93addbe66503cb02b8b2966f
Reviewed-on: https://chromium-review.googlesource.com/1101846
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568534}

content/browser/frame_host/render_frame_host_impl.cc

@@ -23,6 +23,7 @@
 #include "base/numerics/safe_conversions.h"
 #include "base/process/kill.h"
 #include "base/task_scheduler/post_task.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "cc/base/switches.h"
@@ -3458,8 +3459,17 @@ void RenderFrameHostImpl::DispatchBeforeUnload(bool for_navigation,
   // the handler ran and allowed the navigation to proceed.
   if (!ShouldDispatchBeforeUnload()) {
     DCHECK(!for_navigation);
-    frame_tree_node_->render_manager()->OnBeforeUnloadACK(
-        true, base::TimeTicks::Now());
+
+    // Dispatch the ACK to prevent re-entrancy.
+    base::OnceClosure task = base::BindOnce(
+        [](base::WeakPtr<RenderFrameHostImpl> self) {
+          if (!self)
+            return;
+          self->frame_tree_node_->render_manager()->OnBeforeUnloadACK(
+              true, base::TimeTicks::Now());
+        },
+        weak_ptr_factory_.GetWeakPtr());
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, std::move(task));
     return;
   }
   TRACE_EVENT_ASYNC_BEGIN1("navigation", "RenderFrameHostImpl BeforeUnload",
@@ -3501,7 +3511,13 @@ void RenderFrameHostImpl::DispatchBeforeUnload(bool for_navigation,
 void RenderFrameHostImpl::SimulateBeforeUnloadAck() {
   DCHECK(is_waiting_for_beforeunload_ack_);
   base::TimeTicks approx_renderer_start_time = send_before_unload_start_time_;
-  OnBeforeUnloadACK(true, approx_renderer_start_time, base::TimeTicks::Now());
+
+  // Dispatch the ACK to prevent re-entrancy.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&RenderFrameHostImpl::OnBeforeUnloadACK,
+                     weak_ptr_factory_.GetWeakPtr(), true,
+                     approx_renderer_start_time, base::TimeTicks::Now()));
 }
 
 bool RenderFrameHostImpl::ShouldDispatchBeforeUnload() {