Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(108)

Unified Diff: content/browser/renderer_host/render_view_host_impl.h

Issue 88503002: Have the unload event execute in background on cross-site navigations (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Rebase + addressed some of Nasko's comments Created 6 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/renderer_host/render_view_host_impl.h
diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
index 52f36ea55d0886212f23bfd47401ced3a7540508..55314cbf22a3aed5108d9ec713f0fcf1a10b6437 100644
--- a/content/browser/renderer_host/render_view_host_impl.h
+++ b/content/browser/renderer_host/render_view_host_impl.h
@@ -9,6 +9,7 @@
#include <string>
#include <vector>
+#include "base/callback.h"
#include "base/compiler_specific.h"
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
@@ -64,6 +65,7 @@ class RenderWidgetHostDelegate;
class SessionStorageNamespace;
class SessionStorageNamespaceImpl;
class TestRenderViewHost;
+class TimeoutMonitor;
struct ContextMenuParams;
struct FileChooserParams;
struct Referrer;
@@ -99,6 +101,37 @@ class CONTENT_EXPORT RenderViewHostImpl
: public RenderViewHost,
public RenderWidgetHostImpl {
public:
+ // Keeps track of the state of the RenderViewHostImpl, more particularly
+ // during swap out.
+ enum RenderViewHostImplState {
+ // The standard state for a RVH handling the communication with a
+ // RenderView.
+ STATE_LIVE = 0,
+ // The RVH has sent the SwapOut request to the renderer, but has not
+ // received the SwapOutACK yet. The new page has not been committed yet
+ // either.
+ STATE_WAITING_FOR_UNLOAD_ACK,
+ // The RVH received the SwapOutACK from the RenderView, but the new page has
+ // not been committed yet.
+ STATE_WAITING_FOR_COMMIT,
+ // The RVH is waiting for the CloseACK from the RenderView.
+ STATE_WAITING_FOR_CLOSE,
+ // The RVH has not received the SwapOutACK yet, but the new page has
+ // committed. The number of active views of the RVH SiteInstanceImpl is not
+ // zero. Upon reeception of the SwapOutACK, the RVH will be swapped out.
nasko 2014/01/27 19:09:51 nit: "reeception" < extra 'e'?
clamy 2014/01/28 12:27:21 Done.
+ STATE_PENDING_SWAP_OUT,
+ // The RVH has not received the SwapOutACK yet, but the new page has
+ // committed. The number of active views of the RVH SiteInstanceImpl is
+ // zero. Upon reeception of the SwapOutACK, the RVH will be shutdown.
nasko 2014/01/27 19:09:51 nit: same as above, extra 'e'
clamy 2014/01/28 12:27:21 Done.
+ STATE_PENDING_SHUTDOWN,
+ // The RVH is swapped out, it is being used as a placeholder to allow for
+ // cross-process communication.
+ STATE_SWAPPED_OUT,
+ };
+ // Helper function to determine whether the RVH state should contribute to the
+ // number of active views of a SiteInstance or not.
+ static bool IsRVHStateActive(RenderViewHostImplState rvh_state);
+
// Convenience function, just like RenderViewHost::FromID.
static RenderViewHostImpl* FromID(int render_process_id, int render_view_id);
@@ -282,9 +315,8 @@ class CONTENT_EXPORT RenderViewHostImpl
return has_accessed_initial_document_;
}
- // Whether this RenderViewHost has been swapped out to be displayed by a
- // different process.
- bool is_swapped_out() const { return is_swapped_out_; }
+ // The current state of this RVH.
+ RenderViewHostImplState rvh_state() const { return rvh_state_; }
// Called on the pending RenderViewHost when the network response is ready to
// commit. We should ensure that the old RenderViewHost runs its unload
@@ -317,11 +349,15 @@ class CONTENT_EXPORT RenderViewHostImpl
// out.
void OnSwappedOut(bool timed_out);
- // Called to notify the renderer that it has been visibly swapped out and
- // replaced by another RenderViewHost, after an earlier call to SwapOut.
- // It is now safe for the process to exit if there are no other active
- // RenderViews.
- void WasSwappedOut();
+ // Called when the RenderFrameHostManager has swapped in a new
+ // RenderFrameHost. Should |this| RVH switch to the pending shutdown state,
+ // |pending_delete_on_swap_out| will be executed upon reception of the
+ // SwapOutACK, or when the unload timer times out.
+ void WasSwappedOut(const base::Closure& pending_delete_on_swap_out);
+
+ // Set |this| as pending shutdown. |on_swap_out| will be called
+ // when the swap out ack is received, or when the unload timer times out.
+ void SetPendingShutdown(const base::Closure& on_swap_out);
// Close the page ignoring whether it has unload events registers.
// This is called after the beforeunload and unload events have fired
@@ -455,9 +491,8 @@ class CONTENT_EXPORT RenderViewHostImpl
return is_waiting_for_beforeunload_ack_;
}
- bool is_waiting_for_unload_ack() {
- return is_waiting_for_unload_ack_;
- }
+ // Whether the RVH is waiting for the unload ack from the renderer.
+ bool IsWaitingForUnloadACK() const;
// Returns whether the given URL is allowed to commit in the current process.
// This is a more conservative check than RenderProcessHost::FilterURL, since
@@ -481,6 +516,18 @@ class CONTENT_EXPORT RenderViewHostImpl
bool main_frame,
const GURL& url);
+ // Increases the refcounting on this RVH. This is done by the FrameTree on
+ // creation of a RenderFrameHost.
+ void increment_ref_count() { ++frames_ref_count_; }
+
+ // Decreases the refcounting on this RVH. This is done by the FrameTree on
+ // destruction of a RenderFrameHost.
+ void decrement_ref_count() { --frames_ref_count_; }
+
+ // Returns the refcount on this RVH, that is the number of RenderFrameHosts
+ // currently using it.
+ int ref_count() { return frames_ref_count_; }
+
// NOTE: Do not add functions that just send an IPC message that are called in
// one or two places. Have the caller send the IPC message directly (unless
// the caller places are in different platforms, in which case it's better
@@ -597,12 +644,15 @@ class CONTENT_EXPORT RenderViewHostImpl
FRIEND_TEST_ALL_PREFIXES(RenderViewHostTest, BasicRenderFrameHost);
FRIEND_TEST_ALL_PREFIXES(RenderViewHostTest, RoutingIdSane);
- // Sets whether this RenderViewHost is swapped out in favor of another,
- // and clears any waiting state that is no longer relevant.
- void SetSwappedOut(bool is_swapped_out);
+ // Updates the state of this RenderViewHost and clears any waiting state
+ // that is no longer relevant.
+ void SetState(RenderViewHostImplState rvh_state);
bool CanAccessFilesOfPageState(const PageState& state) const;
+ // The number of RenderFrameHost which have a reference to this RVH.
+ int frames_ref_count_;
+
// Our delegate, which wants to know about changes in the RenderView.
RenderViewHostDelegate* delegate_;
@@ -637,9 +687,8 @@ class CONTENT_EXPORT RenderViewHostImpl
// first commit.
bool has_accessed_initial_document_;
- // Whether this RenderViewHost is currently swapped out, such that the view is
- // being rendered by another process.
- bool is_swapped_out_;
+ // The current state of this RVH.
+ RenderViewHostImplState rvh_state_;
// The frame id of the main (top level) frame. This value is set on the
// initial navigation of a RenderView and reset when the RenderView's
@@ -658,20 +707,13 @@ class CONTENT_EXPORT RenderViewHostImpl
// Set to true when there is a pending ViewMsg_ShouldClose message. This
// ensures we don't spam the renderer with multiple beforeunload requests.
- // When either this value or is_waiting_for_unload_ack_ is true, the value of
+ // When either this value or IsWaitingForUnloadACK is true, the value of
// unload_ack_is_for_cross_site_transition_ indicates whether this is for a
// cross-site transition or a tab close attempt.
bool is_waiting_for_beforeunload_ack_;
- // Set to true when there is a pending ViewMsg_Close message. Also see
- // is_waiting_for_beforeunload_ack_, unload_ack_is_for_cross_site_transition_.
- bool is_waiting_for_unload_ack_;
-
- // Set to true when waiting for ViewHostMsg_SwapOut_ACK has timed out.
- bool has_timed_out_on_unload_;
-
// Valid only when is_waiting_for_beforeunload_ack_ or
- // is_waiting_for_unload_ack_ is true. This tells us if the unload request
+ // IsWaitingForUnloadACK is true. This tells us if the unload request
// is for closing the entire tab ( = false), or only this RenderViewHost in
// the case of a cross-site transition ( = true).
bool unload_ack_is_for_cross_site_transition_;
@@ -706,6 +748,16 @@ class CONTENT_EXPORT RenderViewHostImpl
scoped_ptr<BrowserMediaPlayerManager> media_player_manager_;
#endif
+ // Used to shutdown this RVH when the unload event is taking too long to
+ // execute.
+ scoped_ptr<TimeoutMonitor> unload_event_monitor_timeout_;
+
+ // Called after receiving the swap out ack when the RVH is in state pending
+ // shutdown. Also called if the unload timer times out.
+ base::Closure pending_shutdown_on_swap_out_;
+
+ base::WeakPtrFactory<RenderViewHostImpl> weak_factory_;
+
DISALLOW_COPY_AND_ASSIGN(RenderViewHostImpl);
};

Powered by Google App Engine
This is Rietveld 408576698