Refactor sudden termination

content/browser/renderer_host/render_view_host_impl.cc

 // Copyright (c) 2012 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/browser/renderer_host/render_view_host_impl.h"
 
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 107 matching lines @@
           base::Bind(base::IgnoreResult(&DismissVirtualKeyboardTask)),
           TimeDelta::FromMilliseconds(kVirtualKeyboardDisplayWaitTimeoutMs));
       ++virtual_keyboard_display_retries;
     } else {
       virtual_keyboard_display_retries = 0;
     }
   }
 }
 #endif
 
+// Enable sudden termination for the current RenderFrameHost of
+// |frame_tree_node| if its SiteInstance is |site_instance|.  Used with
+// FrameTree::ForEach.
+bool EnableSuddenTermination(scoped_refptr<SiteInstance> site_instance,

[clamy, 2015/02/03 12:54:03]

Note that since SiteInstance is refcounted, we need to have a scoped_refptr as
an argument of the function to be able to bind it.

[Charlie Reis, 2015/02/03 19:33:15]

I don't think that's true, is it?  It only needs to be a scoped_refptr in cases
that ownership is involved.

There's no chance that the SiteInstance will be deleted in this function, since
it's purely used for comparison and there aren't any actions here that could
cause it to go away.

We should just pass a raw pointer to it from the call sites.

[clamy, 2015/02/04 13:16:19]

There is a static assert in base::Bind preventing the code to compile if trying
to bind a raw pointer of a ref_counted class
(https://code.google.com/p/chromium/codesearch#chromium/src/base/bind.h&l=100&...).
I agree that the scoped_refptr makes no sense from an ownership point of view (I
had written the raw pointer version before hitting the compile assert). It seems
that SiteInstance has a unique ID, so I went with comparing that instead.
Another option would be to have SiteInstance support weak pointers and bind a
weak pointer but I don't think that's desirable.

[Charlie Reis, 2015/02/04 21:34:57]

Ah, base::Bind...  Sorry for missing that aspect before.

Yes, using the SiteInstance ID seems like a reasonable workaround here.

+                             FrameTreeNode* frame_tree_node) {
+  if (frame_tree_node->current_frame_host()->GetSiteInstance()
+      == site_instance.get()) {
+    frame_tree_node->current_frame_host()
+        ->set_override_sudden_termination_status(true);
+  }
+  return true;
+}
+
 }  // namespace
 
 // static
 const int64 RenderViewHostImpl::kUnloadTimeoutMS = 1000;
 
 ///////////////////////////////////////////////////////////////////////////////
 // RenderViewHost, public:
 
 // static
 RenderViewHost* RenderViewHost::FromID(int render_process_id,
@@ skipping 38 matching lines @@
       instance_(static_cast<SiteInstanceImpl*>(instance)),
       waiting_for_drag_context_response_(false),
       enabled_bindings_(0),
       page_id_(-1),
       is_active_(!swapped_out),
       is_swapped_out_(swapped_out),
       main_frame_routing_id_(main_frame_routing_id),
       run_modal_reply_msg_(NULL),
       run_modal_opener_id_(MSG_ROUTING_NONE),
       is_waiting_for_close_ack_(false),
-      sudden_termination_allowed_(false),
       render_view_termination_status_(base::TERMINATION_STATUS_STILL_RUNNING),
       virtual_keyboard_requested_(false),
       is_focused_element_editable_(false),
       updating_web_preferences_(false),
       weak_factory_(this) {
   DCHECK(instance_.get());
   CHECK(delegate_);  // http://crbug.com/82827
 
   GetProcess()->EnableSendQueue();
 
@@ skipping 351 matching lines @@
     // This RenderViewHost doesn't have a live renderer, so just skip the unload
     // event and close the page.
     ClosePageIgnoringUnloadEvents();
   }
 }
 
 void RenderViewHostImpl::ClosePageIgnoringUnloadEvents() {
   StopHangMonitorTimeout();
   is_waiting_for_close_ack_ = false;
 
-  sudden_termination_allowed_ = true;
+  // Enable sudden termination for all RenderFrameHosts in the FrameTree.
+  FrameTree* frame_tree = static_cast<RenderFrameHostImpl*>(GetMainFrame())
+      ->frame_tree_node()->frame_tree();
+  frame_tree->ForEach(base::Bind(&EnableSuddenTermination, instance_));

[Charlie Reis, 2015/02/03 19:33:15]

instance_.get()

   delegate_->Close(this);
 }
 
 #if defined(OS_ANDROID)
 void RenderViewHostImpl::ActivateNearestFindResult(int request_id,
                                                    float x,
                                                    float y) {
   Send(new InputMsg_ActivateNearestFindResult(GetRoutingID(),
                                               request_id, x, y));
 }
@@ skipping 270 matching lines @@
 }
 
 void RenderViewHostImpl::LoadStateChanged(
     const GURL& url,
     const net::LoadStateWithParam& load_state,
     uint64 upload_position,
     uint64 upload_size) {
   delegate_->LoadStateChanged(url, load_state, upload_position, upload_size);
 }
 
-bool RenderViewHostImpl::SuddenTerminationAllowed() const {
-  return sudden_termination_allowed_ ||
-      GetProcess()->SuddenTerminationAllowed();
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // RenderViewHostImpl, IPC message handlers:
 
 bool RenderViewHostImpl::OnMessageReceived(const IPC::Message& msg) {
   if (!BrowserMessageFilter::CheckCanDispatchOnUI(msg, this))
     return true;
 
   // Filter out most IPC messages if this renderer is swapped out.
   // We still want to handle certain ACKs to keep our state consistent.
   if (is_swapped_out_) {
@@ skipping 608 matching lines @@
   FrameTree* frame_tree = delegate_->GetFrameTree();
 
   frame_tree->ResetForMainFrameSwap();
 }
 
 void RenderViewHostImpl::SelectWordAroundCaret() {
   Send(new ViewMsg_SelectWordAroundCaret(GetRoutingID()));
 }
 
 }  // namespace content