Commit 04bf53f7 authored by Yuri Wiitala's avatar Yuri Wiitala Committed by Commit Bot

Crash fix: RenderProcessHostImpl media stream counts.

Multiple fixes around RenderFrameHostImpl not always notifying its
RenderProcessHostImpl that audio streams have been added/removed.
Before these fixes, RenderProcessHostImpl was sometimes leaving a
render process backgrounded indefinitely, or sometimes leaving it
non-backgrounded indefinitely.

Bug: 887208
Change-Id: I1713887c96382b8bdf032c82bb49723fc0219f71
Reviewed-on: https://chromium-review.googlesource.com/1232475
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593354}
parent 54d64ad5
...@@ -694,6 +694,12 @@ RenderFrameHostImpl::~RenderFrameHostImpl() { ...@@ -694,6 +694,12 @@ RenderFrameHostImpl::~RenderFrameHostImpl() {
if (delegate_ && render_frame_created_) if (delegate_ && render_frame_created_)
delegate_->RenderFrameDeleted(this); delegate_->RenderFrameDeleted(this);
// Ensure that the render process host has been notified that all audio
// streams from this frame have terminated. This is required to ensure the
// process host has the correct media stream count, which affects its
// background priority.
OnAudibleStateChanged(false);
// If this was the last active frame in the SiteInstance, the // If this was the last active frame in the SiteInstance, the
// DecrementActiveFrameCount call will trigger the deletion of the // DecrementActiveFrameCount call will trigger the deletion of the
// SiteInstance's proxies. // SiteInstance's proxies.
...@@ -1241,8 +1247,7 @@ void RenderFrameHostImpl::RenderProcessGone(SiteInstanceImpl* site_instance) { ...@@ -1241,8 +1247,7 @@ void RenderFrameHostImpl::RenderProcessGone(SiteInstanceImpl* site_instance) {
// process should be ignored until the next commit. // process should be ignored until the next commit.
set_nav_entry_id(0); set_nav_entry_id(0);
if (is_audible_) OnAudibleStateChanged(false);
GetProcess()->OnMediaStreamRemoved();
} }
void RenderFrameHostImpl::ReportContentSecurityPolicyViolation( void RenderFrameHostImpl::ReportContentSecurityPolicyViolation(
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/navigation_handle_impl.h"
#include "content/browser/interface_provider_filtering.h" #include "content/browser/interface_provider_filtering.h"
#include "content/browser/renderer_host/input/timeout_monitor.h" #include "content/browser/renderer_host/input/timeout_monitor.h"
#include "content/browser/renderer_host/render_process_host_impl.h"
#include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/frame_messages.h" #include "content/common/frame_messages.h"
#include "content/public/browser/javascript_dialog_manager.h" #include "content/public/browser/javascript_dialog_manager.h"
...@@ -1970,4 +1971,47 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, ...@@ -1970,4 +1971,47 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
wc->SetJavaScriptDialogManagerForTesting(nullptr); wc->SetJavaScriptDialogManagerForTesting(nullptr);
} }
IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
NotifiesProcessHostOfAudibleAudio) {
const auto RunPostedTasks = []() {
base::RunLoop run_loop;
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
run_loop.QuitClosure());
run_loop.Run();
};
// Note: Just using the beforeunload.html test document to spin-up a
// renderer. Any document will do.
EXPECT_TRUE(NavigateToURL(
shell(), GetTestUrl("render_frame_host", "beforeunload.html")));
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
auto* frame = static_cast<RenderFrameHostImpl*>(
shell()->web_contents()->GetMainFrame());
auto* process = static_cast<RenderProcessHostImpl*>(frame->GetProcess());
ASSERT_EQ(0, process->get_media_stream_count_for_testing());
// Audible audio output should cause the media stream count to increment.
frame->OnAudibleStateChanged(true);
RunPostedTasks();
EXPECT_EQ(1, process->get_media_stream_count_for_testing());
// Silence should cause the media stream count to decrement.
frame->OnAudibleStateChanged(false);
RunPostedTasks();
EXPECT_EQ(0, process->get_media_stream_count_for_testing());
// Start audible audio output again, and then crash the renderer. Expect the
// media stream count to be zero after the crash.
frame->OnAudibleStateChanged(true);
RunPostedTasks();
EXPECT_EQ(1, process->get_media_stream_count_for_testing());
RenderProcessHostWatcher crash_observer(
process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
process->Shutdown(0);
crash_observer.Wait();
RunPostedTasks();
EXPECT_EQ(0, process->get_media_stream_count_for_testing());
}
} // namespace content } // namespace content
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment