Track CSS visibility state of OOPIFs on the browser side

When an OOPIF is hidden through changing CSS properties of its frame-
owner element, the state is reported to the browser and the IPC handled
in the CrossProcessFrameConnector associated with RWHV. This will also
lead to a call to RenderWidgetHostImpl::WasShown/WasHidden for the
corresponding local root.

However, when the browser is shown all RWHVs are asked to Show which
will invalidate the current state of CSS visibility. Specifically, this
causes a hidden frame to generate compositor frames.

This CL will add a bit to RenderWidgetHostViewChildFrame to remember its
CSS visibility state. Also when RWHVCF::Show is called, the view will
verify that neither itself nor an ancestor view are CSS invisible before
calling RenderWidgetHostImpl::WasShown.

BUG=628700

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I42c3d7f50aacd47b12f0de151c856233e8942510
Reviewed-on: https://chromium-review.googlesource.com/583729
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491066}

content/browser/frame_host/cross_process_frame_connector.cc

@@ -81,10 +81,14 @@ void CrossProcessFrameConnector::set_view(
 
   view_ = view;
 
-  // Attach ourselves to the new view and size it appropriately.
+  // Attach ourselves to the new view and size it appropriately. Also update
+  // visibility in case the frame owner is hidden in parent process. We should
+  // try to move these updates to a single IPC (see https://crbug.com/750179).
   if (view_) {
     view_->SetCrossProcessFrameConnector(this);
     SetRect(child_frame_rect_);
+    if (is_hidden_)
+      OnVisibilityChanged(false);
   }
 }
 
@@ -277,6 +281,7 @@ void CrossProcessFrameConnector::OnUpdateViewportIntersection(
 }
 
 void CrossProcessFrameConnector::OnVisibilityChanged(bool visible) {
+  is_hidden_ = !visible;
   if (!view_)
     return;
 

content/browser/frame_host/cross_process_frame_connector.h

@@ -138,6 +138,7 @@ class CONTENT_EXPORT CrossProcessFrameConnector {
   }
 
   bool is_inert() const { return is_inert_; }
+  bool is_hidden() const { return is_hidden_; }
 
   // Exposed for tests.
   RenderWidgetHostViewBase* GetRootRenderWidgetHostViewForTesting() {
@@ -169,6 +170,10 @@ class CONTENT_EXPORT CrossProcessFrameConnector {
   gfx::Rect viewport_intersection_rect_;
   bool is_inert_ = false;
 
+  // Visibility state of the corresponding frame owner element in parent process
+  // which is set through CSS.
+  bool is_hidden_ = false;
+
   bool is_scroll_bubbling_;
 };
 

content/browser/frame_host/render_widget_host_view_child_frame.cc

@@ -203,6 +203,10 @@ bool RenderWidgetHostViewChildFrame::IsSurfaceAvailableForCopy() const {
 void RenderWidgetHostViewChildFrame::Show() {
   if (!host_->is_hidden())
     return;
+
+  if (!CanBecomeVisible())
+    return;
+
   host_->WasShown(ui::LatencyInfo());
 }
 
@@ -904,4 +908,21 @@ gfx::Point RenderWidgetHostViewChildFrame::GetViewOriginInRoot() const {
   return gfx::Point();
 }
 
+bool RenderWidgetHostViewChildFrame::CanBecomeVisible() {
+  if (!frame_connector_)
+    return true;
+
+  if (frame_connector_->is_hidden())
+    return false;
+
+  RenderWidgetHostViewBase* parent_view = GetParentView();
+  if (!parent_view || !parent_view->IsRenderWidgetHostViewChildFrame()) {
+    // Root frame does not have a CSS visibility property.
+    return true;
+  }
+
+  return static_cast<RenderWidgetHostViewChildFrame*>(parent_view)
+      ->CanBecomeVisible();
+}
+
 }  // namespace content

content/browser/frame_host/render_widget_host_view_child_frame.h

@@ -258,6 +258,12 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
   }
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
+                           HiddenOOPIFWillNotGenerateCompositorFrames);
+  FRIEND_TEST_ALL_PREFIXES(
+      SitePerProcessBrowserTest,
+      HiddenOOPIFWillNotGenerateCompositorFramesAfterNavigation);
+
   virtual void SendSurfaceInfoToEmbedderImpl(
       const viz::SurfaceInfo& surface_info,
       const viz::SurfaceSequence& sequence);
@@ -273,6 +279,11 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
 
   virtual bool HasEmbedderChanged();
 
+  // Returns false if the view cannot be shown. This is the case where the frame
+  // associated with this view or a cross process ancestor frame has been hidden
+  // using CSS.
+  bool CanBecomeVisible();
+
   using FrameSwappedCallbackList = std::deque<std::unique_ptr<base::Closure>>;
   // Since frame-drawn callbacks are "fire once", we use std::deque to make
   // it convenient to swap() when processing the list.

content/browser/site_per_process_browsertest.cc

@@ -7158,6 +7158,233 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, VisibilityChanged) {
   EXPECT_TRUE(show_observer.WaitUntilSatisfied());
 }
 
+// A class which counts the number of times a RenderWidgetHostViewChildFrame
+// swaps compositor frames.
+class ChildFrameCompositorFrameSwapCounter {
+ public:
+  explicit ChildFrameCompositorFrameSwapCounter(
+      RenderWidgetHostViewChildFrame* view)
+      : view_(view), weak_factory_(this) {
+    RegisterCallback();
+  }
+
+  ~ChildFrameCompositorFrameSwapCounter() {}
+
+  // Wait until at least |count| new frames are swapped.
+  void WaitForNewFrames(size_t count) {
+    while (counter_ < count) {
+      base::RunLoop loop;
+      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+          FROM_HERE, loop.QuitClosure(), TestTimeouts::tiny_timeout());
+      loop.Run();
+    }
+  }
+
+  void ResetCounter() { counter_ = 0; }
+  size_t GetCount() const { return counter_; }
+
+ private:
+  void RegisterCallback() {
+    view_->RegisterFrameSwappedCallback(base::MakeUnique<base::Closure>(
+        base::Bind(&ChildFrameCompositorFrameSwapCounter::OnFrameSwapped,
+                   weak_factory_.GetWeakPtr())));
+  }
+  void OnFrameSwapped() {
+    counter_++;
+
+    // Register a new callback as the old one is released now.
+    RegisterCallback();
+  }
+
+  size_t counter_ = 0;
+
+ private:
+  RenderWidgetHostViewChildFrame* view_;
+  base::WeakPtrFactory<ChildFrameCompositorFrameSwapCounter> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChildFrameCompositorFrameSwapCounter);
+};
+
+// This test verifies that hiding an OOPIF in CSS will stop generating
+// compositor frames for the OOPIF and any nested OOPIFs inside it. This holds
+// even when the whole page is shown.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       HiddenOOPIFWillNotGenerateCompositorFrames) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/frame_tree/page_with_two_frames.html"));
+  ASSERT_TRUE(NavigateToURL(shell(), main_url));
+  ASSERT_EQ(shell()->web_contents()->GetLastCommittedURL(), main_url);
+
+  GURL cross_site_url_b =
+      embedded_test_server()->GetURL("b.com", "/counter.html");
+
+  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+
+  NavigateFrameToURL(root->child_at(0), cross_site_url_b);
+
+  NavigateFrameToURL(root->child_at(1), cross_site_url_b);
+
+  // Now inject code in the first frame to create a nested OOPIF.
+  RenderFrameHostCreatedObserver new_frame_created_observer(
+      shell()->web_contents(), 1);
+  ASSERT_TRUE(ExecuteScript(
+      root->child_at(0)->current_frame_host(),
+      "document.body.appendChild(document.createElement('iframe'));"));
+  new_frame_created_observer.Wait();
+
+  GURL cross_site_url_a =
+      embedded_test_server()->GetURL("a.com", "/counter.html");
+
+  // Navigate the nested frame.
+  TestFrameNavigationObserver observer(root->child_at(0)->child_at(0));
+  ASSERT_TRUE(ExecuteScript(
+      root->child_at(0)->current_frame_host(),
+      base::StringPrintf("document.querySelector('iframe').src = '%s';",
+                         cross_site_url_a.spec().c_str())));
+  observer.Wait();
+
+  RenderWidgetHostViewChildFrame* first_child_view =
+      static_cast<RenderWidgetHostViewChildFrame*>(
+          root->child_at(0)->current_frame_host()->GetView());
+  RenderWidgetHostViewChildFrame* second_child_view =
+      static_cast<RenderWidgetHostViewChildFrame*>(
+          root->child_at(1)->current_frame_host()->GetView());
+  RenderWidgetHostViewChildFrame* nested_child_view =
+      static_cast<RenderWidgetHostViewChildFrame*>(
+          root->child_at(0)->child_at(0)->current_frame_host()->GetView());
+
+  ChildFrameCompositorFrameSwapCounter first_counter(first_child_view);
+  ChildFrameCompositorFrameSwapCounter second_counter(second_child_view);
+  ChildFrameCompositorFrameSwapCounter third_counter(nested_child_view);
+
+  const size_t kFrameCountLimit = 20u;
+
+  // Wait for a minimum number of compositor frames for the second frame.
+  second_counter.WaitForNewFrames(kFrameCountLimit);
+  ASSERT_LE(kFrameCountLimit, second_counter.GetCount());
+
+  // Now make sure all frames have roughly the counter value in the sense that
+  // no counter value is more than twice any other.
+  float ratio = static_cast<float>(first_counter.GetCount()) /
+                static_cast<float>(second_counter.GetCount());
+  EXPECT_GT(2.5f, ratio + 1 / ratio) << "Ratio is: " << ratio;
+
+  ratio = static_cast<float>(first_counter.GetCount()) /
+          static_cast<float>(third_counter.GetCount());
+  EXPECT_GT(2.5f, ratio + 1 / ratio) << "Ratio is: " << ratio;
+
+  // Make sure all views can become visible.
+  EXPECT_TRUE(first_child_view->CanBecomeVisible());
+  EXPECT_TRUE(second_child_view->CanBecomeVisible());
+  EXPECT_TRUE(nested_child_view->CanBecomeVisible());
+
+  // Hide the first frame and wait for the notification to be posted by its
+  // RenderWidgetHost.
+  RenderWidgetHostVisibilityObserver hide_observer(
+      root->child_at(0)->current_frame_host()->GetRenderWidgetHost(), false);
+
+  // Hide the first frame.
+  ASSERT_TRUE(ExecuteScript(
+      shell(),
+      "document.getElementsByName('frame1')[0].style.visibility = 'hidden'"));
+  ASSERT_TRUE(hide_observer.WaitUntilSatisfied());
+  EXPECT_TRUE(first_child_view->FrameConnectorForTesting()->is_hidden());
+
+  // Verify that only the second view can become visible now.
+  EXPECT_FALSE(first_child_view->CanBecomeVisible());
+  EXPECT_TRUE(second_child_view->CanBecomeVisible());
+  EXPECT_FALSE(nested_child_view->CanBecomeVisible());
+
+  // Now hide and show the WebContents (to simulate a tab switch).
+  shell()->web_contents()->WasHidden();
+  shell()->web_contents()->WasShown();
+
+  first_counter.ResetCounter();
+  second_counter.ResetCounter();
+  third_counter.ResetCounter();
+
+  // We expect the second counter to keep running.
+  second_counter.WaitForNewFrames(kFrameCountLimit);
+  ASSERT_LT(kFrameCountLimit, second_counter.GetCount() + 1u);
+
+  // Verify that the counter for other two frames did not count much.
+  ratio = static_cast<float>(first_counter.GetCount()) /
+          static_cast<float>(second_counter.GetCount());
+  EXPECT_GT(0.5f, ratio) << "Ratio is: " << ratio;
+
+  ratio = static_cast<float>(third_counter.GetCount()) /
+          static_cast<float>(second_counter.GetCount());
+  EXPECT_GT(0.5f, ratio) << "Ratio is: " << ratio;
+}
+
+// This test verifies that navigating a hidden OOPIF to cross-origin will not
+// lead to creating compositor frames for the new OOPIF renderer.
+IN_PROC_BROWSER_TEST_F(
+    SitePerProcessBrowserTest,
+    HiddenOOPIFWillNotGenerateCompositorFramesAfterNavigation) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/frame_tree/page_with_two_frames.html"));
+  ASSERT_TRUE(NavigateToURL(shell(), main_url));
+  ASSERT_EQ(shell()->web_contents()->GetLastCommittedURL(), main_url);
+
+  GURL cross_site_url_b =
+      embedded_test_server()->GetURL("b.com", "/counter.html");
+
+  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+
+  NavigateFrameToURL(root->child_at(0), cross_site_url_b);
+
+  NavigateFrameToURL(root->child_at(1), cross_site_url_b);
+
+  // Hide the first frame and wait for the notification to be posted by its
+  // RenderWidgetHost.
+  RenderWidgetHostVisibilityObserver hide_observer(
+      root->child_at(0)->current_frame_host()->GetRenderWidgetHost(), false);
+
+  // Hide the first frame.
+  ASSERT_TRUE(ExecuteScript(
+      shell(),
+      "document.getElementsByName('frame1')[0].style.visibility = 'hidden'"));
+  ASSERT_TRUE(hide_observer.WaitUntilSatisfied());
+
+  // Now navigate the first frame to another OOPIF process.
+  TestFrameNavigationObserver navigation_observer(
+      root->child_at(0)->current_frame_host());
+  GURL cross_site_url_c =
+      embedded_test_server()->GetURL("c.com", "/counter.html");
+  ASSERT_TRUE(ExecuteScript(
+      web_contents(),
+      base::StringPrintf("document.getElementsByName('frame1')[0].src = '%s';",
+                         cross_site_url_c.spec().c_str())));
+  navigation_observer.Wait();
+
+  // Now investigate compositor frame creation.
+  RenderWidgetHostViewChildFrame* first_child_view =
+      static_cast<RenderWidgetHostViewChildFrame*>(
+          root->child_at(0)->current_frame_host()->GetView());
+
+  RenderWidgetHostViewChildFrame* second_child_view =
+      static_cast<RenderWidgetHostViewChildFrame*>(
+          root->child_at(1)->current_frame_host()->GetView());
+
+  EXPECT_FALSE(first_child_view->CanBecomeVisible());
+
+  ChildFrameCompositorFrameSwapCounter first_counter(first_child_view);
+  ChildFrameCompositorFrameSwapCounter second_counter(second_child_view);
+
+  const size_t kFrameCountLimit = 20u;
+
+  // Wait for a certain number of swapped compositor frames generated for the
+  // second child view. During the same interval the first frame should not have
+  // swapped any compositor frames.
+  second_counter.WaitForNewFrames(kFrameCountLimit);
+  ASSERT_LT(kFrameCountLimit, second_counter.GetCount() + 1u);
+
+  float ratio = static_cast<float>(first_counter.GetCount()) /
+                static_cast<float>(second_counter.GetCount());
+  EXPECT_GT(0.5f, ratio) << "Ratio is: " << ratio;
+}
+
 // Verify that sandbox flags inheritance works across multiple levels of
 // frames.  See https://crbug.com/576845.
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, SandboxFlagsInheritance) {

content/test/data/counter.html

+<html>
+<body>
+  <p> Counting up every 50ms </p>
+  <input id="input" value="0"/>
+  <script>
+  window.addEventListener('load', function() {
+    var inputElement = document.getElementById('input');
+    window.setInterval(function() {
+      inputElement.value = inputElement.value * 1 + 1;
+    }, 50);
+  });
+  </script>
+</body>
+</html>