Make w3cwrapper.js's done() match the semantics in testharness.js

Make w3cwrapper.js's done() match the semantics in testharness.js

This should fix any tests where done() can be called multiple times.

BUG=333880
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169720

[ddorwin, 2014-03-20 17:25:03]

Thanks! LGTM with nit (and questions/discussion).

https://codereview.chromium.org/205893003/diff/1/LayoutTests/media/W3C/w3cwra...
File LayoutTests/media/W3C/w3cwrapper.js (right):

https://codereview.chromium.org/205893003/diff/1/LayoutTests/media/W3C/w3cwra...
LayoutTests/media/W3C/w3cwrapper.js:19: // match the semantics of testharness.js
done(), where nothing that
nit: Match

https://codereview.chromium.org/205893003/diff/1/LayoutTests/media/W3C/w3cwra...
LayoutTests/media/W3C/w3cwrapper.js:20: // happens after that call has any
effect on the test result.
Does this mean that W3C tests assume the test terminates but other Blink layout
tests do not? If so, the latter seems broken but would probably be much riskier
to fix.

Or maybe this particular test is broken because it does not clear the event
handler or set some variable the first time it is called. Or maybe the W3C
testharness.js takes on this responsibility where as Blink tests have been
written to clean themselves up.

[philipj_slow, 2014-03-21 03:00:52]

https://codereview.chromium.org/205893003/diff/1/LayoutTests/media/W3C/w3cwra...
File LayoutTests/media/W3C/w3cwrapper.js (right):

https://codereview.chromium.org/205893003/diff/1/LayoutTests/media/W3C/w3cwra...
LayoutTests/media/W3C/w3cwrapper.js:19: // match the semantics of testharness.js
done(), where nothing that
On 2014/03/20 17:25:03, ddorwin wrote:
> nit: Match

Done.

https://codereview.chromium.org/205893003/diff/1/LayoutTests/media/W3C/w3cwra...
LayoutTests/media/W3C/w3cwrapper.js:20: // happens after that call has any
effect on the test result.
On 2014/03/20 17:25:03, ddorwin wrote:
> Does this mean that W3C tests assume the test terminates but other Blink
layout
> tests do not? If so, the latter seems broken but would probably be much
riskier
> to fix.

It means that assertions or exceptions after t.done() in testharness.js
async_test are ignored. This test would pass:

async_test(function(t) {
  t.done();
  assert_unreached();
  throw new Error();
});

> Or maybe this particular test is broken because it does not clear the event
> handler or set some variable the first time it is called. Or maybe the W3C
> testharness.js takes on this responsibility where as Blink tests have been
> written to clean themselves up.

The test looked fine to me. It's just that since testharness.js has this
behavior, tests don't need to worry about clearing event handlers or anything
like that. I assume that tests written for each framework work as they should,
for the most part. (FWIW, I prefer testharness.js, precisely because you don't
need to worry about what happens after the test is finished.)

[philipj_slow, 2014-03-21 03:00:58]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-21 03:01:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/205893003/20001

[commit-bot: I haz the power, 2014-03-21 03:02:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-21 03:02:38]

Try jobs failed on following builders:
  tryserver.blink on win_blink_compile_dbg

[philipj_slow, 2014-03-21 04:18:30]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-21 04:18:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/205893003/20001

[commit-bot: I haz the power, 2014-03-21 04:59:35]

Change committed as 169720