Expose the visibility state of a frame to the browser process.

Expose the visibility state of a frame to the browser process.

A frame knows whether it is visible or hidden based on its
RenderWidgetHost. In addition, a frame can be prerendered. To have that
information, it is required to go via ContentBrowserClient.

BUG=450634
Committed: https://crrev.com/f8c23a200068daeaac3139c59c9868b5a6058f75
Cr-Commit-Position: refs/heads/master@{#313264}

[mlamouri (slow - plz ping), 2015-01-22 16:36:28]

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

[mlamouri (slow - plz ping), 2015-01-22 18:18:58]

Taking into account your comments in https://codereview.chromium.org/860393004,
I've removed GetVisibilityState() from RenderFrameHost.

PTAL :)

[Charlie Reis, 2015-01-22 21:03:55]

https://codereview.chromium.org/871443004/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/871443004/diff/20001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_impl.cc:1485: ?
blink::WebPageVisibilityStateHidden
This indent doesn't seem right.  I would have done 4 spaces past "return", but
I've also seen 4 spaces past "GetRenderWidgetHost()" from clang-format.  This is
3 spaces past.

https://codereview.chromium.org/871443004/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/871443004/diff/20001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_impl.h:401: // of a frame a define
in Blink.
nit: are defined

https://codereview.chromium.org/871443004/diff/20001/content/public/browser/c...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/871443004/diff/20001/content/public/browser/c...
content/public/browser/content_browser_client.h:592: virtual bool
IsPrerendering(RenderFrameHost* render_frame_host);
Prerendering is not a concept in content/, so we should name this something
else.  Are you just trying to get the ContentBrowserClient to participate in the
decision about visibility?

https://codereview.chromium.org/871443004/diff/20001/content/public/browser/r...
File content/public/browser/render_frame_host.h (right):

https://codereview.chromium.org/871443004/diff/20001/content/public/browser/r...
content/public/browser/render_frame_host.h:14: #include
"third_party/WebKit/public/platform/WebPageVisibilityState.h"
Unnecessary?

[mlamouri (slow - plz ping), 2015-01-23 12:32:58]

I've updated the ContentBrowserClient to look slightly like
ContentRendererClient except that the override method isn't about
"ShouldOverride" but "Override". It's not clear to me why it was needed to have
this returned boolean.

Also, I'm hitting the same bug as in https://codereview.chromium.org/860393004

https://codereview.chromium.org/871443004/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/871443004/diff/20001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_impl.cc:1485: ?
blink::WebPageVisibilityStateHidden
On 2015/01/22 at 21:03:55, Charlie Reis (slow until 1-29) wrote:
> This indent doesn't seem right.  I would have done 4 spaces past "return", but
I've also seen 4 spaces past "GetRenderWidgetHost()" from clang-format.  This is
3 spaces past.

Fixed.

https://codereview.chromium.org/871443004/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/871443004/diff/20001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_impl.h:401: // of a frame a define
in Blink.
On 2015/01/22 at 21:03:55, Charlie Reis (slow until 1-29) wrote:
> nit: are defined

Sorry for those typos :(

https://codereview.chromium.org/871443004/diff/20001/content/public/browser/c...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/871443004/diff/20001/content/public/browser/c...
content/public/browser/content_browser_client.h:592: virtual bool
IsPrerendering(RenderFrameHost* render_frame_host);
On 2015/01/22 at 21:03:55, Charlie Reis (slow until 1-29) wrote:
> Prerendering is not a concept in content/, so we should name this something
else.  Are you just trying to get the ContentBrowserClient to participate in the
decision about visibility?

Oh, ok. I was wondering why ContentRenderClient wasn't explicit about
"prerender" status. I am a bit surprised that content/ doesn't know about
prerendering given that blink/ has a WebPageVisibilityPrerender status and we
need content/ to set that I guess.
Anyway, I will mirror this to what ContentRenderClient does then.

https://codereview.chromium.org/871443004/diff/20001/content/public/browser/r...
File content/public/browser/render_frame_host.h (right):

https://codereview.chromium.org/871443004/diff/20001/content/public/browser/r...
content/public/browser/render_frame_host.h:14: #include
"third_party/WebKit/public/platform/WebPageVisibilityState.h"
On 2015/01/22 at 21:03:55, Charlie Reis (slow until 1-29) wrote:
> Unnecessary?

Right. Leftovers from moving to RFH to RFHI.

[mlamouri (slow - plz ping), 2015-01-23 17:57:23]

I've updated to use ::GetView(). PTAL.

[mlamouri (slow - plz ping), 2015-01-26 13:19:52]

mlamouri@chromium.org changed reviewers:
+ jochen@chromium.org

[mlamouri (slow - plz ping), 2015-01-26 13:19:53]

Jochen, given the site isolation week happening this week, creis@ might not be
available. Could you PTAL?

[jochen (gone - plz use gerrit), 2015-01-26 16:18:51]

just checking, is the RWH also hidden if we display an interstitial on top?

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

https://codereview.chromium.org/871443004/diff/100001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.h:415: // of a frame are
define in Blink.
define*d*

https://codereview.chromium.org/871443004/diff/100001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl_browsertest.cc (right):

https://codereview.chromium.org/871443004/diff/100001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl_browsertest.cc:49: };
disallow copy/assign

[mlamouri (slow - plz ping), 2015-01-26 17:40:43]

When there is an interstitial, it does hide the RWHV so this should return
hidden. It matches what I see in the render process (ie.
document.visibilityState).

I've updated the CL to expose the value to the RenderFrameHost so johnme@ can
use it in https://codereview.chromium.org/866443003

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

https://codereview.chromium.org/871443004/diff/100001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.h:415: // of a frame are
define in Blink.
On 2015/01/26 at 16:18:51, jochen (slow) wrote:
> define*d*

Done.

https://codereview.chromium.org/871443004/diff/100001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl_browsertest.cc (right):

https://codereview.chromium.org/871443004/diff/100001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl_browsertest.cc:49: };
On 2015/01/26 at 16:18:51, jochen (slow) wrote:
> disallow copy/assign

Done.

[jochen (gone - plz use gerrit), 2015-01-27 09:55:36]

lgtm

[mlamouri (slow - plz ping), 2015-01-27 09:56:33]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2015-01-27 09:57:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/871443004/140001

[commit-bot: I haz the power, 2015-01-27 09:57:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-27 09:57:45]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[mlamouri (slow - plz ping), 2015-01-27 10:13:36]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2015-01-27 10:14:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/871443004/160001

[commit-bot: I haz the power, 2015-01-27 11:44:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-27 11:44:56]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[mlamouri (slow - plz ping), 2015-01-27 12:30:20]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2015-01-27 12:30:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/871443004/160001

[commit-bot: I haz the power, 2015-01-27 12:30:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-27 12:31:00]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[mlamouri (slow - plz ping), 2015-01-27 13:01:54]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2015-01-27 13:02:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/871443004/160001

[commit-bot: I haz the power, 2015-01-27 13:10:36]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-01-27 13:11:22]

Patchset 9 (id:??) landed as
https://crrev.com/f8c23a200068daeaac3139c59c9868b5a6058f75
Cr-Commit-Position: refs/heads/master@{#313264}