Add TestingInstance::EvalScript method which posts a message that the test page can eval().

Add TestingInstance::ExecuteScript method which posts a message that the test page
can eval(). Change InputEvent and PostMessage tests to use this.
Add proxy for PPB_Testing_Dev::SimulateInputEvent function.
BUG=NONE
TEST=ui_tests
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114924

[bbudge, 2011-12-13 22:15:51]

This is ready for review. I will have to merge with the CL that adds
GetDocumentURL to PPB_Testing_Dev.

[dmichael (off chromium), 2011-12-13 23:40:55]

http://codereview.chromium.org/8920005/diff/15003/ppapi/api/dev/ppb_testing_d...
File ppapi/api/dev/ppb_testing_dev.idl (right):

http://codereview.chromium.org/8920005/diff/15003/ppapi/api/dev/ppb_testing_d...
ppapi/api/dev/ppb_testing_dev.idl:111: * The exception, if any, will be returned
in *exception. As with the PPB_Var
I'm not sure what this has to do with the PPB_Var interface. Assuming it was
copy/pasted, and the source should've said PPB_Var_Deprecated.

http://codereview.chromium.org/8920005/diff/15003/ppapi/tests/test_input_even...
File ppapi/tests/test_input_event.cc (right):

http://codereview.chromium.org/8920005/diff/15003/ppapi/tests/test_input_even...
ppapi/tests/test_input_event.cc:49: expected_char_text_(),
As we discussed on IM, I think you've uncovered a bug in input event creation in
NaCl (my bad) and we should fix that rather than make the test work around it.

[bbudge, 2011-12-14 03:00:04]

Punting on test_input_events, do test_post_message instead.

http://codereview.chromium.org/8920005/diff/15003/ppapi/api/dev/ppb_testing_d...
File ppapi/api/dev/ppb_testing_dev.idl (right):

http://codereview.chromium.org/8920005/diff/15003/ppapi/api/dev/ppb_testing_d...
ppapi/api/dev/ppb_testing_dev.idl:111: * The exception, if any, will be returned
in *exception. As with the PPB_Var
On 2011/12/13 23:40:55, dmichael wrote:
> I'm not sure what this has to do with the PPB_Var interface. Assuming it was
> copy/pasted, and the source should've said PPB_Var_Deprecated.

Done.

http://codereview.chromium.org/8920005/diff/15003/ppapi/tests/test_input_even...
File ppapi/tests/test_input_event.cc (right):

http://codereview.chromium.org/8920005/diff/15003/ppapi/tests/test_input_even...
ppapi/tests/test_input_event.cc:49: expected_char_text_(),
On 2011/12/13 23:40:55, dmichael wrote:
> As we discussed on IM, I think you've uncovered a bug in input event creation
in
> NaCl (my bad) and we should fix that rather than make the test work around it.

Done.

[dmichael (off chromium), 2011-12-15 04:02:37]

I hate to be a pain, but this seems to require a lot of icky code that I think
we could avoid. Would you be willing to try another approach? Here's what I have
in mind...  in the ppapi testing framework, we have special "test" messages that
the plugin can send to cause some action to happen in the JavaScript. We could
add another one for 'ExecuteScript'. Doing it this way should be less code and
would isolate the changes to ppapi/tests.

You'd add another command to test_case.html...  e.g., see this part:
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/ppapi/tests/test_case...
And you could add a 'ExecuteScriptForTest' (or whatever you want to name it)
method to testing_instance:
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/ppapi/tests/testing_i...

It would send a message that looks something like:
TESTING_MESSAGE:ExecuteScript:alert("My script rocks!");

test_case.html would know to handle the ExecuteScript message by evaluating the
last part ("alert('My script rocks!');" in this case).

Does that make sense?

Doing it this way, you won't be able to get a result or an exception, but I
don't think the tests actually use those anyway.

[bbudge, 2011-12-15 18:57:01]

On 2011/12/15 04:02:37, dmichael wrote:
> I hate to be a pain, but this seems to require a lot of icky code that I think
> we could avoid. Would you be willing to try another approach? Here's what I
have
> in mind...  in the ppapi testing framework, we have special "test" messages
that
> the plugin can send to cause some action to happen in the JavaScript. We could
> add another one for 'ExecuteScript'. Doing it this way should be less code and
> would isolate the changes to ppapi/tests.
> 
> You'd add another command to test_case.html...  e.g., see this part:
>
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/ppapi/tests/test_case...
> And you could add a 'ExecuteScriptForTest' (or whatever you want to name it)
> method to testing_instance:
>
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/ppapi/tests/testing_i...
> 
> It would send a message that looks something like:
> TESTING_MESSAGE:ExecuteScript:alert("My script rocks!");
> 
> test_case.html would know to handle the ExecuteScript message by evaluating
the
> last part ("alert('My script rocks!');" in this case).
> 
> Does that make sense?
> 
> Doing it this way, you won't be able to get a result or an exception, but I
> don't think the tests actually use those anyway.

Using PostMessage instead should work fine. It just seemed like this would be a
little more powerful. I like that your method requires no new functions, and the
page has to execute the script, since that seems to reduce the risk if a plugin
improperly got access to the testing interface.

I don't think it's that much "icky" code (except for the NaCl boilerplate), but
I'll try your approach.

[bbudge, 2011-12-16 22:32:36]

Dave, this CL uses the PostMessage technique to run script on the test page. I
kept the NaCl proxy code for PPB_Testing_Dev::SimulateInputEvent in there since
we'll need it soon. The test_input_event.cc file is unchanged, so the test is
disabled for NaCl right now.

http://codereview.chromium.org/8920005/diff/27027/ppapi/native_client/src/sha...
File ppapi/native_client/src/shared/ppapi_proxy/ppb_rpc_client.cc (right):

http://codereview.chromium.org/8920005/diff/27027/ppapi/native_client/src/sha...
ppapi/native_client/src/shared/ppapi_proxy/ppb_rpc_client.cc:1464: shm_id
Not sure why this looks different in the review tool, but it's up to date with
.srpc file.

[dmichael (off chromium), 2011-12-16 22:38:55]

http://codereview.chromium.org/8920005/diff/27027/ppapi/tests/test_input_even...
File ppapi/tests/test_input_event.cc (right):

http://codereview.chromium.org/8920005/diff/27027/ppapi/tests/test_input_even...
ppapi/tests/test_input_event.cc:50: std::string
js_code("TESTING_MESSAGE:ExecuteScript:");
Could you instead add an "ExecuteScript" convenience method to TestingInstance?

[bbudge, 2011-12-16 23:24:06]

http://codereview.chromium.org/8920005/diff/27027/ppapi/tests/test_input_even...
File ppapi/tests/test_input_event.cc (right):

http://codereview.chromium.org/8920005/diff/27027/ppapi/tests/test_input_even...
ppapi/tests/test_input_event.cc:50: std::string
js_code("TESTING_MESSAGE:ExecuteScript:");
Good idea. Done.
On 2011/12/16 22:38:56, dmichael wrote:
> Could you instead add an "ExecuteScript" convenience method to
TestingInstance?

[dmichael (off chromium), 2011-12-16 23:26:17]

LGTM, thanks for being patient with my pickyness. It looks great.

[dmichael (off chromium), 2011-12-16 23:27:17]

On 2011/12/16 23:26:17, dmichael wrote:
> LGTM, thanks for being patient with my pickyness. It looks great.

Oops, once you fix the compile error, assuming the tests pass, lgtm ;-)

[bbudge, 2011-12-17 00:20:19]

On 2011/12/16 23:27:17, dmichael wrote:
> On 2011/12/16 23:26:17, dmichael wrote:
> > LGTM, thanks for being patient with my pickyness. It looks great.
> 
> Oops, once you fix the compile error, assuming the tests pass, lgtm ;-)

I fixed the compile error by renaming the method. I wrote the comment describing
what the method does, and so 'EvalScript' seemed like a good descriptive name.
Execute returns a result and exception, Eval just calls eval(). What do you
think?

[dmichael (off chromium), 2011-12-17 00:49:34]

lgtm
On Dec 16, 2011 5:20 PM, <bbudge@chromium.org> wrote:

> On 2011/12/16 23:27:17, dmichael wrote:
>
>> On 2011/12/16 23:26:17, dmichael wrote:
>> > LGTM, thanks for being patient with my pickyness. It looks great.
>>
>
>  Oops, once you fix the compile error, assuming the tests pass, lgtm ;-)
>>
>
> I fixed the compile error by renaming the method. I wrote the comment
> describing
> what the method does, and so 'EvalScript' seemed like a good descriptive
> name.
> Execute returns a result and exception, Eval just calls eval(). What do you
> think?
>
>
http://codereview.chromium.**org/8920005/<http://codereview.chromium.org/8920...
>

[bbudge, 2011-12-17 04:21:20]

On 2011/12/17 00:49:34, dmichael wrote:
> lgtm
> On Dec 16, 2011 5:20 PM, <mailto:bbudge@chromium.org> wrote:
> 
> > On 2011/12/16 23:27:17, dmichael wrote:
> >
> >> On 2011/12/16 23:26:17, dmichael wrote:
> >> > LGTM, thanks for being patient with my pickyness. It looks great.
> >>
> >
> >  Oops, once you fix the compile error, assuming the tests pass, lgtm ;-)
> >>
> >
> > I fixed the compile error by renaming the method. I wrote the comment
> > describing
> > what the method does, and so 'EvalScript' seemed like a good descriptive
> > name.
> > Execute returns a result and exception, Eval just calls eval(). What do you
> > think?
> >
> >
>
http://codereview.chromium.**org/8920005/%3Chttp://codereview.chromium.org/89...>
> >

+ppapi/ppapi_sources.gypi so the NaCl module actually contains the tests! Grrrr.

[bbudge, 2011-12-17 05:42:13]

On 2011/12/17 04:21:20, bbudge1 wrote:
> On 2011/12/17 00:49:34, dmichael wrote:
> > lgtm
> > On Dec 16, 2011 5:20 PM, <mailto:bbudge@chromium.org> wrote:
> > 
> > > On 2011/12/16 23:27:17, dmichael wrote:
> > >
> > >> On 2011/12/16 23:26:17, dmichael wrote:
> > >> > LGTM, thanks for being patient with my pickyness. It looks great.
> > >>
> > >
> > >  Oops, once you fix the compile error, assuming the tests pass, lgtm ;-)
> > >>
> > >
> > > I fixed the compile error by renaming the method. I wrote the comment
> > > describing
> > > what the method does, and so 'EvalScript' seemed like a good descriptive
> > > name.
> > > Execute returns a result and exception, Eval just calls eval(). What do
you
> > > think?
> > >
> > >
> >
>
http://codereview.chromium.**org/8920005/%253Chttp://codereview.chromium.org/...>
> > >
> 
> +ppapi/ppapi_sources.gypi so the NaCl module actually contains the tests!
Grrrr.

SendingArrayBuffer fails in NaCl test.

[dmichael (off chromium), 2011-12-17 05:44:36]

That's fine, it's not proxied yet. It should be disabled in oop and nacl.

On Fri, Dec 16, 2011 at 10:42 PM, <bbudge@chromium.org> wrote:

> On 2011/12/17 04:21:20, bbudge1 wrote:
>
>> On 2011/12/17 00:49:34, dmichael wrote:
>> > lgtm
>> > On Dec 16, 2011 5:20 PM, <mailto:bbudge@chromium.org> wrote:
>> >
>> > > On 2011/12/16 23:27:17, dmichael wrote:
>> > >
>> > >> On 2011/12/16 23:26:17, dmichael wrote:
>> > >> > LGTM, thanks for being patient with my pickyness. It looks great.
>> > >>
>> > >
>> > >  Oops, once you fix the compile error, assuming the tests pass, lgtm
>> ;-)
>> > >>
>> > >
>> > > I fixed the compile error by renaming the method. I wrote the comment
>> > > describing
>> > > what the method does, and so 'EvalScript' seemed like a good
>> descriptive
>> > > name.
>> > > Execute returns a result and exception, Eval just calls eval(). What
>> do
>>
> you
>
>> > > think?
>> > >
>> > >
>> >
>>
>
> http://codereview.chromium.****org/8920005/%253Chttp://codere**
> view.chromium.org/8920005/ <http://codereview.chromium.org/8920005/>>
>
>  > >
>>
>
>  +ppapi/ppapi_sources.gypi so the NaCl module actually contains the tests!
>>
> Grrrr.
>
> SendingArrayBuffer fails in NaCl test.
>
>
http://codereview.chromium.**org/8920005/<http://codereview.chromium.org/8920...
>

[commit-bot: I haz the power, 2011-12-17 14:17:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bbudge@chromium.org/8920005/31030

[commit-bot: I haz the power, 2011-12-17 14:17:55]

Presubmit check for 8920005-31030 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found lines longer than 80 characters (first 5 shown).
  ppapi/native_client/src/shared/ppapi_proxy/ppb_rpc_server.cc, line 2710, 85
chars

Presubmit checks took 1.9s to calculate.