Charlie:

We're kinda stuck here. Two pages in the renderer, one spinning and locking it up. We can close the UI, but if the render process is shared, that process is jammed.

Here's my proposal. If we have to force-close a tab, we kill the render process serving it, because the other tabs it's serving are hung. Either that, or we should immediately pop up the hung renderer dialog. Should we wait until the dialog shows after 30s?

dalecurtis@: One thing we noticed here is that audio continues to play if the main renderer thread is hung, even when the tab is closed (as long as another tab keeps the renderer process alive).  This is surprising to me, since the WebContents and RenderFrameHost go away, even if the RenderFrameImpl in the renderer process doesn't.

Is there any way to get the browser process to stop playing the audio if the RenderFrameHost goes away?

(In contrast, the audio does stop when closing a tab in a shared process that isn't hung, even though the Document object apparently lives on in the renderer.)

(Breadcrumb: Avi discovered this while investigating the crash in issue 717410.)

Audio is on it's on special snowflake socket and threads in both processes. AudioRendererHost is owned by the RenderProcessHostImpl -- though maxmorin@ is working on moving this to be RenderFrame based with Mojo.

To stop it you'll want to shutdown the renderer side portion by delivering an OnChannelClosing() to the AudioMessageFilter in the renderer process. We can pause or mute audio from the browser process, or deliver errors which will stop the renderer process, but no mechanism exists for that today outside of BrowserThread::IO channel closing.

Post Mojo migration, streams will be closed (browser-side) when the frame they belong to is destroyed, which would fix this AFAICT. We could also do this in the current code in AudioRendererHost, but unless there's a hurry, it seems like redundant work.

re #6: Right. It means when we migrate audio to a separate process we need to be able to close audio stream from the browser side.

This seems to have been broken forever, so I suppose there's not a huge rush. What is the timeline for the Mojo migration?

The plan is to finish implementing, have a field trial, and hopefully also launch it this Q.

Cool, that seems ok.  The key thing (as mentioned in #7) is that the audio can stop when the browser process's RenderFrameHost goes away, not just when the renderer's RenderFrame goes away (which might never happen if it's hung).  Thanks!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3910038909a964cdb5e7ce46e6701e07062f5557

commit 3910038909a964cdb5e7ce46e6701e07062f5557
Author: Max Morin <maxmorin@chromium.org>
Date: Mon Jun 05 14:43:47 2017

Glue for RendererAudioOutputStreamFactory.

This CL enables RenderFrameHostImpls to handle
RendererAudioOutputStreamFactory requests. It also makes sure that no
factory outlives the frame it belongs to, by making the frame own the
factory.
This is conditional on
RendererAudioOutputStreamFactoryContextImpl::UseMojoFactories(), which
is hardcoded to be false. A feature flag will be introduced in a
followup CL.

I manually verified that that the repro case from 717825 doesn't
reproduce in a build with mojo streams enabled.

BUG= 425368 , 717825 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ief3d8c221c3ca905626daa3315c2ef69e0ff3975
Reviewed-on: https://chromium-review.googlesource.com/518049
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476982}
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/renderer_host/media/render_frame_audio_output_stream_factory.cc
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/renderer_host/media/render_frame_audio_output_stream_factory.h
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/renderer_host/media/renderer_audio_output_stream_factory_context_impl.cc
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/renderer_host/media/renderer_audio_output_stream_factory_context_impl.h
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/renderer_host/media/renderer_audio_output_stream_factory_context_impl_unittest.cc
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/public/browser/render_process_host.h
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/3910038909a964cdb5e7ce46e6701e07062f5557/content/public/test/mock_render_process_host.h

[bulk-edit: disregard if N/A] Can the owner please set milestone to this bug if applicable?