Skip to content

Commit

Permalink
Hide RenderWidgetHostViewChildFrame if Occluded
Browse files Browse the repository at this point in the history
The RenderWidgetHostView::WasOccluded and ~::WasUnOccluded are not overridden. On MacOSX minimizing
the browser leads to calling WebContents::WasOccluded which is therefore not triggering the
visibility of child RenderWidgets.

This CL implements the methods for RWHVCF and adds a test which verifies requestAnimationFrame is
throttled for occluded child renderer processes.

Bug: 903455
Change-Id: If836a872e6372abf31017cc81dcaa3e7d31d65b9
Reviewed-on: https://chromium-review.googlesource.com/c/1357303
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614162}
  • Loading branch information
ehsan-karamad authored and Commit Bot committed Dec 5, 2018
1 parent 76d39b3 commit a03e158
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,14 @@ bool RenderWidgetHostViewChildFrame::IsShowing() {
return !host()->is_hidden();
}

void RenderWidgetHostViewChildFrame::WasOccluded() {
Hide();
}

void RenderWidgetHostViewChildFrame::WasUnOccluded() {
Show();
}

gfx::Rect RenderWidgetHostViewChildFrame::GetViewBounds() const {
gfx::Rect rect;
if (frame_connector_) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
void Show() override;
void Hide() override;
bool IsShowing() override;
void WasUnOccluded() override;
void WasOccluded() override;
gfx::Rect GetViewBounds() const override;
gfx::Size GetVisibleViewportSize() const override;
void SetInsets(const gfx::Insets& insets) override;
Expand Down
46 changes: 46 additions & 0 deletions content/browser/site_per_process_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14488,4 +14488,50 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
EXPECT_FALSE(delete_a14.deleted());
}

#if !defined(OS_ANDROID)
// This test verifies that after occluding a WebContents the RAF inside a
// cross-process child frame is throttled.
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
OccludedRenderWidgetThrottlesRAF) {
GURL main_url(embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b)"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));
FrameTreeNode* root = web_contents()->GetFrameTree()->root();
FrameTreeNode* subframe = root->child_at(0);
GURL page_with_raf_counter =
embedded_test_server()->GetURL("a.com", "/page_with_raf_counter.html");
NavigateFrameToURL(subframe, page_with_raf_counter);

// Initially page is visible - wait some time and then ensure a good number of
// rafs have been generated.
auto wait_for_half_a_second = []() {
base::RunLoop run_loop;
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, run_loop.QuitClosure(),
base::TimeDelta::FromMilliseconds(500));
run_loop.Run();
};

ASSERT_TRUE(ExecuteScript(subframe, "reset_count();"));
wait_for_half_a_second();
int32_t default_raf_count = EvalJs(subframe, "raf_count").ExtractInt();
// On a 60 fps we should expect more than 30 counts - however purely for
// sanity checking and avoiding unnecessary flakes adding a comparison for a
// much lower value. This verifies that we did get *some* rAFs.
EXPECT_GT(default_raf_count, 5);
web_contents()->WasOccluded();
ASSERT_TRUE(ExecuteScript(subframe, "reset_count();"));
wait_for_half_a_second();
int32_t raf_count = EvalJs(subframe, "raf_count").ExtractInt();
// If the frame is throttled, we should expect 0 rAFs.
EXPECT_EQ(raf_count, 0);
// Sanity-check: unoccluding will reverse the effect.
web_contents()->WasShown();
ASSERT_TRUE(ExecuteScript(subframe, "reset_count();"));
wait_for_half_a_second();
ASSERT_TRUE(ExecuteScriptAndExtractInt(
subframe, "window.domAutomationController.send(raf_count)", &raf_count));
EXPECT_GT(raf_count, 5);
}
#endif
} // namespace content
15 changes: 11 additions & 4 deletions content/browser/web_contents/web_contents_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1451,8 +1451,10 @@ void WebContentsImpl::IncrementCapturerCount(const gfx::Size& capture_size) {
// TODO(fdoray): Replace RenderWidgetHostView::WasUnOccluded() with a method
// to explicitly notify the RenderWidgetHostView that capture began.
// https://crbug.com/668690
for (RenderWidgetHostView* view : GetRenderWidgetHostViewsInTree())
view->WasUnOccluded();
if (auto* main_view = GetRenderWidgetHostView())
main_view->WasUnOccluded();
if (!ShowingInterstitialPage())
SetVisibilityForChildViews(true);
}
}

Expand Down Expand Up @@ -1729,8 +1731,13 @@ void WebContentsImpl::SetMainFrameImportance(

void WebContentsImpl::WasOccluded() {
if (!IsBeingCaptured()) {
for (RenderWidgetHostView* view : GetRenderWidgetHostViewsInTree())
view->WasOccluded();
if (auto* main_view = GetRenderWidgetHostView())
main_view->WasOccluded();
if (!ShowingInterstitialPage()) {
// Occluding child view is treated as hiding the view
// (https://crbug.com/903455).
SetVisibilityForChildViews(false);
}
}
SetVisibility(Visibility::OCCLUDED);
}
Expand Down
17 changes: 17 additions & 0 deletions content/test/data/page_with_raf_counter.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!doctype html>
<h1>This page contains a RAF loop and does some measurements of the interval</h1>
<script>
var recent_time_stamps = new Array(10);
var raf_count = 0;

function reset_count() {
raf_count = 0;
}
function on_raf(t) {
raf_count++;
recent_time_stamps.shift();
recent_time_stamps.push(t);
window.requestAnimationFrame(on_raf);
}
window.requestAnimationFrame(on_raf);
</script>

0 comments on commit a03e158

Please sign in to comment.