u2f tests broken in terms of their pref handling

RESOLVED FIXED in Firefox 48

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bz, Assigned: baku)

Tracking

Trunk
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

They're racy in that they assume that nothing enumerates the window before they've set their prefs, for example.  This is blocking other tests (e.g. bug 1255692) from landing because minor timing changes will make the u2f tests fail.

They're also using SpecialPowers to directly set their prefs and leaving them in random states (not to mention not really being e10s-friendly) after the test, instead of using pushPrefEnv like tests are supposed to.
Flags: needinfo?(jjones)
Flags: needinfo?(dkeeler)
Flags: needinfo?(amarchesini)
See bug 1255692 comment 5 for the sorts of problems this causes.  Basically, right now these tests are preventing any worker tests from being added to the tree.
(Assignee)

Comment 2

2 years ago
Created attachment 8729542 [details] [diff] [review]
u2f.patch
Assignee: nobody → amarchesini
Flags: needinfo?(jjones)
Flags: needinfo?(dkeeler)
Flags: needinfo?(amarchesini)
Attachment #8729542 - Flags: review?(bzbarsky)
Comment on attachment 8729542 [details] [diff] [review]
u2f.patch

Please have someone from bug 1231681 review this; I have too many other things on my plate right now, sorry.
Attachment #8729542 - Flags: review?(bzbarsky)
(Assignee)

Updated

2 years ago
Attachment #8729542 - Flags: review?(jjones)
Blocks: 1120715

Comment 4

2 years ago
If this can't land in a very short time and we don't have 100% confidence that it will fix the issues observed in the dependencies, but 1231681 needs to be backed out ASAP.
Flags: needinfo?(jjones)

Comment 5

2 years ago
Comment on attachment 8729542 [details] [diff] [review]
u2f.patch

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

Everything looks like the tests are still good. I'm sorry I didn't know about the Push/Pop pref handling a month ago!
Attachment #8729542 - Flags: review?(jjones) → review+

Updated

2 years ago
Flags: needinfo?(jjones)

Comment 7

2 years ago
Thanks for the quick fix guys!
Yes, thank you!
Component: DOM: Security → DOM: Device Interfaces

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8836b439d0b3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.