Add tests for closing a frame within the scope of a getusermedia callback.

Add tests for closing a frame within the scope of a getusermedia callback.

BUG=472617, 474370
Committed: https://crrev.com/64aae326a6b32fdfba377dc5e060fd4713a2295b
Cr-Commit-Position: refs/heads/master@{#324633}

[perkj_chrome, 2015-04-09 14:32:05]

perkj@chromium.org changed reviewers:
+ magjed@chromium.org, phoglund@chromium.org

[perkj_chrome, 2015-04-09 14:32:05]

please?

[phoglund_chromium, 2015-04-09 15:05:12]

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
File content/test/data/media/getusermedia.html (right):

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
content/test/data/media/getusermedia.html:299: console.log("Removed Iframe.");
Remove this log statement.

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
content/test/data/media/getusermedia.html:302:
setTimeout(window.parent.eventOccured, 1);
Can we do something more useful, like assign the stream to a video element
outside the nuked iframe and wait for it to play? What's supposed to happen with
the stream, it it invalidated? If it should still work, I think that's a nice
way of doing something more meaningful than just sleeping a second.

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
content/test/data/media/getusermedia.html:302:
setTimeout(window.parent.eventOccured, 1);
Also, consider just calling testSuccessful or whatever it's called rather than
expect/occurred, I think that may be a bit clearer since there's only one event.

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
content/test/data/media/getusermedia.html:326: console.log("Removed Iframe.");
Remove

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
content/test/data/media/getusermedia.html:329:
setTimeout(window.parent.eventOccured, 1);
Same thing here.

[perkj_chrome, 2015-04-10 09:39:06]

PTAL

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
File content/test/data/media/getusermedia.html (right):

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
content/test/data/media/getusermedia.html:302:
setTimeout(window.parent.eventOccured, 1);
On 2015/04/09 15:05:12, phoglund wrote:
> Also, consider just calling testSuccessful or whatever it's called rather than
> expect/occurred, I think that may be a bit clearer since there's only one
event.

Done.

https://codereview.chromium.org/1073783003/diff/1/content/test/data/media/get...
content/test/data/media/getusermedia.html:329:
setTimeout(window.parent.eventOccured, 1);
On 2015/04/09 15:05:12, phoglund wrote:
> Same thing here.

moved this to reportTestSuccess to fix all test cases.

[phoglund_chromium, 2015-04-10 09:47:58]

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
File content/test/data/media/getusermedia.html (right):

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
content/test/data/media/getusermedia.html:295: console.log("getUserMedia
succeeded.");
Remove.

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
content/test/data/media/getusermedia.html:318: console.log("getUserMedia
failed.");
Remove.

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
File content/test/data/media/webrtc_test_utilities.js (right):

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
content/test/data/media/webrtc_test_utilities.js:29: // If a test is reported to
succeed and later a crash occurs
Well, it's not "in the scope of the timer", it's rather we enqueue a new
javascript task after the current task completes. What about phrasing it like
this:

This function enqueues sending an OK back to the test, rather than doing it
directly. We do this so we catch crashes that occur in the current execution
context, but after reportTestSuccess is invoked.

[perkj_chrome, 2015-04-10 09:57:29]

ptal

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
File content/test/data/media/getusermedia.html (right):

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
content/test/data/media/getusermedia.html:295: console.log("getUserMedia
succeeded.");
On 2015/04/10 09:47:58, phoglund wrote:
> Remove.

Done.

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
content/test/data/media/getusermedia.html:318: console.log("getUserMedia
failed.");
On 2015/04/10 09:47:58, phoglund wrote:
> Remove.

Done.

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
File content/test/data/media/webrtc_test_utilities.js (right):

https://codereview.chromium.org/1073783003/diff/40001/content/test/data/media...
content/test/data/media/webrtc_test_utilities.js:29: // If a test is reported to
succeed and later a crash occurs
On 2015/04/10 09:47:58, phoglund wrote:
> Well, it's not "in the scope of the timer", it's rather we enqueue a new
> javascript task after the current task completes. What about phrasing it like
> this:
> 
> This function enqueues sending an OK back to the test, rather than doing it
> directly. We do this so we catch crashes that occur in the current execution
> context, but after reportTestSuccess is invoked.

Done.

[phoglund_chromium, 2015-04-10 09:58:40]

lgtm (still one nit)

https://codereview.chromium.org/1073783003/diff/60001/content/test/data/media...
File content/test/data/media/webrtc_test_utilities.js (right):

https://codereview.chromium.org/1073783003/diff/60001/content/test/data/media...
content/test/data/media/webrtc_test_utilities.js:29: // This function enqueues
sending an OK back to the test, rather than doing
Nit: indent comments 2 steps.

[perkj_chrome, 2015-04-10 12:26:13]

Please?

[phoglund_chromium, 2015-04-10 12:37:16]

ship it!!

[perkj_chrome, 2015-04-10 12:49:44]

The CQ bit was checked by perkj@chromium.org

[perkj_chrome, 2015-04-10 12:49:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
phoglund@chromium.org
Link to the patchset: https://codereview.chromium.org/1073783003/#ps60002
(title: " ")

[commit-bot: I haz the power, 2015-04-10 12:50:16]

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

[commit-bot: I haz the power, 2015-04-10 14:51:52]

Committed patchset #7 (id:60002)

[commit-bot: I haz the power, 2015-04-10 14:53:12]

Patchset 7 (id:??) landed as
https://crrev.com/64aae326a6b32fdfba377dc5e060fd4713a2295b
Cr-Commit-Position: refs/heads/master@{#324633}