Add means for running some tests only o-o-p, add messaging o-o-p test.

Add means for running some tests only o-o-p. Add messaging non-main thread test.

BUG=92909
TEST=included

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98826

[dmichael (off chromium), 2011-08-16 19:42:26]

This is just a tiny baby step towards allowing calls off of the main thread.
PostMessage is low-hanging fruit, and even it won't be safe for Strings with
this CL. But this adds a few of the hooks we'll need:
- Ability to run tests only out-of-process.
- Ability to make ppapi tests create/join threads on all platforms.

I have a start at wrappers for synchronization primitives, too, but didn't need
it for this CL.

http://codereview.chromium.org/7648033/diff/4001/ppapi/proxy/plugin_dispatche...
File ppapi/proxy/plugin_dispatcher.cc (left):

http://codereview.chromium.org/7648033/diff/4001/ppapi/proxy/plugin_dispatche...
ppapi/proxy/plugin_dispatcher.cc:107: DCHECK(MessageLoop::current());
Note: I didn't see anything in Send that required being on the main thread.
Clearly there's other work to be done elsewhere, but I think Send needs to be
callable off the main thread.

http://codereview.chromium.org/7648033/diff/4001/ppapi/tests/test_case.h
File ppapi/tests/test_case.h (right):

http://codereview.chromium.org/7648033/diff/4001/ppapi/tests/test_case.h#newc...
ppapi/tests/test_case.h:150: } while (false)
This is just so you can safely do stuff like:
if (blah)
  RUN_TEST(blab);
(I could have used curly braces, but wanted to foolproof it)

[brettw, 2011-08-16 22:55:53]

http://codereview.chromium.org/7648033/diff/6001/ppapi/c/pp_macros.h
File ppapi/c/pp_macros.h (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/c/pp_macros.h#newcode97
ppapi/c/pp_macros.h:97: #if defined(__linux__) || defined(__APPLE__) ||
defined(__FreeBSD__) || \
I'd rather not have this in this shared header file. Is the thread header file
OK?

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h
File ppapi/tests/pp_thread.h (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h#newc...
ppapi/tests/pp_thread.h:61: return (pthread_create((pthread_t*)thread,
It doesn't seem like this (pthread_t*) cast should be necessary.

[polina, 2011-08-17 17:58:15]

http://codereview.chromium.org/7648033/diff/6001/ppapi/ppapi_tests.gypi
File ppapi/ppapi_tests.gypi (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/ppapi_tests.gypi#newco...
ppapi/ppapi_tests.gypi:75: 'tests/pp_thread.h',
why in this in the tests dir?

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h
File ppapi/tests/pp_thread.h (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h#newc...
ppapi/tests/pp_thread.h:46: PP_INLINE void* PP_POSIXThreadFunctionThunk(void*
param) {
s/param/posix_thread_arg/?

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h#newc...
ppapi/tests/pp_thread.h:73: typedef DWORD (PP_WindowsThreadFunction)(void*
data);
missing newlines for visual separation?

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
File ppapi/tests/test_post_message.cc (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
ppapi/tests/test_post_message.cc:296: for (int32_t j = 0; j <
kMessagesToSendPerThread; ++j)
this can be i, no?

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
ppapi/tests/test_post_message.cc:300: for (int32_t i = 0; i < kThreadsToRun;
++i) {
don't need {}

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
ppapi/tests/test_post_message.cc:309: // can't just use std::find or equality
comparison. So we first put the
you should be able to use the equality comparison at least in cases when it is
an int, no?

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
ppapi/tests/test_post_message.cc:316: kMessagesToSendPerThread);
wrong indentation

[dmichael (off chromium), 2011-08-17 21:15:15]

Thanks for the reviews. Comments addressed, PTAL.

http://codereview.chromium.org/7648033/diff/6001/ppapi/c/pp_macros.h
File ppapi/c/pp_macros.h (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/c/pp_macros.h#newcode97
ppapi/c/pp_macros.h:97: #if defined(__linux__) || defined(__APPLE__) ||
defined(__FreeBSD__) || \
On 2011/08/16 22:55:53, brettw wrote:
> I'd rather not have this in this shared header file. Is the thread header file
> OK?
Done; that makes the most sense for this CL. I think we may eventually want to
move it back; I have a pp_synchronization.h not in this CL that will use it, and
I suspect we'll eventually want pp_thread.h and pp_synchronization.h to live in
ppapi/c.

http://codereview.chromium.org/7648033/diff/6001/ppapi/ppapi_tests.gypi
File ppapi/ppapi_tests.gypi (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/ppapi_tests.gypi#newco...
ppapi/ppapi_tests.gypi:75: 'tests/pp_thread.h',
On 2011/08/17 17:58:15, polina wrote:
> why in this in the tests dir?
By request from Brett. I was going to put it in ppapi/c, but we don't really
need it for anything other than tests (yet?), and all 3rd party (NaCl)
developers can just use posix.

I left it in the style it would need to be to move in to ppapi/c/ so that it can
go there if/when we need it for other things (like maybe thread-safe C++
wrappers)

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h
File ppapi/tests/pp_thread.h (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h#newc...
ppapi/tests/pp_thread.h:46: PP_INLINE void* PP_POSIXThreadFunctionThunk(void*
param) {
On 2011/08/17 17:58:15, polina wrote:
> s/param/posix_thread_arg/?
Done.

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h#newc...
ppapi/tests/pp_thread.h:61: return (pthread_create((pthread_t*)thread,
On 2011/08/16 22:55:53, brettw wrote:
> It doesn't seem like this (pthread_t*) cast should be necessary.
Done.

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/pp_thread.h#newc...
ppapi/tests/pp_thread.h:73: typedef DWORD (PP_WindowsThreadFunction)(void*
data);
On 2011/08/17 17:58:15, polina wrote:
> missing newlines for visual separation?
I'm not sure where you wanted them. I added one between the typedef and the
PP_CreateThread definition.

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
File ppapi/tests/test_post_message.cc (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
ppapi/tests/test_post_message.cc:296: for (int32_t j = 0; j <
kMessagesToSendPerThread; ++j)
On 2011/08/17 17:58:15, polina wrote:
> this can be i, no?
Done.

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
ppapi/tests/test_post_message.cc:300: for (int32_t i = 0; i < kThreadsToRun;
++i) {
On 2011/08/17 17:58:15, polina wrote:
> don't need {}
Done.

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
ppapi/tests/test_post_message.cc:309: // can't just use std::find or equality
comparison. So we first put the
On 2011/08/17 17:58:15, polina wrote:
> you should be able to use the equality comparison at least in cases when it is
> an int, no?
Yes, but that doesn't save me anything, since it may come back as a double, and
I can't use equality in that case. The comment was wrong below, though, so I
updated it.

http://codereview.chromium.org/7648033/diff/6001/ppapi/tests/test_post_messag...
ppapi/tests/test_post_message.cc:316: kMessagesToSendPerThread);
On 2011/08/17 17:58:15, polina wrote:
> wrong indentation
Done.

[brettw, 2011-08-18 16:19:50]

LGTM

[polina, 2011-08-18 20:02:33]

LGTM

http://codereview.chromium.org/7648033/diff/4001/ppapi/tests/test_case.h
File ppapi/tests/test_case.h (right):

http://codereview.chromium.org/7648033/diff/4001/ppapi/tests/test_case.h#newc...
ppapi/tests/test_case.h:150: } while (false)
On 2011/08/16 19:42:26, dmichael wrote:
> This is just so you can safely do stuff like:
> if (blah)
>   RUN_TEST(blab);
> (I could have used curly braces, but wanted to foolproof it)

it's better this way
we use this approach in nacl_macros.h, etc.
thanks for making the improvement

http://codereview.chromium.org/7648033/diff/6001/ppapi/ppapi_tests.gypi
File ppapi/ppapi_tests.gypi (right):

http://codereview.chromium.org/7648033/diff/6001/ppapi/ppapi_tests.gypi#newco...
ppapi/ppapi_tests.gypi:75: 'tests/pp_thread.h',
On 2011/08/17 21:15:15, dmichael wrote:
> On 2011/08/17 17:58:15, polina wrote:
> > why in this in the tests dir?
> By request from Brett. I was going to put it in ppapi/c, but we don't really
> need it for anything other than tests (yet?), and all 3rd party (NaCl)
> developers can just use posix.
> 
> I left it in the style it would need to be to move in to ppapi/c/ so that it
can
> go there if/when we need it for other things (like maybe thread-safe C++
> wrappers)

I would add a comment explaining all this to the header.