Refactor sudden termination

content/browser/renderer_host/render_process_host_impl.h

Index: content/browser/renderer_host/render_process_host_impl.h
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h
index ac87d9c1a42d3cc79408ea6ba21e84e9cffd9abe..8b0b3bc90eab81a96923647b8704e87d547529fa 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -121,7 +121,6 @@ class CONTENT_EXPORT RenderProcessHostImpl
   void Cleanup() override;
   void AddPendingView() override;
   void RemovePendingView() override;
-  void SetSuddenTerminationAllowed(bool enabled) override;
   bool SuddenTerminationAllowed() const override;
   IPC::ChannelProxy* GetChannel() override;
   void AddFilter(BrowserMessageFilter* filter) override;
@@ -311,7 +310,7 @@ class CONTENT_EXPORT RenderProcessHostImpl
   // Control message handlers.
   void OnShutdownRequest();
   void OnDumpHandlesDone();
-  void SuddenTerminationChanged(bool enabled);
+  void OnSuddenTerminationChanged(bool enabled);
   void OnUserMetricsRecordAction(const std::string& action);
   void OnSavedPageAsMHTML(int job_id, int64 mhtml_file_size);
   void OnCloseACK(int old_route_id);
@@ -412,13 +411,13 @@ class CONTENT_EXPORT RenderProcessHostImpl
   // The observers watching our lifetime.
   ObserverList<RenderProcessHostObserver> observers_;
 
-  // True if the process can be shut down suddenly.  If this is true, then we're
-  // sure that all the RenderViews in the process can be shutdown suddenly.  If
-  // it's false, then specific RenderViews might still be allowed to be shutdown
-  // suddenly by checking their SuddenTerminationAllowed() flag.  This can occur
-  // if one WebContents has an unload event listener but another WebContents in
-  // the same process doesn't.
-  bool sudden_termination_allowed_;
+  // Whether the sudden termination of the process was enabled on a process

[Charlie Reis, 2015/01/29 23:20:41]

was enabled on -> is allowed at

[clamy, 2015/02/02 16:16:27]

Done.

+  // level. This is independant from the status of the various RenderFrames in

[Charlie Reis, 2015/01/29 23:20:41]

independent

[clamy, 2015/02/02 16:16:27]

Done.

+  // the process: even if this is false, some RenderFrames may be shutdown
+  // suddenly if their SuddenTerminationAllowed() flag is true. Similarly, if it
+  // is true, but some of the RenderFrames cannot be shutdown suddenly, then the
+  // process cannot be suddenly terminated.

[Charlie Reis, 2015/01/29 23:20:41]

I don't think this comment is accurate, if I'm following things correctly.  This
flag tracks *all* of the beforeunload, unload, and non-frame-specific (i.e., DOM
storage callback) cases.

See RendererBlinkPlatformImpl::suddenTerminationChanged, and the fact that
LocalDOMWindow::enableSuddenTermination calls it.  That means that the presence
of beforeunload handlers is tracked both in RFH and RPH.

We should make that clear here.

[clamy, 2015/02/02 16:16:27]

My plan was to get rid of the calls to
RendererBlinkPlatformImpl::suddenTerminationChanged, in order not to have the
same thing tracked twice in RFH and RPH. If it turns out it is not possible I'll
update the comment.

+  bool sudden_termination_enabled_;

[Charlie Reis, 2015/01/29 23:20:41]

Let's stick with allowed rather than enabled, since they're basically equivalent
and we don't need the churn.

[clamy, 2015/02/02 16:16:27]

Done.

 
   // Set to true if we shouldn't send input events.  We actually do the
   // filtering for this at the render widget level.