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 690ccef2b4ee079989385e6a42d7e5441874ac18..e4456718241b310f233ee9a31e20f72284d5be48 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -121,7 +121,7 @@ class CONTENT_EXPORT RenderProcessHostImpl
   void Cleanup() override;
   void AddPendingView() override;
   void RemovePendingView() override;
-  void SetSuddenTerminationAllowed(bool enabled) override;
+  void SuddenTerminationChangedForFrame(bool enabled) override;
   bool SuddenTerminationAllowed() const override;
   IPC::ChannelProxy* GetChannel() override;
   void AddFilter(BrowserMessageFilter* filter) override;
@@ -311,7 +311,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 +412,21 @@ 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_;
+  // Number of frames that have disallowed sudden termination.  If this is 0,
+  // then we're sure that all the RenderFrames in the process can be shutdown
+  // suddenly.  If it's not 0, then specific RenderFrames 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.
+  int sudden_termination_disallows_;
+
+  // Whether the sudden termination of the process was enabled on a process
+  // level. This is independant from the status of the various RenderFrames in
+  // 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.
+  bool sudden_termination_enabled_;

[clamy, 2015/01/20 16:54:15]

After discussing with michaeln@ it seems we cannot completely switch to tracking
the sudden termination status per RenderFrame, and still have to disable it per
process for the DOM local storage. I'll update the design doc accordingly.

[Charlie Reis, 2015/01/23 06:01:32]

That's fine, but I don't understand why we're adding a count of frames here. 
RPH should just track the presence of DOM storage callbacks, and
RFH::SuddenTerminationAllowed() can include a call to
GetProcess()->SuddenTerminationAllowed(), as we used to do in RVH.

Keeping a count in this class feels overly complex and bug-prone to me, after
fighting a long string of bugs with a similar counter for DidStartLoading (see
https://crbug.com/429399).

[clamy, 2015/01/26 16:38:40]

I removed the count of frames that have sudden termination disallowed. My main
concern is that the RenderProcessHostImpl may attempt to shutdown fastly even
though there are some handlers to run (in FastShutdownIfPossible
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...).
Is there a handy way for the RPHI to access all its associated RFH (and then
check their sudden termination status)? I couldn't find one, which is why I
introduced the tracking in the first place.

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

I think my comments in WebContentsImpl::NeedToFireBeforeUnload and
RenderProcessHostImpl::FastShutdownIfPossible might resolve this.

At a high level:
RPH::SuddenTerminationAllowed() already returns true if any frame has either
type of handler, or if there's a DOM storage callback.  It shouldn't need to ask
the frames individually.

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