Fire visibilityChange event on out-of-process iframes.

content/renderer/render_view_impl.cc

Index: content/renderer/render_view_impl.cc
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 4a9410c788105f93c7c1f856a5b39f873a4c8964..13dfb6f18da6866d22da588195d86e7d6dbcaa35 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1336,6 +1336,9 @@ bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(PageMsg_UpdateWindowScreenRect,
                         OnUpdateWindowScreenRect)
     IPC_MESSAGE_HANDLER(PageMsg_SetZoomLevel, OnSetZoomLevel)
+    IPC_MESSAGE_HANDLER(PageMsg_WasHidden, OnPageWasHidden)
+    IPC_MESSAGE_HANDLER(PageMsg_WasShown, OnPageWasShown)
+
 #if defined(OS_ANDROID)
     IPC_MESSAGE_HANDLER(ViewMsg_UpdateTopControlsState,
                         OnUpdateTopControlsState)
@@ -2311,10 +2314,6 @@ bool RenderViewImpl::GetContentStateImmediately() const {
   return send_content_state_immediately_;
 }
 
-blink::WebPageVisibilityState RenderViewImpl::GetVisibilityState() const {
-  return visibilityState();
-}
-
 void RenderViewImpl::DidStartLoading() {
   main_render_frame_->didStartLoading(true);
 }
@@ -2777,9 +2776,7 @@ void RenderViewImpl::Close() {
   RenderThread::Get()->Send(new ViewHostMsg_Close_ACK(GetRoutingID()));
 }
 
-void RenderViewImpl::OnWasHidden() {
-  RenderWidget::OnWasHidden();
-
+void RenderViewImpl::OnPageWasHidden() {
 #if defined(OS_ANDROID) && defined(ENABLE_WEBRTC)
   RenderThreadImpl::current()->video_capture_impl_manager()->
       SuspendDevices(true);
@@ -2787,21 +2784,26 @@ void RenderViewImpl::OnWasHidden() {
     speech_recognition_dispatcher_->AbortAllRecognitions();
 #endif
 
-  if (webview())
-    webview()->setVisibilityState(visibilityState(), false);
+  if (webview()) {
+    blink::WebPageVisibilityState visibilityState =
+        GetMainRenderFrame() ? GetMainRenderFrame()->visibilityState()

[kenrb, 2016/05/24 20:25:18]

Why do we defer to the main frame's visibility state for the RenderViewImpl in
the top-level frame's renderer? It looks like they should always match, since
the top RWHV will get a Hide message immediately before the PageMsg_WasHidden,
but in future when RenderWidget is separated from RenderView this seems like it
would be wrong.

[lfg, 2016/05/24 23:33:50]

This is a bit tricky to fix, it's not as simple as tracking separately the
visibility for the view. The only thing that overrides the visibilityState is
prerender, which does by looking up the frame, and I can't fully split the
visibility without fixing prerender to make it OOPIF-aware.

Note that I just moved the code that was implicitly done in
RenderViewImpl::visibilityState() before. Would it be ok to just add a TODO
here?

[kenrb, 2016/05/25 20:29:14]

I agree it should work fine as is, but I think this becomes a race condition
when RenderWidget and RenderView get split. It is reasonable to file a bug for
this and make it a blocker for that work (if you I'm right about the race).

[lfg, 2016/05/31 19:54:01]

I've added a TODO with a link to the prerendering bug, since fixing prerendering
is the proper way of handling this.

+                             : blink::WebPageVisibilityStateHidden;
+    webview()->setVisibilityState(visibilityState, false);
+  }
 }
 
-void RenderViewImpl::OnWasShown(bool needs_repainting,
-                                const ui::LatencyInfo& latency_info) {
-  RenderWidget::OnWasShown(needs_repainting, latency_info);
-
+void RenderViewImpl::OnPageWasShown() {
 #if defined(OS_ANDROID) && defined(ENABLE_WEBRTC)
   RenderThreadImpl::current()->video_capture_impl_manager()->
       SuspendDevices(false);
 #endif
 
-  if (webview())
-    webview()->setVisibilityState(visibilityState(), false);
+  if (webview()) {
+    blink::WebPageVisibilityState visibilityState =
+        GetMainRenderFrame() ? GetMainRenderFrame()->visibilityState()
+                             : blink::WebPageVisibilityStateVisible;
+    webview()->setVisibilityState(visibilityState, false);
+  }
 }
 
 GURL RenderViewImpl::GetURLForGraphicsContext3D() {
@@ -3078,16 +3080,10 @@ double RenderViewImpl::zoomFactorToZoomLevel(double factor) const {
 }
 
 blink::WebPageVisibilityState RenderViewImpl::visibilityState() const {
-  blink::WebPageVisibilityState current_state = is_hidden() ?
-      blink::WebPageVisibilityStateHidden :
-      blink::WebPageVisibilityStateVisible;
-  blink::WebPageVisibilityState override_state = current_state;
-  // TODO(jam): move this method to WebFrameClient.
-  if (GetContentClient()->renderer()->
-          ShouldOverridePageVisibilityState(main_render_frame_,
-                                            &override_state))
-    return override_state;
-  return current_state;
+  // This method should only be used to determine initial visibility.

[kenrb, 2016/05/24 20:25:18]

This comment isn't very clear, what does 'initial visibility' mean exactly? And
should the comment be moved to the declaration in WebViewClient.h, since it is
relevant to callers of the method?

[lfg, 2016/05/24 23:33:51]

Perhaps it should be moved. When the WebViewImpl is created, there's no main
frame created yet, and we must provide the WebView with an initial
WebPageVisibilityState. This initial visibility is set by calling
WebViewImpl::setVisibilityState() with 'true' on the second argument. Subsequent
calls to setVisibilityState() will only trigger a visibilityChange event if the
new visibility state is different from the previous one.

I was afraid of changing the initial visibility to be only executed after the
main frame is created. since there's a lot of stuff that happens between those
two states, but I could give it a try. WDYT?

[kenrb, 2016/05/25 20:29:14]

Now, that I understand this better, I'm not sure if there is a good reason to
change that. Would it simplify anything?

[lfg, 2016/05/31 19:54:01]

Yes, we would be able to remove visibilityState() from WebViewClient. I'll try
to do that, but I'll do in a separate patch.

+  DCHECK(!main_render_frame_);
+  return is_hidden() ? blink::WebPageVisibilityStateHidden
+                     : blink::WebPageVisibilityStateVisible;
 }
 
 void RenderViewImpl::draggableRegionsChanged() {