Ensure RenderViewHost is not shut down until a screenshot of the page is taken.

content/browser/renderer_host/render_view_host_impl.cc

Index: content/browser/renderer_host/render_view_host_impl.cc
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index e942a281cffce1162c11f819581336d49e3e70a1..d371542edcaa734ac6a8e6eee9d427ad66de5a7c 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -214,6 +214,8 @@ RenderViewHostImpl::RenderViewHostImpl(
       sudden_termination_allowed_(false),
       render_view_termination_status_(base::TERMINATION_STATUS_STILL_RUNNING),
       virtual_keyboard_requested_(false),
+      pending_shutdown_on_copy_requests_done_(false),
+      copy_requests_(0),
       weak_factory_(this) {
   DCHECK(instance_.get());
   CHECK(delegate_);  // http://crbug.com/82827
@@ -258,6 +260,12 @@ RenderViewHostImpl::~RenderViewHostImpl() {
   CrossSiteRequestManager::GetInstance()->SetHasPendingCrossSiteRequest(
       GetProcess()->GetID(), GetRoutingID(), false);
 
+  // The host shouldn't be destroyed when there is an outstanding copy request,
+  // otherwise the underlying layer would be destroyed underneath the copy
+  // request before it has a chance to complete.
+  // crbug.com/340682
+  DCHECK(copy_requests_ == 0);
+
   // If this was swapped out, it already decremented the active view
   // count of the SiteInstance it belongs to.
   if (IsRVHStateActive(rvh_state_))
@@ -647,7 +655,10 @@ void RenderViewHostImpl::OnSwappedOut(bool timed_out) {
       break;
     case STATE_PENDING_SHUTDOWN:
       DCHECK(!pending_shutdown_on_swap_out_.is_null());
-      pending_shutdown_on_swap_out_.Run();
+      if (copy_requests_ == 0)
+        pending_shutdown_on_swap_out_.Run();
+      else
+        pending_shutdown_on_copy_requests_done_ = true;
       break;
     default:
       NOTREACHED();
@@ -656,13 +667,28 @@ void RenderViewHostImpl::OnSwappedOut(bool timed_out) {
 
 void RenderViewHostImpl::WasSwappedOut(
     const base::Closure& pending_delete_on_swap_out) {
-  Send(new ViewMsg_WasSwappedOut(GetRoutingID()));
+  // Sending the ViewMsg_WasSwappedOut message would result in
+  // ChildProcessHostMsg_ShutdownRequest, which would lead to the destruction of
+  // the host and the cc layer. If there is an outstanding copy request, delay
+  // sending this message until it completes (see CopyFromBackingStoreCallback).
+  // crbug.com/340682
+  if (copy_requests_ == 0)
+    Send(new ViewMsg_WasSwappedOut(GetRoutingID()));
   if (rvh_state_ == STATE_WAITING_FOR_UNLOAD_ACK) {
     SetState(STATE_PENDING_SWAP_OUT);
-    if (!instance_->active_view_count())
+    if (!instance_->active_view_count()) {
       SetPendingShutdown(pending_delete_on_swap_out);
+    }
   } else if (rvh_state_ == STATE_WAITING_FOR_COMMIT) {
-    SetState(STATE_SWAPPED_OUT);
+    if (copy_requests_ == 0) {
+      SetState(STATE_SWAPPED_OUT);
+    } else {
+      // Let the copy request complete before swapping out the host so that it
+      // doesn't get destroyed. pending_delete_on_swap_out will be executed from
+      // CopyFromBackingStoreCallback when copy_requests_ goes down to 0.
+      // crbug.com/340682
+      SetPendingShutdown(pending_delete_on_swap_out);

[clamy, 2014/03/28 11:03:39]

There is still a problem with your solution here. Let's suppose that we have
more than one active_view_count in the SiteInstance. If one of them is swapped
out and hits this path, it will be set to PENDING_SHUTDOWN. And there is no way
to get back from PENDING_SHUTDOWN to SWAPPED_OUT, which is the state the RVH
should end up in when copy_requests_ goes down to 0. Not to mention that the
corresponding RFH has been taken out of the map of swapped out hosts.

+    }
   } else if (rvh_state_ == STATE_DEFAULT) {
     // When the RenderView is not live, the RenderFrameHostManager will call
     // CommitPending directly, without calling SwapOut on the old RVH. This will
@@ -1719,6 +1745,48 @@ void RenderViewHostImpl::ClearFocusedElement() {
   Send(new ViewMsg_ClearFocusedElement(GetRoutingID()));
 }
 
+void RenderViewHostImpl::CopyFromBackingStoreCallback(
+    const base::Callback<void(bool, const SkBitmap&)>& callback,
+    bool success,
+    const SkBitmap& bitmap) {
+  --copy_requests_;
+  callback.Run(success, bitmap);
+  if (copy_requests_ == 0) {
+    if (!IsRVHStateActive(rvh_state_)) {
+      // RVH was in an active state when |copy_requests_| was incremented, so it
+      // went into inactive state while |copy_requests_| was greater than 0.
+      // This means that WasSwappedOut()  was called, but sending of
+      // ViewMsg_WasSwappedOut message was delayed until all copy requests
+      // complete.
+      Send(new ViewMsg_WasSwappedOut(GetRoutingID()));
+    }
+
+    if (rvh_state_ == STATE_PENDING_SHUTDOWN &&
+        pending_shutdown_on_copy_requests_done_) {
+      DCHECK(!pending_shutdown_on_swap_out_.is_null());
+      pending_shutdown_on_swap_out_.Run();
+    }
+  }
+}
+
+void RenderViewHostImpl::CopyFromBackingStore(
+    const gfx::Rect& src_rect,
+    const gfx::Size& accelerated_dst_size,
+    const base::Callback<void(bool, const SkBitmap&)>& callback,
+    const SkBitmap::Config& bitmap_config) {
+  DCHECK(IsRVHStateActive(rvh_state_));
+  if (!IsRVHStateActive(rvh_state_)) {
+    callback.Run(false, SkBitmap());
+    return;
+  }
+  ++copy_requests_;
+  RenderWidgetHostImpl::CopyFromBackingStore(src_rect, accelerated_dst_size,
+      base::Bind(&RenderViewHostImpl::CopyFromBackingStoreCallback,
+                 weak_factory_.GetWeakPtr(),
+                 callback),
+                 bitmap_config);
+}
+
 void RenderViewHostImpl::Zoom(PageZoom zoom) {
   Send(new ViewMsg_Zoom(GetRoutingID(), zoom));
 }