Issue 1207005: Add a Pepper audio basic functionality unit test.

Issue 1207005: Add a Pepper audio basic functionality unit test. (Closed)

Created:
10 years, 9 months ago by neb

Modified:
9 years, 7 months ago

Reviewers:
brettw

CC:
chromium-reviews_googlegroups.com, brettw+cc_chromium.org, ncarter (slow), fbarchard, Alpha Left Google, ben+cc_chromium.org, John Grabowski, Erik does not do reviews, idana, jam, Aaron Boodman, dpranke+watch_chromium.org, pam+watch_chromium.org, awong, Paweł Hajdan Jr., darin (slow to review), amit, tim (not reviewing), scherkus (not reviewing)

Visibility:
Public.

Description

Add a Pepper audio basic functionality unit test. Also changed TestSink to be derived from IPC::Channel and made MockRenderThread service AddFilter/RemoveFilter, so that it can be used by MessageFilters. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=42656

Patch Set 1 #

Patch Set 2 : Fixed whitespace #

Patch Set 3 : Fix a build breakage #

Total comments: 6

Patch Set 4 : Fixed style issues. #

Created: 10 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+55 lines, -9 lines)			Patch
M	chrome/browser/renderer_host/mock_render_process_host.cc	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/common/ipc_test_sink.h	View		2 chunks	+3 lines, -2 lines	0 comments	Download
M	chrome/common/ipc_test_sink.cc	View		1 chunk	+3 lines, -2 lines	0 comments	Download
M	chrome/renderer/mock_render_thread.h	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/renderer/mock_render_thread.cc	View	1 2 3	2 chunks	+11 lines, -1 line	0 comments	Download
M	chrome/renderer/pepper_devices_unittest.cc	View	1 2 3	3 chunks	+27 lines, -0 lines	0 comments	Download
M	ipc/ipc_channel.h	View	1 2 3	1 chunk	+7 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

brettw

LGTM http://codereview.chromium.org/1207005/diff/16001/17004 File chrome/renderer/mock_render_thread.cc (right): http://codereview.chromium.org/1207005/diff/16001/17004#newcode79 chrome/renderer/mock_render_thread.cc:79: sink_.Send(const_cast<IPC::Message *>(&msg)); I don't normally see people put ...

10 years, 9 months ago (2010-03-24 23:43:15 UTC) #2

neb

10 years, 9 months ago (2010-03-25 18:41:03 UTC) #3

http://codereview.chromium.org/1207005/diff/16001/17004
File chrome/renderer/mock_render_thread.cc (right):

http://codereview.chromium.org/1207005/diff/16001/17004#newcode79
chrome/renderer/mock_render_thread.cc:79: sink_.Send(const_cast<IPC::Message
*>(&msg));
On 2010/03/24 23:43:16, brettw wrote:
> I don't normally see people put a space before the * here.

Done.

http://codereview.chromium.org/1207005/diff/16001/17006
File chrome/renderer/pepper_devices_unittest.cc (right):

http://codereview.chromium.org/1207005/diff/16001/17006#newcode120
chrome/renderer/pepper_devices_unittest.cc:120: // Audio callback, currently
empty
On 2010/03/24 23:43:16, brettw wrote:
> Period at the end of comments.

Done.

http://codereview.chromium.org/1207005/diff/16001/17007
File ipc/ipc_channel.h (right):

http://codereview.chromium.org/1207005/diff/16001/17007#newcode100
ipc/ipc_channel.h:100: // Used by TestSink. Setting impl to 0 lets unsupported
functions segfault.
On 2010/03/24 23:43:16, brettw wrote:
> Can you expand on this a tiny bit? I'd suggest
> 
> Used in Chrome by the TestSink to provide a dummy channel implementation for
> testing. TestSink overrides the "interesting" functions in Channel so no
actual
> implementation is needed. This will cause un-overridden calls to segfault. Do
> not use outside of test code!

Done.

Expand Messages | Collapse Messages