Move all the browser_devices* tests to browser/base/content/test/webrtc

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

(Depends on 1 bug)

unspecified
Firefox 50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(3 attachments)

We already have a couple browser_devices* tests and I expect a few more to come. I think it's time to move them to their own folder, e.g. browser/base/content/test/webrtc
Attachment #8764638 - Flags: review?(gijskruitbosch+bugs)
Attachment #8764639 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8764638 [details] [diff] [review]
part 2 - merge get_user_media_helpers.js into head.js

Review of attachment 8764638 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me on the assumption that the content added to head.js is essentially identical (haven't checked, but you told me it essentially is).
Attachment #8764638 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8764636 [details] [diff] [review]
part 1 - move browser_devices_* tests to a webrtc folder

Review of attachment 8764636 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/webrtc/head.js
@@ +1,4 @@
> +Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> +
> +XPCOMUtils.defineLazyModuleGetter(this, "Promise",
> +  "resource://gre/modules/Promise.jsm");

Can we do a followup patch where we nuke this from orbit...

@@ +2,5 @@
> +
> +XPCOMUtils.defineLazyModuleGetter(this, "Promise",
> +  "resource://gre/modules/Promise.jsm");
> +
> +function waitForCondition(condition, nextTest, errorMsg, retryTimes) {

and replace this with the BrowserTestUtils equivalent

@@ +25,5 @@
> +  }, 100);
> +  var moveOn = function() { clearInterval(interval); nextTest(); };
> +}
> +
> +function promiseWaitForCondition(aConditionFn) {

getting rid of this as well.
Attachment #8764636 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8764639 [details] [diff] [review]
part 3 - Cleanup

Review of attachment 8764639 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me but please note the license notes.

::: browser/base/content/test/webrtc/browser_devices_get_user_media_in_frame.js
@@ -1,3 @@
> -/* This Source Code Form is subject to the terms of the Mozilla Public
> - * License, v. 2.0. If a copy of the MPL was not distributed with this
> - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Err, do we know that we can do this license change, ie are all the contributors to this file moco employees?

::: browser/base/content/test/webrtc/head.js
@@ +1,2 @@
> +/* Any copyright is dedicated to the Public Domain.
> +   http://creativecommons.org/publicdomain/zero/1.0/ */

Don't need a license in test files.
Attachment #8764639 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #7)

> browser/base/content/test/webrtc/browser_devices_get_user_media_in_frame.js
> @@ -1,3 @@
> > -/* This Source Code Form is subject to the terms of the Mozilla Public
> > - * License, v. 2.0. If a copy of the MPL was not distributed with this
> > - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> Err, do we know that we can do this license change, ie are all the
> contributors to this file moco employees?

For actual code contributions (ie. writing code), yes all contributors are moco employees. For tree-wide search & replace changes, it's less clear.
Depends on: 1282394
You need to log in before you can comment on or make changes to this bug.