Issue 7108051: Implement out-of-process proxy for PPB_Buffer_Dev.

Issue 7108051: Implement out-of-process proxy for PPB_Buffer_Dev. (Closed)

Created:
9 years, 6 months ago by Ami GONE FROM CHROMIUM

Modified:
9 years, 6 months ago

Reviewers:
piman

CC:
chromium-reviews, piman+watch_chromium.org, darin-cc_chromium.org, Paweł Hajdan Jr., vjain, vrk (LEFT CHROMIUM)

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

Implement out-of-process proxy for PPB_Buffer_Dev. BUG=85427, 85441 TEST=./ninja/ui_tests --gtest_filter=*PPAPITest.Buffer Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88630

Patch Set 1 #

Total comments: 6

Patch Set 2 : piman@ CR responses + fix for win trybot (hopefully). #

Patch Set 3 : exclude out-of-process testing for OS_MACOSX, where apparently sandboxing dies at startup. #

Created: 9 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+223 lines, -36 lines)			Patch
M	chrome/test/ui/ppapi_uitest.cc	View	1 2	3 chunks	+17 lines, -0 lines	0 comments	Download
A	ppapi/c/trusted/ppb_buffer_trusted.h	View		1 chunk	+24 lines, -0 lines	0 comments	Download
M	ppapi/cpp/dev/buffer_dev.h	View	1	2 chunks	+5 lines, -4 lines	0 comments	Download
M	ppapi/cpp/dev/buffer_dev.cc	View		1 chunk	+4 lines, -1 line	0 comments	Download
M	ppapi/ppapi_cpp.gypi	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	ppapi/ppapi_shared.gypi	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	ppapi/proxy/ppapi_messages.h	View		2 chunks	+1 line, -2 lines	0 comments	Download
M	ppapi/proxy/ppb_buffer_proxy.h	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	ppapi/proxy/ppb_buffer_proxy.cc	View	1	6 chunks	+50 lines, -21 lines	0 comments	Download
M	ppapi/shared_impl/resource_object_base.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	ppapi/tests/test_buffer.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	ppapi/tests/test_buffer.cc	View	1	2 chunks	+26 lines, -0 lines	0 comments	Download
A	ppapi/thunk/ppb_buffer_trusted_api.h	View		1 chunk	+22 lines, -0 lines	0 comments	Download
A	ppapi/thunk/ppb_buffer_trusted_thunk.cc	View		1 chunk	+34 lines, -0 lines	0 comments	Download
M	ppapi/thunk/resource_creation_api.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	ppapi/thunk/thunk.h	View		3 chunks	+2 lines, -2 lines	0 comments	Download
M	webkit/plugins/ppapi/plugin_module.cc	View		2 chunks	+3 lines, -0 lines	0 comments	Download
M	webkit/plugins/ppapi/ppb_buffer_impl.h	View		5 chunks	+9 lines, -3 lines	0 comments	Download
M	webkit/plugins/ppapi/ppb_buffer_impl.cc	View		3 chunks	+17 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

piman

LGTM+nit Thanks for the OOP unit test support. The implementation of the shm transport looks ...

9 years, 6 months ago (2011-06-09 23:11:32 UTC) #2

Ami GONE FROM CHROMIUM

9 years, 6 months ago (2011-06-09 23:35:24 UTC) #3

I copy/pasted the int->platformfile conversion code from another file. 
Hopefully you're OK with me giving you another (copy of the same) TODO :)

http://codereview.chromium.org/7108051/diff/1/ppapi/cpp/dev/buffer_dev.h
File ppapi/cpp/dev/buffer_dev.h (right):

http://codereview.chromium.org/7108051/diff/1/ppapi/cpp/dev/buffer_dev.h#newc...
ppapi/cpp/dev/buffer_dev.h:21: virtual ~Buffer_Dev();
On 2011/06/09 23:11:32, piman wrote:
> nit: move this after the other constructor.

Done.

http://codereview.chromium.org/7108051/diff/1/ppapi/proxy/ppb_buffer_proxy.cc
File ppapi/proxy/ppb_buffer_proxy.cc (right):

http://codereview.chromium.org/7108051/diff/1/ppapi/proxy/ppb_buffer_proxy.cc...
ppapi/proxy/ppb_buffer_proxy.cc:171: if
(trusted_buffer.object()->GetSharedMemory(&local_fd))
On 2011/06/09 23:11:32, piman wrote:
> nit: if (... != PP_OK)
> I think that's more readable.

Done.

http://codereview.chromium.org/7108051/diff/1/ppapi/tests/test_buffer.cc
File ppapi/tests/test_buffer.cc (right):

http://codereview.chromium.org/7108051/diff/1/ppapi/tests/test_buffer.cc#newc...
ppapi/tests/test_buffer.cc:94: char* data =
reinterpret_cast<char*>(buffer->data());
On 2011/06/09 23:11:32, piman wrote:
> static_cast

Done.

Expand Messages | Collapse Messages