Refactoring layout tests for AudioContext.suspend() and resume()

This patch includes a fix and refactoring on several layout tests for promise from AudioContext.suspend() and .resume().

|Audit| task runner handles the sequential execution of asynchrous test tasks.

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

[hongchan, 2015-02-20 21:19:16]

hongchan@chromium.org changed reviewers:
+ rtoy@chromium.org

[hongchan, 2015-02-20 21:21:28]

PTAL.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
File LayoutTests/webaudio/audiocontext-suspend-resume-expected.txt (right):

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-suspend-resume-expected.txt:10: PASS p2 =
context.resume() did not throw exception.
The variable |p2| was not used previously.

[Raymond Toy, 2015-02-20 21:35:14]

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
File LayoutTests/webaudio/audiocontext-close.html (right):

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:8: </head>
Unnecessary indentation change?

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:26: // Task 1: test online context
(1).
Unless you need to, I would not say "Task 1: ...".  Just "Task: ..."

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:55: // Context is released, so
verify that we cannot create any more nodes,
"released" -> "closed"

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:152:
shouldBeEqualToString("offline.state", "closed");
I wonder if we should get the object (and state) from the event object instead
of using the global offline context object.

WDYT?

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:163: finishJSTest();
Does the order done() and finishJSTest() matter?

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
File LayoutTests/webaudio/audiocontext-suspend-resume.html (right):

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-suspend-resume.html:8: </head>
Unneeded indentation change?

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-suspend-resume.html:90: // We don't care about
the actual result of the offline rendering.
This seems indented incorrectly or inconsistently.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/offline...
File LayoutTests/webaudio/offlineaudiocontext-promise.html (right):

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/offline...
LayoutTests/webaudio/offlineaudiocontext-promise.html:6: <script
src="resources/audio-testing.js"></script>
Does the order matter?

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/offline...
LayoutTests/webaudio/offlineaudiocontext-promise.html:30:
testFailed("AudioBuffer returned by oncomplete and promise are NOT identical");
I don't think this change should be part of this CL. Can you undo this, and
create a new CL to fix this?

[hongchan, 2015-02-20 22:20:09]

For offlinecontext-promise.html, I will file another CL. I think the test also
needs a bit of rearrangement. (including typo, linebreak and etc.)

WDYT?

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
File LayoutTests/webaudio/audiocontext-close.html (right):

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:8: </head>
On 2015/02/20 at 21:35:13, Raymond Toy wrote:
> Unnecessary indentation change?

As we discussed, we keep the indentation.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:26: // Task 1: test online context
(1).
On 2015/02/20 at 21:35:13, Raymond Toy wrote:
> Unless you need to, I would not say "Task 1: ...".  Just "Task: ..."

Done.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:55: // Context is released, so
verify that we cannot create any more nodes,
On 2015/02/20 at 21:35:13, Raymond Toy wrote:
> "released" -> "closed"

Done.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:152:
shouldBeEqualToString("offline.state", "closed");
On 2015/02/20 at 21:35:13, Raymond Toy wrote:
> I wonder if we should get the object (and state) from the event object instead
of using the global offline context object.
> 
> WDYT?

I can always agree with the idea of not using globals. Done.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-close.html:163: finishJSTest();
On 2015/02/20 at 21:35:13, Raymond Toy wrote:
> Does the order done() and finishJSTest() matter?

If I understand correctly, finishJSTest() signals the testRunner to shut down
the current one and move on to the next one. I would like to close the Audio
task runner properly before moving on.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
File LayoutTests/webaudio/audiocontext-suspend-resume.html (right):

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-suspend-resume.html:8: </head>
On 2015/02/20 at 21:35:14, Raymond Toy wrote:
> Unneeded indentation change?

We agreed to keep the indentation as shown above.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/audioco...
LayoutTests/webaudio/audiocontext-suspend-resume.html:90: // We don't care about
the actual result of the offline rendering.
On 2015/02/20 at 21:35:13, Raymond Toy wrote:
> This seems indented incorrectly or inconsistently.

I see. I naively put the comment based on the original position. Done.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/offline...
File LayoutTests/webaudio/offlineaudiocontext-promise.html (right):

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/offline...
LayoutTests/webaudio/offlineaudiocontext-promise.html:6: <script
src="resources/audio-testing.js"></script>
On 2015/02/20 at 21:35:14, Raymond Toy wrote:
> Does the order matter?

Just to make it consistent and by the order of scope: 1) JS layout test library
2) audio context compatibility check 3) web audio specific testing utility.

https://codereview.chromium.org/945023002/diff/1/LayoutTests/webaudio/offline...
LayoutTests/webaudio/offlineaudiocontext-promise.html:30:
testFailed("AudioBuffer returned by oncomplete and promise are NOT identical");
On 2015/02/20 at 21:35:14, Raymond Toy wrote:
> I don't think this change should be part of this CL. Can you undo this, and
create a new CL to fix this?

Will do!

[hongchan, 2015-02-20 22:23:04]

Sorry, I missed one change in audiocontext-suspend-resume.html. Will do it in
the next patch.

https://codereview.chromium.org/945023002/diff/20001/LayoutTests/webaudio/aud...
File LayoutTests/webaudio/audiocontext-close.html (right):

https://codereview.chromium.org/945023002/diff/20001/LayoutTests/webaudio/aud...
LayoutTests/webaudio/audiocontext-close.html:152:
shouldBeEqualToString("offline.state", "closed");
I will do this in the next patch set. Sorry for missing this change.

[hongchan, 2015-02-20 22:55:38]

Sorry for the spamming. 

Fixed to check 'event' object from Promise argument, not the global context
object.

[Raymond Toy, 2015-02-23 17:26:43]

lgtm with nits

https://codereview.chromium.org/945023002/diff/40001/LayoutTests/webaudio/aud...
File LayoutTests/webaudio/audiocontext-close.html (right):

https://codereview.chromium.org/945023002/diff/40001/LayoutTests/webaudio/aud...
LayoutTests/webaudio/audiocontext-close.html:152:
shouldBeEqualToString("event.target.state", "closed");
Does this actually work?  Doesn't event need to be global?

[hongchan, 2015-02-23 17:38:05]

On 2015/02/23 at 17:26:43, rtoy wrote:
> lgtm with nits
> 
>
https://codereview.chromium.org/945023002/diff/40001/LayoutTests/webaudio/aud...
> File LayoutTests/webaudio/audiocontext-close.html (right):
> 
>
https://codereview.chromium.org/945023002/diff/40001/LayoutTests/webaudio/aud...
> LayoutTests/webaudio/audiocontext-close.html:152:
shouldBeEqualToString("event.target.state", "closed");
> Does this actually work?  Doesn't event need to be global?

The anonymous function always falls under the global scope. So technically the
|event| will be in the global scope. But I can be more specific by using
|Should| exception checker. Should I?

[Raymond Toy, 2015-02-23 17:50:12]

On 2015/02/23 17:38:05, hoch wrote:
> On 2015/02/23 at 17:26:43, rtoy wrote:
> > lgtm with nits
> > 
> >
>
https://codereview.chromium.org/945023002/diff/40001/LayoutTests/webaudio/aud...
> > File LayoutTests/webaudio/audiocontext-close.html (right):
> > 
> >
>
https://codereview.chromium.org/945023002/diff/40001/LayoutTests/webaudio/aud...
> > LayoutTests/webaudio/audiocontext-close.html:152:
> shouldBeEqualToString("event.target.state", "closed");
> > Does this actually work?  Doesn't event need to be global?
> 
> The anonymous function always falls under the global scope. So technically the
> |event| will be in the global scope. But I can be more specific by using
> |Should| exception checker. Should I?

Ah. I'd just leave it, since we use should* everywhere else

[hongchan, 2015-02-23 17:52:41]

On 2015/02/23 at 17:50:12, rtoy wrote:
> On 2015/02/23 17:38:05, hoch wrote:
> > On 2015/02/23 at 17:26:43, rtoy wrote:
> > > lgtm with nits
> > > 
> > >
> >
https://codereview.chromium.org/945023002/diff/40001/LayoutTests/webaudio/aud...
> > > File LayoutTests/webaudio/audiocontext-close.html (right):
> > > 
> > >
> >
https://codereview.chromium.org/945023002/diff/40001/LayoutTests/webaudio/aud...
> > > LayoutTests/webaudio/audiocontext-close.html:152:
> > shouldBeEqualToString("event.target.state", "closed");
> > > Does this actually work?  Doesn't event need to be global?
> > 
> > The anonymous function always falls under the global scope. So technically
the
> > |event| will be in the global scope. But I can be more specific by using
> > |Should| exception checker. Should I?
> 
> Ah. I'd just leave it, since we use should* everywhere else

Thanks for the review! Starting try bots...

[hongchan, 2015-02-23 21:12:54]

The CQ bit was checked by hongchan@chromium.org

[commit-bot: I haz the power, 2015-02-23 21:13:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/945023002/40001

[commit-bot: I haz the power, 2015-02-23 22:45:04]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190694