From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Bryant Chandler Date: Mon, 24 Jun 2024 17:28:53 +0000 Subject: Fix pointer tear down order problem Holding a RenderFrameHost* in the `OnceBinding` isn't safe, because the `RenderFrameHost` can be destroyed before the binding. This CL changes the task strategy so that the RenderFrameHost* doesn't need to be bound in a callback. Tested using the repro steps in the bug and this change stops it from reproducing. Fixed: 347373236 Change-Id: Id639f317b0f37a508833aba9fe52ffc5c0ed590c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5640501 Reviewed-by: Dave Tapuska Commit-Queue: Bryant Chandler Reviewed-by: Guido Urdaneta Cr-Commit-Position: refs/heads/main@{#1318653} diff --git a/content/browser/renderer_host/media/media_stream_manager.cc b/content/browser/renderer_host/media/media_stream_manager.cc index 65bee7132f6583cb64d774756423942382c8eed6..2c27db7ee49193cbf91158d576e29bb255378a3e 100644 --- a/content/browser/renderer_host/media/media_stream_manager.cc +++ b/content/browser/renderer_host/media/media_stream_manager.cc @@ -2887,25 +2887,19 @@ void MediaStreamManager::GetRawDeviceIdsOpenedForFrame( RenderFrameHost* render_frame_host, blink::mojom::MediaStreamType type, GetRawDeviceIdsOpenedForFrameCallback callback) const { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - CHECK(render_frame_host); - auto collect_all_render_frame_host_ids = base::BindOnce( - [](RenderFrameHost* render_frame_host) { - base::flat_set all_render_frame_host_ids; - render_frame_host->ForEachRenderFrameHost( - [&all_render_frame_host_ids](RenderFrameHost* render_frame_host) { - all_render_frame_host_ids.insert( - render_frame_host->GetGlobalId()); - }); - return all_render_frame_host_ids; - }, - render_frame_host); - - GetUIThreadTaskRunner()->PostTaskAndReplyWithResult( - FROM_HERE, std::move(collect_all_render_frame_host_ids), - base::BindPostTaskToCurrentDefault( - base::BindOnce(&MediaStreamManager::GetRawDeviceIdsOpenedForFrameIds, - base::Unretained(this), type, std::move(callback)))); + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + base::flat_set all_render_frame_host_ids; + render_frame_host->ForEachRenderFrameHost( + [&all_render_frame_host_ids](RenderFrameHost* render_frame_host) { + all_render_frame_host_ids.insert(render_frame_host->GetGlobalId()); + }); + + GetIOThreadTaskRunner()->PostTask( + FROM_HERE, + base::BindOnce(&MediaStreamManager::GetRawDeviceIdsOpenedForFrameIds, + base::Unretained(this), type, std::move(callback), + all_render_frame_host_ids)); } void MediaStreamManager::GetRawDeviceIdsOpenedForFrameIds( diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index b0b1d0aea5bf251411d0cab06562fd77d8d7c549..819c73370d6e6fc5e6ab24e78646a206b6a5873d 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -10808,12 +10808,9 @@ void WebContentsImpl::GetMediaCaptureRawDeviceIdsOpened( return; } - GetIOThreadTaskRunner({})->PostTask( - FROM_HERE, - base::BindOnce(&MediaStreamManager::GetRawDeviceIdsOpenedForFrame, - base::Unretained(media_stream_manager), - GetPrimaryMainFrame(), type, - base::BindPostTaskToCurrentDefault(std::move(callback)))); + media_stream_manager->GetRawDeviceIdsOpenedForFrame( + GetPrimaryMainFrame(), type, + base::BindPostTaskToCurrentDefault(std::move(callback))); } } // namespace content