Fixing WebViewVisibilityTests for OOPIF-<webview>.

content/browser/frame_host/cross_process_frame_connector.cc

Index: content/browser/frame_host/cross_process_frame_connector.cc
diff --git a/content/browser/frame_host/cross_process_frame_connector.cc b/content/browser/frame_host/cross_process_frame_connector.cc
index 1d8b23c041186c75129f4e8b0e6e2c893fe119a8..47822657a022cf1dd0eaa50e1889a082e0553308 100644
--- a/content/browser/frame_host/cross_process_frame_connector.cc
+++ b/content/browser/frame_host/cross_process_frame_connector.cc
@@ -233,6 +233,17 @@ void CrossProcessFrameConnector::OnVisibilityChanged(bool visible) {
   if (!view_)
     return;
 
+  // If there is an inner WebContents, iot should be notified of the change in

[lazyboy, 2016/01/20 00:45:35]

s/iot/it.

[EhsanK, 2016/01/26 18:36:27]

Done.

+  // the visibility.

[kenrb, 2016/01/22 22:26:01]

I would suggest expanding the comment to explain why the early return is needed
here, omitting the call to RenderWidgetHostView::Show()/Hide().

[EhsanK, 2016/01/26 18:36:27]

Done.

+  if (frame_proxy_in_parent_renderer_->frame_tree_node()
+          ->render_manager()
+          ->ForInnerDelegate()) {
+    RenderWidgetHostImpl::From(view_->GetRenderWidgetHost())
+        ->delegate()
+        ->ForwardVisibilityChange(visible);

[no sievers, 2016/01/26 20:33:06]

Something feels off about this needing to bubble up to the WebContents as
discussed offline.

It would seem more natural if the |view_| could propagate this on its own, but
it sounds like the hierarchy is lost to the RWHVChildFrame, the way things
currently are.

[EhsanK, 2016/01/27 16:03:22]

Acknowledged. As there is a separate WebContents here I believe it has to be
notified the way things are right now (For oopif-based <webview>). Whether there
will be a hierarchy for RWHVs and following that, a better way to seamlessly
forward the change to the inner WebContents is something I will try to find out.
I should also emphasize that some of the WebViewVisibilityTests directly rely on
the WebContentsObserver being called on a visibility change at the WebContents
level. That is why calling WebContents::WasShown/Hidden is necessary.

[kenrb, 2016/01/27 16:38:16]

I might not entirely understand your reasoning, but this approach looks okay to
me. This method is for passing down a visibility message from a Frame object in
a parent renderer process to the RenderWidgetHostView of the actual content, and
it seems like the right place to have the knowledge of the fact that the content
within this Frame (or <webview>, in this case) has its own WebContents that
should get the message instead. Importantly, it isn't returning the message to
the same WebContents that might have initiated the visibility change with
WasShown() or WasHidden().

This is basically recursion.

+    return;
+  }
+
   if (visible)
     view_->Show();
   else