Refactor sudden termination

content/browser/renderer_host/render_process_host_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_RENDERER_HOST_RENDER_PROCESS_HOST_IMPL_H_
 #define CONTENT_BROWSER_RENDERER_HOST_RENDER_PROCESS_HOST_IMPL_H_
 
 #include <map>
 #include <queue>
 #include <string>
@@ skipping 103 matching lines @@
   base::ProcessHandle GetHandle() const override;
   BrowserContext* GetBrowserContext() const override;
   bool InSameStoragePartition(StoragePartition* partition) const override;
   int GetID() const override;
   bool HasConnection() const override;
   void SetIgnoreInputEvents(bool ignore_input_events) override;
   bool IgnoreInputEvents() const override;
   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;
   bool FastShutdownForPageCount(size_t count) override;
   bool FastShutdownStarted() const override;
   base::TimeDelta GetChildProcessIdleTime() const override;
   void ResumeRequestsForView(int route_id) override;
   void FilterURL(bool empty_allowed, GURL* url) override;
 #if defined(ENABLE_WEBRTC)
   void EnableAecDump(const base::FilePath& file) override;
@@ skipping 169 matching lines @@
 
   // Creates and adds the IO thread message filters.
   void CreateMessageFilters();
 
   // Registers Mojo services to be exposed to the renderer.
   void RegisterMojoServices();
 
   // 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);
 
   // Generates a command line to be used to spawn a renderer and appends the
   // results to |*command_line|.
   void AppendRendererCommandLine(base::CommandLine* command_line) const;
 
   // Copies applicable command line switches from the given |browser_cmd| line
   // flags to the output |renderer_cmd| line flags. Not all switches will be
@@ skipping 80 matching lines @@
   int id_;
 
   BrowserContext* browser_context_;
 
   // Owned by |browser_context_|.
   StoragePartitionImpl* storage_partition_impl_;
 
   // 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.
   bool ignore_input_events_;
 
   // Records the last time we regarded the child process active.
   base::TimeTicks child_process_activity_time_;
 
   // Indicates whether this is a RenderProcessHost of a Browser Plugin guest
   // renderer.
@@ skipping 62 matching lines @@
   bool subscribe_uniform_enabled_;
 
   base::WeakPtrFactory<RenderProcessHostImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderProcessHostImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_RENDER_PROCESS_HOST_IMPL_H_