Separate IPC logistics from business logic and clean up the code.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b08e842c3fab03521e0c1fb954c39453c96edeab commit b08e842c3fab03521e0c1fb954c39453c96edeab Author: maxmorin <maxmorin@chromium.org> Date: Fri Nov 18 09:34:01 2016 Factor out hadndling of the authorization IPC from AudioRendererHost. This is part of an effort to make AudioRendererHost small enough to easily replace with a class using mojo. The pending/completed authorizations are still stored in ARH, since the mojo implementation won't need to store pending authorizations. BUG= 656923 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2424163004 Cr-Commit-Position: refs/heads/master@{#433160} [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/BUILD.gn [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/bad_message.h [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/renderer_host/media/audio_input_device_manager.cc [add] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/renderer_host/media/audio_output_authorization_handler.cc [add] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/renderer_host/media/audio_output_authorization_handler.h [add] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/renderer_host/media/audio_renderer_host.cc [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/renderer_host/media/audio_renderer_host.h [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/browser/renderer_host/media/audio_renderer_host_unittest.cc [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/public/browser/media_device_id.cc [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/public/browser/media_device_id.h [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/content/test/BUILD.gn [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/media/audio/fake_audio_manager.cc [modify] https://crrev.com/b08e842c3fab03521e0c1fb954c39453c96edeab/tools/metrics/histograms/histograms.xml
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/070d5b996806694c9303f75d9a419392041dec9c commit 070d5b996806694c9303f75d9a419392041dec9c Author: maxmorin <maxmorin@chromium.org> Date: Fri Nov 25 08:59:40 2016 Fix shutdown crash in AudioOutputAuthorizationHandler. If fetching device parameters is scheduled during shutdown, it is possible that the UI thread starts deleting the AudioManager, causing a crash. The problem arises like this: 1. AudioOutputAuthorizationHandler posts GetAudioParametersOnDeviceThread to the audio thread. 2. Shutdown is started, browser threads other than UI are stopped and audio_manager_ in BrowserMainLoop is reset. The causes g_last_created in AudioManager to be nulled and a task to delete the pointer is posted to the audio thread. 3. GetAudioParametersOnDeviceThread runs and calls AudioManager::Get(), which returns g_last_created (i.e. null), and dereferences it, causing a crash. On the contrary, if the AudioManager pointer was stored in AudioOutputAuthorizationhandler and passed into GetAudioParametersOnDeviceThread, we wouldn't have a problem with the UI thread nulling the global pointer. We know that the AudioManager won't be destructed before it fires because the task to destroy the AudioManager is posted after the IO thread is shutdown, and GetAudioParametersOnDeviceThread is posted before/during the shutdown of the IO thread. The AudioManager* is now passed in to the constructor of AudioOutputAuthorizationhandler. Drive-by fix: Remove MakeAuthorizationData, since it's only called form a single place now. The string argument was irrelevant (and even the wrong kind of id). Also a drive-by fix: elimination of bare new in initialization of AudioOutputAuthorizationhandler. BUG= 656923 ,667981 Review-Url: https://codereview.chromium.org/2529853002 Cr-Commit-Position: refs/heads/master@{#434460} [modify] https://crrev.com/070d5b996806694c9303f75d9a419392041dec9c/content/browser/renderer_host/media/audio_output_authorization_handler.cc [modify] https://crrev.com/070d5b996806694c9303f75d9a419392041dec9c/content/browser/renderer_host/media/audio_output_authorization_handler.h [modify] https://crrev.com/070d5b996806694c9303f75d9a419392041dec9c/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc [modify] https://crrev.com/070d5b996806694c9303f75d9a419392041dec9c/content/browser/renderer_host/media/audio_renderer_host.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/65f77fb7054d31082b55e7385617c819f31902ec commit 65f77fb7054d31082b55e7385617c819f31902ec Author: maxmorin <maxmorin@chromium.org> Date: Mon Dec 05 13:45:36 2016 Factor out AudioOutputDelegate from AudioRendererHost. This change moves logic related to IPC requests regarding a single stream into its own class. This prepares for adding a mojo alternative to ARH. The mojo implementation will have a separate interface for an audio stream, which makes it very reasonable to have a class responsible for handling requests to a stream. This change also slims down ARH, so that it's easier to replace. Reference for mojofication work: https://docs.google.com/document/d/1PjV_EutyJANmsEjhH9mTNZWSslTPvAit7Cy2RO3cbmk/edit, in particular the diagram https://docs.google.com/drawings/d/1xg6z2FwLdTe5eG-Nvek9q-IfWzMI01XxlbVH3y0mzAE/edit. BUG= 656923 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel,linux_chromium_msan_rel_ng,linux_chromium_chromeos_asan_rel_ng,linux_chromium_chromeos_msan_rel_ng,linux_chromium_tsan_rel_ng,linux_chromium_ubsan_rel_ng;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel,mac_chromium_asan_rel_ng;master.tryserver.chromium.win:win_optional_gpu_tests_rel,win_chromium_syzyasan_rel Review-Url: https://codereview.chromium.org/2443573003 Cr-Commit-Position: refs/heads/master@{#436278} [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/BUILD.gn [add] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/audio_device_thread.cc [add] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/audio_device_thread.h [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/browser_main_loop.cc [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/browser_main_loop.h [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc [add] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/renderer_host/media/audio_output_delegate.cc [add] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/renderer_host/media/audio_output_delegate.h [add] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/renderer_host/media/audio_output_delegate_unittest.cc [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/renderer_host/media/audio_renderer_host.cc [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/renderer_host/media/audio_renderer_host.h [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/renderer_host/media/audio_renderer_host_unittest.cc [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/content/test/BUILD.gn [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/media/audio/audio_output_controller.cc [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/media/audio/audio_output_controller.h [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/media/audio/audio_output_controller_unittest.cc [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/media/audio/audio_streams_tracker.cc [modify] https://crrev.com/65f77fb7054d31082b55e7385617c819f31902ec/media/audio/audio_streams_tracker.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d8cd11c786e0173afcc7031d85aa3cec75376b6c commit d8cd11c786e0173afcc7031d85aa3cec75376b6c Author: dalecurtis <dalecurtis@chromium.org> Date: Tue Dec 13 21:56:41 2016 Cleanup AudioDeviceThread to avoid accidental misuse. The code as written returned the current task runner whenever AudioDeviceThread::GetTaskRunner() was called, when actually what we want is for it to always be bound to the constructed thread. This isn't an issue given that we didn't expose AudioDeviceThread outside of BrowserMainLoop, but this prevents any future misuse and cleans up some style issues. Note: This also renames AudioDeviceThread to AudioManagerThread since we already have an AudioDeviceThread in the renderer. BUG= 656923 TEST=none Review-Url: https://codereview.chromium.org/2558303004 Cr-Commit-Position: refs/heads/master@{#438301} [modify] https://crrev.com/d8cd11c786e0173afcc7031d85aa3cec75376b6c/content/browser/BUILD.gn [delete] https://crrev.com/a690a3b43c0772ef91d7207587969ba2e63a0891/content/browser/audio_device_thread.cc [delete] https://crrev.com/a690a3b43c0772ef91d7207587969ba2e63a0891/content/browser/audio_device_thread.h [add] https://crrev.com/d8cd11c786e0173afcc7031d85aa3cec75376b6c/content/browser/audio_manager_thread.cc [add] https://crrev.com/d8cd11c786e0173afcc7031d85aa3cec75376b6c/content/browser/audio_manager_thread.h [modify] https://crrev.com/d8cd11c786e0173afcc7031d85aa3cec75376b6c/content/browser/browser_main_loop.cc [modify] https://crrev.com/d8cd11c786e0173afcc7031d85aa3cec75376b6c/content/browser/browser_main_loop.h [modify] https://crrev.com/d8cd11c786e0173afcc7031d85aa3cec75376b6c/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc [modify] https://crrev.com/d8cd11c786e0173afcc7031d85aa3cec75376b6c/content/browser/renderer_host/media/audio_output_delegate_unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c83cf384c45aa4b3aaacacc65c88f634f29824d2 commit c83cf384c45aa4b3aaacacc65c88f634f29824d2 Author: maxmorin <maxmorin@chromium.org> Date: Mon Apr 10 06:31:58 2017 Remove some unnecessary stuff from AudioOutputDelegateImplUnitTest. Not sure why they are there to begin with. BUG= 656923 Review-Url: https://codereview.chromium.org/2800013003 Cr-Commit-Position: refs/heads/master@{#463180} [modify] https://crrev.com/c83cf384c45aa4b3aaacacc65c88f634f29824d2/content/browser/renderer_host/media/audio_output_delegate_impl_unittest.cc
Comment 1 by bugdroid1@chromium.org
, Nov 18 2016