WebUI Testing: async support - global mocking, deferred runs, continued run.

- Add ability to save all matched args and call a function with them in mock.will()
- Add ability to mock globals
- Add ability to call global with savedArgs
- Add ability to reuse savedArgs
- Add ability to continue an async test
- Add ability for test fixture's SetUp to defer RunTest until conditions are right and call function.
- Add tests for generated async test
- Add one print preview test to expect a global function to be called.
- Defer TearDown to testDone so async tests aren't torn down until they're complete.
- Try the SetUp call so errors are shown.

R=mmenke@chromium.org,dtseng@chromium.org,arv@chromium.org
BUG=91737
TEST=browser_tests --gtest_filter=PrintPreviewWebUITest.TestPrinterChangeUpdatesPreview

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

[Sheridan Rawlins, 2011-08-13 07:56:33]

This CL should pave the way for print preview async testing of things such as
- open a PDF, print, and see that the preview is non-modifiable.
- open a large doc, and verify that pipelining goes as expected.
- other interactions with real C++ handler with JS mocking of printers (which
won't be available on trybots or the same on others' computers).

[Sheridan Rawlins, 2011-08-19 14:51:21]

I've extended the CL to tackle some of Matt's async use cases, and adjusted the
summary & subject.  This is a good chunk of functionality that Matt and I can
build on in other patches.  Please have a look.

Thanks,

-SCR

[mmenke, 2011-08-22 16:26:44]

I think your code is basically fine, but a lot of the test_api.js function
comments aren't terribly illuminating.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:856: * then forces |this| to be an instance
of |realMatcher|.
You're defining a class in terms of one of its methods, and an argument to that
method.  Reading this, I have no clue what this class actually does.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:864: this.argumentMatches =
this.argumentMatches;
?

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:895: * |realMatcher|.
This isn't terribly illuminating for someone trying to use this class.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:998: * Syntactic sugar for will() on a
Mock4JS.Mock.
nit:  "for use with" make it clearer that this should be used for the argument
to "will".

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1009: * Syntactic sugar for will() on a
Mock4JS.Mock.
nit:  "for use with"

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1023: * expectations and still call the
global function.
nit:  Say what it does, not just why it exists.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1052: * Syntactic sugar for will() on a
Mock4JS.Mock.
nit:  For use with.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1056: *     method is invoked.
nit:  Name of a registered mock global function.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1119: testDone();
You should also call testDone on expect failures, which don't throw an assertion
that can be caught here.

[arv (Not doing code reviews), 2011-08-22 20:57:18]

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
File chrome/test/data/webui/async_gen.js (right):

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:22: TearDown: function() {
this should have been tearDown. Only constructors should start with an upper
case character.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:31: browsePreload: 'chrome://history/',
This seems a bit arbitrary. Care to add a comment?

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:75: self = this;
missing var

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:75: self = this;
self is not used

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:125: SetUp: function() {
setUp

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:135: RunTest: function() {
runTest

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:138: this,
Array.prototype.slice.call(arguments));
this slice is not needed. apply is arguments aware

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:7: **/
Close these JSDoc comments with */ instead of **/

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:43: log: print,
what about error and info?

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:125: PreLoad: function() {},
Can the names of these be fixed?

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:359: if (typeof(mockClass.prototype[func]) ==
'function') {
typeof should not have parentheses

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:860: * @extends {realMatcher}
This annotation is a bit strange

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:865: this.__proto__ = realMatcher;
This seems a bit hacky. How about?

var obj = Object.create(realMatcher);
obj.arguments_ = arguments;
obj.argumentMatches = SaveArgumentMatcher.prototype.argumentMatches;
return obj;

or

return {
  __proto__: realMatcher,
  arguments_: arguments,
  argumentMatches: SaveArgumentMatcher.prototype.argumentMatches
};

[Sheridan Rawlins, 2011-08-23 01:06:15]

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
File chrome/test/data/webui/async_gen.js (right):

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:22: TearDown: function() {
On 2011/08/22 20:57:18, arv wrote:
> this should have been tearDown. Only constructors should start with an upper
> case character.

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:31: browsePreload: 'chrome://history/',
Moved kDummyURL into WebUIBrowserTest and made DUMMY_URL constant for js.

Done.

On 2011/08/22 20:57:18, arv wrote:
> This seems a bit arbitrary. Care to add a comment?

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:75: self = this;
On 2011/08/22 20:57:18, arv wrote:
> self is not used

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:125: SetUp: function() {
On 2011/08/22 20:57:18, arv wrote:
> setUp

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:135: RunTest: function() {
On 2011/08/22 20:57:18, arv wrote:
> runTest

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:138: this,
Array.prototype.slice.call(arguments));
On 2011/08/22 20:57:18, arv wrote:
> this slice is not needed. apply is arguments aware

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:7: **/
On 2011/08/22 20:57:18, arv wrote:
> Close these JSDoc comments with */ instead of **/

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:43: log: print,
Only the window is needed, actually.  I nuked these in favor of another pattern
I saw where the window or this is passed to the function.  Let me know if that's
ok.

On 2011/08/22 20:57:18, arv wrote:
> what about error and info?

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:125: PreLoad: function() {},
On 2011/08/22 20:57:18, arv wrote:
> Can the names of these be fixed?

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:359: if (typeof(mockClass.prototype[func]) ==
'function') {
On 2011/08/22 20:57:18, arv wrote:
> typeof should not have parentheses

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:856: * then forces |this| to be an instance
of |realMatcher|.
I rewrote this with composition instead of hacky inheritance & adjusted the
comment.

On 2011/08/22 16:26:44, Matt Menke wrote:
> You're defining a class in terms of one of its methods, and an argument to
that
> method.  Reading this, I have no clue what this class actually does.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:860: * @extends {realMatcher}
See new version.

On 2011/08/22 20:57:18, arv wrote:
> This annotation is a bit strange

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:864: this.argumentMatches =
this.argumentMatches;
Yeah, this was weird - it could have been this.argumentMatches =
SaveArgumentMatcher.prototype.argumentMatches, but it's not needed with rewrite.

On 2011/08/22 16:26:44, Matt Menke wrote:
> ?

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:865: this.__proto__ = realMatcher;
See new version with composition instead of the hacky inheritance.

On 2011/08/22 20:57:18, arv wrote:
> This seems a bit hacky. How about?
> 
> var obj = Object.create(realMatcher);
> obj.arguments_ = arguments;
> obj.argumentMatches = SaveArgumentMatcher.prototype.argumentMatches;
> return obj;
> 
> or
> 
> return {
>   __proto__: realMatcher,
>   arguments_: arguments,
>   argumentMatches: SaveArgumentMatcher.prototype.argumentMatches
> };

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:895: * |realMatcher|.
OK.  Added more context to comments.

On 2011/08/22 16:26:44, Matt Menke wrote:
> This isn't terribly illuminating for someone trying to use this class.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:998: * Syntactic sugar for will() on a
Mock4JS.Mock.
On 2011/08/22 16:26:44, Matt Menke wrote:
> nit:  "for use with" make it clearer that this should be used for the argument
> to "will".

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1009: * Syntactic sugar for will() on a
Mock4JS.Mock.
On 2011/08/22 16:26:44, Matt Menke wrote:
> nit:  "for use with"

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1023: * expectations and still call the
global function.
On 2011/08/22 16:26:44, Matt Menke wrote:
> nit:  Say what it does, not just why it exists.

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1052: * Syntactic sugar for will() on a
Mock4JS.Mock.
On 2011/08/22 16:26:44, Matt Menke wrote:
> nit:  For use with.

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1056: *     method is invoked.
On 2011/08/22 16:26:44, Matt Menke wrote:
> nit:  Name of a registered mock global function.

Done.

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1119: testDone();
I don't think so. There should be a distinction between assert and expect
inasmuch as tests can continue after expect failures.  Since it is now possible
to defer a test and continue a test, the test should continue until either a
fatal error (assert) or the test calls testDone() regardless of whether its
complete and synchronous or spread across several calls.
On 2011/08/22 16:26:44, Matt Menke wrote:
> You should also call testDone on expect failures, which don't throw an
assertion
> that can be caught here.

[mmenke, 2011-08-23 01:36:09]

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1119: testDone();
On 2011/08/23 01:06:15, Sheridan Rawlins wrote:
> I don't think so. There should be a distinction between assert and expect
> inasmuch as tests can continue after expect failures.  Since it is now
possible
> to defer a test and continue a test, the test should continue until either a
> fatal error (assert) or the test calls testDone() regardless of whether its
> complete and synchronous or spread across several calls.
> On 2011/08/22 16:26:44, Matt Menke wrote:
> > You should also call testDone on expect failures, which don't throw an
> assertion
> > that can be caught here.
> 

Then should probably add a way to tell if there's been expect errors, so a test
can more easily tell if it should abort.  Yea, I know you had this before and I
said it wasn't necessary before...Well, now I think it is.  :)

[Sheridan Rawlins, 2011-08-23 05:52:18]

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1119: testDone();
Ok.  I added a whenTestDone enum to describe when to call testDone. Does this
satisfy your needs and is it 'readable' too?

On 2011/08/23 01:36:09, Matt Menke wrote:
> On 2011/08/23 01:06:15, Sheridan Rawlins wrote:
> > I don't think so. There should be a distinction between assert and expect
> > inasmuch as tests can continue after expect failures.  Since it is now
> possible
> > to defer a test and continue a test, the test should continue until either a
> > fatal error (assert) or the test calls testDone() regardless of whether its
> > complete and synchronous or spread across several calls.
> > On 2011/08/22 16:26:44, Matt Menke wrote:
> > > You should also call testDone on expect failures, which don't throw an
> > assertion
> > > that can be caught here.
> > 
> 
> Then should probably add a way to tell if there's been expect errors, so a
test
> can more easily tell if it should abort.  Yea, I know you had this before and
I
> said it wasn't necessary before...Well, now I think it is.  :)

[mmenke, 2011-08-23 15:26:28]

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/10011/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1119: testDone();
On 2011/08/23 05:52:18, Sheridan Rawlins wrote:
> Ok.  I added a whenTestDone enum to describe when to call testDone. Does this
> satisfy your needs and is it 'readable' too?
> 
> On 2011/08/23 01:36:09, Matt Menke wrote:
> > On 2011/08/23 01:06:15, Sheridan Rawlins wrote:
> > > I don't think so. There should be a distinction between assert and expect
> > > inasmuch as tests can continue after expect failures.  Since it is now
> > possible
> > > to defer a test and continue a test, the test should continue until either
a
> > > fatal error (assert) or the test calls testDone() regardless of whether
its
> > > complete and synchronous or spread across several calls.
> > > On 2011/08/22 16:26:44, Matt Menke wrote:
> > > > You should also call testDone on expect failures, which don't throw an
> > > assertion
> > > > that can be caught here.
> > > 
> > 
> > Then should probably add a way to tell if there's been expect errors, so a
> test
> > can more easily tell if it should abort.  Yea, I know you had this before
and
> I
> > said it wasn't necessary before...Well, now I think it is.  :)
> 

Seems reasonable to me.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/asyn...
File chrome/test/data/webui/async_gen.js (right):

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:56: GEN('    ~AsyncWebUIMessageHandler()
{}');
Couldn't these go in the inl file?

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/prin...
File chrome/test/data/webui/print_preview.js (right):

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/prin...
chrome/test/data/webui/print_preview.js:81: setUseCloudPrint(false, "");
nit:  Single quotes.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:152: return function() {
Might be a good idea for this function to pass along its arguments, too.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1045: * @param {Function} funcName The name
of the global function to call.
nit:  Would "string" be more accurate than "Function" here?  Same goes for
callGlobalWithSavedArgs.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1162: if (errors.length && this.whenTestDone_
>= WhenTestDone.EXPECT) {
Should use == here.  Otherwise, ALWAYS behaves a little oddly.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1164: return result;
Returning |result| when it's not the result of the final action seems a little
odd here.  Not sure on the best solution.  Maybe just move the errors.length
check out of this loop?

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1208: Array.prototype.slice.call(arguments));
You should remove |whenTestDone| from |arguments|.  Would be a good idea to have
a unit test or two for this.

[Sheridan Rawlins, 2011-08-24 02:33:53]

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/asyn...
File chrome/test/data/webui/async_gen.js (right):

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:56: GEN('    ~AsyncWebUIMessageHandler()
{}');
Sadly, clang bots hork it up unless it's in a .cc file (which the generated file
is, but the -inl.h file is not).

On 2011/08/23 15:26:29, Matt Menke wrote:
> Couldn't these go in the inl file?

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/prin...
File chrome/test/data/webui/print_preview.js (right):

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/prin...
chrome/test/data/webui/print_preview.js:81: setUseCloudPrint(false, "");
On 2011/08/23 15:26:29, Matt Menke wrote:
> nit:  Single quotes.

Done.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:152: return function() {
On 2011/08/23 15:26:29, Matt Menke wrote:
> Might be a good idea for this function to pass along its arguments, too.

Done.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1045: * @param {Function} funcName The name
of the global function to call.
On 2011/08/23 15:26:29, Matt Menke wrote:
> nit:  Would "string" be more accurate than "Function" here?  Same goes for
> callGlobalWithSavedArgs.

Done.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1162: if (errors.length && this.whenTestDone_
>= WhenTestDone.EXPECT) {
On 2011/08/23 15:26:29, Matt Menke wrote:
> Should use == here.  Otherwise, ALWAYS behaves a little oddly.

Done.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1164: return result;
On 2011/08/23 15:26:29, Matt Menke wrote:
> Returning |result| when it's not the result of the final action seems a little
> odd here.  Not sure on the best solution.  Maybe just move the errors.length
> check out of this loop?

Done.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1208: Array.prototype.slice.call(arguments));
On 2011/08/23 15:26:29, Matt Menke wrote:
> You should remove |whenTestDone| from |arguments|.  Would be a good idea to
have
> a unit test or two for this.

Done.

[mmenke, 2011-08-24 15:27:50]

LGTM, with the two "Important" comments addressed.

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/asyn...
File chrome/test/data/webui/async_gen.js (right):

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:56: GEN('    ~AsyncWebUIMessageHandler()
{}');
Might want to add a comment along those lines.

On 2011/08/24 02:33:53, Sheridan Rawlins wrote:
> Sadly, clang bots hork it up unless it's in a .cc file (which the generated
file
> is, but the -inl.h file is not).
> 
> On 2011/08/23 15:26:29, Matt Menke wrote:
> > Couldn't these go in the inl file?
>

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/asyn...
File chrome/test/data/webui/async_gen.js (right):

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:123: }, 123),
Think this test would be a little cleaner with strings instead of random ints. 
Maybe use 'val1' or 'var1' instead of 123, for example.

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:345: * Registers the global function name,
object and callback.
Important:  Add a comment that this can only be called before the
DOMContentLoaded event, and maybe add an error of some sort when it's done after
that.

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1092: function
callGlobalWithSavedArgs(savedArgs, funcName) {
Might be nice to have a way to have this record, and possibly end the test, in
response to any thrown errors.  Feel free to do this in a later CL, or not at
all.

Currently thinking just using continueTest will make my life simpler.

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1190: this.whenTestDone_ ==
WhenTestDone.ALWAYS) {
Important:  Should also abort on WhenTestDone.EXPECT here.

[Sheridan Rawlins, 2011-08-24 21:15:45]

Thanks so much for your review!  It's really been great to think these cases
through.

I added some more tests, and FWIW, you should be fine with just using
continueTest (you shouldn't need the global stuff, which was written with
PrintPreview in mind).

-SCR

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/asyn...
File chrome/test/data/webui/async_gen.js (right):

http://codereview.chromium.org/7645007/diff/17002/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:56: GEN('    ~AsyncWebUIMessageHandler()
{}');
On 2011/08/24 15:27:50, Matt Menke wrote:
> Might want to add a comment along those lines.
> 
> On 2011/08/24 02:33:53, Sheridan Rawlins wrote:
> > Sadly, clang bots hork it up unless it's in a .cc file (which the generated
> file
> > is, but the -inl.h file is not).
> > 
> > On 2011/08/23 15:26:29, Matt Menke wrote:
> > > Couldn't these go in the inl file?
> > 

Done.

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/asyn...
File chrome/test/data/webui/async_gen.js (right):

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/asyn...
chrome/test/data/webui/async_gen.js:123: }, 123),
First off, wow, what a coincidence that 123 fell on line 123...

But good point. 

Done.

On 2011/08/24 15:27:50, Matt Menke wrote:
> Think this test would be a little cleaner with strings instead of random ints.

> Maybe use 'val1' or 'var1' instead of 123, for example.

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:345: * Registers the global function name,
object and callback.
How 'bout this instead - I made it possible to mock globals at either preload or
postload time by checking the state of whether to defer the mocking.

Done.

On 2011/08/24 15:27:50, Matt Menke wrote:
> Important:  Add a comment that this can only be called before the
> DOMContentLoaded event, and maybe add an error of some sort when it's done
after
> that.

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1092: function
callGlobalWithSavedArgs(savedArgs, funcName) {
You can do this with runAllActionsAsync.  See example in 
WebUIBrowserAsyncGenTest.TestRegisterMockGlobals

I think that continueTest _will_ work nicely for you.  Most of the global
mocking was intended for print preview, which needs to preload some stuff and
wait for a global to be called to continue the test.  I wrote
WebUIBrowserAsyncGenDeferredToGlobalTest.TestDeferRunTestToGlobal to do what the
PrintPreview test will need to do.

On 2011/08/24 15:27:50, Matt Menke wrote:
> Might be nice to have a way to have this record, and possibly end the test, in
> response to any thrown errors.  Feel free to do this in a later CL, or not at
> all.
> 
> Currently thinking just using continueTest will make my life simpler.

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:1190: this.whenTestDone_ ==
WhenTestDone.ALWAYS) {
Ok.  How 'bout != NEVER.

Done.

On 2011/08/24 15:27:50, Matt Menke wrote:
> Important:  Should also abort on WhenTestDone.EXPECT here.

[mmenke, 2011-08-24 21:33:30]

And still LGTM, though I suggest you have the two locations were you overwrite
global functions share code.

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
File chrome/test/data/webui/test_api.js (right):

http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
chrome/test/data/webui/test_api.js:345: * Registers the global function name,
object and callback.
On 2011/08/24 21:15:45, Sheridan Rawlins wrote:
> How 'bout this instead - I made it possible to mock globals at either preload
or
> postload time by checking the state of whether to defer the mocking.
> 
> Done.
> 
> On 2011/08/24 15:27:50, Matt Menke wrote:
> > Important:  Add a comment that this can only be called before the
> > DOMContentLoaded event, and maybe add an error of some sort when it's done
> after
> > that.
> 

Sounds like a better idea to me.

[Sheridan Rawlins, 2011-08-25 00:42:59]

Done.

On 2011/08/24 21:33:30, Matt Menke wrote:
> And still LGTM, though I suggest you have the two locations were you overwrite
> global functions share code.
> 
>
http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
> File chrome/test/data/webui/test_api.js (right):
> 
>
http://codereview.chromium.org/7645007/diff/25002/chrome/test/data/webui/test...
> chrome/test/data/webui/test_api.js:345: * Registers the global function name,
> object and callback.
> On 2011/08/24 21:15:45, Sheridan Rawlins wrote:
> > How 'bout this instead - I made it possible to mock globals at either
preload
> or
> > postload time by checking the state of whether to defer the mocking.
> > 
> > Done.
> > 
> > On 2011/08/24 15:27:50, Matt Menke wrote:
> > > Important:  Add a comment that this can only be called before the
> > > DOMContentLoaded event, and maybe add an error of some sort when it's done
> > after
> > > that.
> > 
> 
> Sounds like a better idea to me.

[arv (Not doing code reviews), 2011-08-25 22:54:20]

LGTM