Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(51)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 880393002: Fix webrtcAudioPrivate API to handle requests from <webview>s. (Closed)

Created:
5 years, 10 months ago by jiayl
Modified:
5 years, 10 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, creis+watch_chromium.org, posciak+watch_chromium.org, nasko+codewatch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, arv+watch_chromium.org, chromium-apps-reviews_chromium.org, wjia+watch_chromium.org, miu+watch_chromium.org, tommi (sloooow) - chröme, henrika (OOO until Aug 14), juberti2
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix webrtcAudioPrivate API to handle requests from <webview>s. BUG=424762 Committed: https://crrev.com/77c6128e5cdf8e2fa7b99e660f353752a17d1345 Cr-Commit-Position: refs/heads/master@{#314036}

Patch Set 1 #

Total comments: 3

Patch Set 2 : for jam's comments #

Total comments: 1

Patch Set 3 : address tommi's comment #

Patch Set 4 : update test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+132 lines, -90 lines) Patch
M chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.h View 4 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc View 1 2 3 6 chunks +54 lines, -24 lines 0 comments Download
M chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc View 1 2 3 8 chunks +14 lines, -8 lines 0 comments Download
M chrome/browser/resources/hangout_services/thunk.js View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/common/extensions/api/webrtc_audio_private.idl View 3 chunks +16 lines, -6 lines 0 comments Download
M content/browser/renderer_host/media/audio_renderer_host.h View 1 3 chunks +4 lines, -5 lines 0 comments Download
M content/browser/renderer_host/media/audio_renderer_host.cc View 1 2 chunks +8 lines, -13 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.h View 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_view_host_impl.h View 1 chunk +0 lines, -2 lines 0 comments Download
M content/browser/renderer_host/render_view_host_impl.cc View 1 chunk +0 lines, -7 lines 0 comments Download
M content/public/browser/render_process_host.h View 1 3 chunks +16 lines, -0 lines 0 comments Download
M content/public/browser/render_view_host.h View 3 chunks +0 lines, -16 lines 0 comments Download
M content/public/test/mock_render_process_host.h View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 21 (8 generated)
jiayl
vrk: webrtc_audio_private_api, media jam: content/public, content/browser/renderer_host
5 years, 10 months ago (2015-01-28 23:08:11 UTC) #4
vrk (LEFT CHROMIUM)
Dale or Tommi, could you take this instead?
5 years, 10 months ago (2015-01-28 23:31:13 UTC) #6
DaleCurtis
Defer to tommi, I don't know this code
5 years, 10 months ago (2015-01-28 23:42:46 UTC) #7
vrk (LEFT CHROMIUM)
On 2015/01/28 23:42:46, DaleCurtis wrote: > Defer to tommi, I don't know this code Well, ...
5 years, 10 months ago (2015-01-29 00:05:14 UTC) #9
jam
lgtm with nits https://codereview.chromium.org/880393002/diff/1/content/browser/renderer_host/media/audio_renderer_host.cc File content/browser/renderer_host/media/audio_renderer_host.cc (right): https://codereview.chromium.org/880393002/diff/1/content/browser/renderer_host/media/audio_renderer_host.cc#newcode150 content/browser/renderer_host/media/audio_renderer_host.cc:150: const { ditto https://codereview.chromium.org/880393002/diff/1/content/browser/renderer_host/media/audio_renderer_host.h File content/browser/renderer_host/media/audio_renderer_host.h ...
5 years, 10 months ago (2015-01-29 00:19:05 UTC) #10
jiayl
Addressed jam's nits. tommi, PTAL.
5 years, 10 months ago (2015-01-29 19:38:43 UTC) #11
jiayl
ping tommi
5 years, 10 months ago (2015-01-30 18:27:09 UTC) #12
tommi (sloooow) - chröme
lgtm. thanks for fixing! https://codereview.chromium.org/880393002/diff/20001/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc File chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc (right): https://codereview.chromium.org/880393002/diff/20001/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc#newcode136 chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:136: content::RenderProcessHost* rhp = NULL; nit: ...
5 years, 10 months ago (2015-01-30 20:17:40 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/880393002/40001
5 years, 10 months ago (2015-01-30 20:35:43 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/20080)
5 years, 10 months ago (2015-01-30 21:50:43 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/880393002/60001
5 years, 10 months ago (2015-01-30 22:48:49 UTC) #19
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 10 months ago (2015-01-31 00:12:22 UTC) #20
commit-bot: I haz the power
5 years, 10 months ago (2015-01-31 00:13:19 UTC) #21
Message was sent while issue was closed. 
Patchset 4 (id:??) landed as
https://crrev.com/77c6128e5cdf8e2fa7b99e660f353752a17d1345
Cr-Commit-Position: refs/heads/master@{#314036}

Powered by Google App Engine
This is Rietveld 408576698