Remove RenderFrameHostImplState and convert it to boolean.

Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c
Cr-Commit-Position: refs/heads/master@{#386232}

[nasko, 2016-04-01 22:37:49]

Description was changed from

==========
Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
==========

to

==========
Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[nasko, 2016-04-01 22:58:58]

nasko@chromium.org changed reviewers:
+ creis@chromium.org

[nasko, 2016-04-01 22:58:59]

Hey Charlie,
Can you review this CL for me? It cleans up the state enum from RenderFrameHost
and turns it into a boolean.
Thanks in advance!
Nasko

[nasko, 2016-04-01 22:59:20]

Description was changed from

==========
Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[Charlie Reis, 2016-04-04 17:16:36]

Cool, thanks!  Some questions and nits below.

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:1165: // If this
RenderFrameHost is not in the default state, it must have already
s/not in the default state/already pending deletion/

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:1190: // simulating the
receipt of swap out ack?
Yes, that sounds right.  Want to do that here or in a separate CL?

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:1894: if (!is_active())
How can is_active() be false in this call?  Wouldn't that make lines 1890-1891
wrong?

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.h:337: // The current state of
this RFH.
This comment needs to be updated to say precisely what is_active() means
(especially since RenderViewHostImpl has a similar method).

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.h:786: // is waiting for
FrameHostMsg_SwapOut_ACK and pending deletion.
nit: and thus pending deletion.

[nasko, 2016-04-08 17:42:07]

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:1165: // If this
RenderFrameHost is not in the default state, it must have already
On 2016/04/04 17:16:36, Charlie Reis (slow til 4-8) wrote:
> s/not in the default state/already pending deletion/

Done.

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:1190: // simulating the
receipt of swap out ack?
On 2016/04/04 17:16:36, Charlie Reis (slow til 4-8) wrote:
> Yes, that sounds right.  Want to do that here or in a separate CL?

Separate CL, unless you feel strongly for including it here.

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:1894: if (!is_active())
On 2016/04/04 17:16:36, Charlie Reis (slow til 4-8) wrote:
> How can is_active() be false in this call?  Wouldn't that make lines 1890-1891
> wrong?

Done.

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.h:337: // The current state of
this RFH.
On 2016/04/04 17:16:36, Charlie Reis wrote:
> This comment needs to be updated to say precisely what is_active() means
> (especially since RenderViewHostImpl has a similar method).

Done.

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.h:786: // is waiting for
FrameHostMsg_SwapOut_ACK and pending deletion.
On 2016/04/04 17:16:36, Charlie Reis wrote:
> nit: and thus pending deletion.

Done.

[Charlie Reis, 2016-04-08 20:25:37]

Thanks!  LGTM with nits.

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1849343004/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:1190: // simulating the
receipt of swap out ack?
On 2016/04/08 17:42:07, nasko (very slow Apr 8-18) wrote:
> On 2016/04/04 17:16:36, Charlie Reis (slow til 4-8) wrote:
> > Yes, that sounds right.  Want to do that here or in a separate CL?
> 
> Separate CL, unless you feel strongly for including it here.

Acknowledged.

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:1200: // simulating the
receipt of swap out ack?
Since we know we want to this, let's rephrase into a statement:

s/shouldn't we/we should/

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:1921: void
RenderFrameHostImpl::ResetWaitingState() {
Let's put a DCHECK(is_active()) just to catch anyone who adds a bad call to
this.

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:2004: // completing a RFH
swap or unload handler.
nit: without completing an unload handler.

(I do wonder whether this case is dead code now, but we can leave it for now.)

Same in the comments below.

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.h:349: // deletion.
Just some wordsmithing:

This method returns true from the time this RenderFrameHost is created until
SwapOut is called, at which point it is pending deletion.

[nasko, 2016-04-08 21:25:33]

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:1200: // simulating the
receipt of swap out ack?
On 2016/04/08 20:25:37, Charlie Reis wrote:
> Since we know we want to this, let's rephrase into a statement:
> 
> s/shouldn't we/we should/

Done.

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:1921: void
RenderFrameHostImpl::ResetWaitingState() {
On 2016/04/08 20:25:37, Charlie Reis wrote:
> Let's put a DCHECK(is_active()) just to catch anyone who adds a bad call to
> this.

Done.

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:2004: // completing a RFH
swap or unload handler.
On 2016/04/08 20:25:37, Charlie Reis wrote:
> nit: without completing an unload handler.
> 
> (I do wonder whether this case is dead code now, but we can leave it for now.)
> 
> Same in the comments below.

Done.

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/1849343004/diff/40001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.h:349: // deletion.
On 2016/04/08 20:25:37, Charlie Reis wrote:
> Just some wordsmithing:
> 
> This method returns true from the time this RenderFrameHost is created until
> SwapOut is called, at which point it is pending deletion.

Done.

[nasko, 2016-04-08 21:25:36]

The CQ bit was checked by nasko@chromium.org

[nasko, 2016-04-08 21:25:37]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/1849343004/#ps60001
(title: "Addressing nits.")

[commit-bot: I haz the power, 2016-04-08 21:25:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849343004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849343004/60001

[commit-bot: I haz the power, 2016-04-08 22:39:01]

Description was changed from

==========
Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-04-08 22:39:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-08 22:41:26]

Description was changed from

==========
Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG=357747
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c
Cr-Commit-Position: refs/heads/master@{#386232}
==========

[commit-bot: I haz the power, 2016-04-08 22:41:27]

Patchset 4 (id:??) landed as
https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c
Cr-Commit-Position: refs/heads/master@{#386232}