Closed Bug 1358395 Opened 4 years ago Closed 4 years ago

Permaorange in dom/tests/mochitest/general/test_interfaces.html and test_interfaces_secureContext.html on Windows for every VR property when Gecko 55 merges to beta on 2017-06-12

Categories

(Core :: WebVR, defect)

55 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr45 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 + fixed

People

(Reporter: philor, Assigned: daoshengmu)

Details

Attachments

(2 files)

One more bit of merge bustage coming up in June: because VR's enabled on Windows on beta/release, when before it was only Android, you have to tell dom/tests/mochitest/general/test_interfaces.html and test_interfaces_secureContext.html to expect that, so they don't scream about "If this is failing: DANGER, are you sure you want to expose the new interface VRStageParameters to all webpages" etc. like they are now in https://treeherder.mozilla.org/logviewer.html#?job_id=93220627&repo=try (ignore that first failure, that's separate permaorange).

[Tracking Requested - why for this release]: if it's not fixed by June, merge bustage, closed tree, delayed b1.
Assignee: nobody → dmu
Assignee: dmu → nobody
I think Kip is more suitable to take this. If I have time recently, I will be back to help this.
Tracking 55+ for this permaorange.
Hi Phil,
I refer Bug 1358010 Comment 0, trying to reproduce it locally. But, I find isRelease is still false at test_interfaces.html (https://dxr.mozilla.org/mozilla-central/rev/b8e9b674033bcd1f3a4c59b9d0ee7619c1a17cc5/dom/tests/mochitest/general/test_interfaces.js) after changing /config/milestone.txt from 55a1 to 55.

Do you have any suggestion to reproduce it locally?
Flags: needinfo?(philringnalda)
According to Bug 1343368, we wanna enable WebVR 1.1 APIs on Windows in release. Therefore, we have to do some modification for this test_interfaces.js.
Certainly possible that you have to clobber after changing the version number (clobbering is a part of doing the actual merge), and I wouldn't be the least bit surprised if artifact builds fail to notice the version number change.
Flags: needinfo?(philringnalda)
dom.gamepad.extensions needs to be enabled at Windows Release build for WebVR usage (https://dxr.mozilla.org/mozilla-central/rev/85e5d15c31691c89b82d6068c26260416493071f/gfx/vr/VRManager.cpp#95). It makes sense to enable dom.gamepad.extensions as well, and it has been tested at test_gamepad_extensions.html for a while and stable.
Comment on attachment 8868494 [details]
Bug 1358395 - Part 1: Enable WebVR APIs of test_interfaces for the release build;

https://reviewboard.mozilla.org/r/140108/#review144324

This LGTM, but will also need a DOM peer review for any changes in test_interfaces.html
Attachment #8868494 - Flags: review?(kgilbert) → review+
Comment on attachment 8869090 [details]
Bug 1358395 - Part 2: Enable GamepadExtensions Web API the for the release build;

https://reviewboard.mozilla.org/r/140714/#review144326

This LGMT, but will need a DOM peer review as well.
Attachment #8869090 - Flags: review?(kgilbert) → review+
Something I just caught, if this is going to ride the trains to release, you'll need to update your mochitest.ini for vr, as some of your tests are marked to skip if release or beta. You can make that a followup to this if you'd like, or just let me know if it's taken care of elsewhere.
Flags: needinfo?(dmu)
(In reply to Kyle Machulis [:qdot] [:kmachulis]  (if a patch has no decent commit message, automatic r-) from comment #17)
> Something I just caught, if this is going to ride the trains to release,
> you'll need to update your mochitest.ini for vr, as some of your tests are
> marked to skip if release or beta. You can make that a followup to this if
> you'd like, or just let me know if it's taken care of elsewhere.

I have already updated my mochitest.ini at Bug 1358010 for dom/vr/test/mochitest.ini. Thanks.
Flags: needinfo?(dmu)
Comment on attachment 8868494 [details]
Bug 1358395 - Part 1: Enable WebVR APIs of test_interfaces for the release build;

https://reviewboard.mozilla.org/r/140108/#review145600

::: dom/tests/mochitest/general/test_interfaces.js:1325
(Diff revision 4)
>              (entry.nightlyAndroid === !(isAndroid && isNightly) && isAndroid) ||
>              (entry.nonReleaseAndroid === !(isAndroid && !isRelease) && isAndroid) ||
>              (entry.xbl === !isXBLScope) ||
>              (entry.desktop === !isDesktop) ||
>              (entry.windows === !isWindows) ||
> +            (entry.nonReleaseWindows === !isRelease && !isWindows) ||

The name 'nonReleaseWindows' for this predicate is kind of hard to understand, and it sounds like you want it only on windows nightly/beta, similar to what the android nonReleaseAndroid is. Maybe releaseNonWindows?
Attachment #8868494 - Flags: review?(kyle) → review+
Comment on attachment 8869090 [details]
Bug 1358395 - Part 2: Enable GamepadExtensions Web API the for the release build;

https://reviewboard.mozilla.org/r/140714/#review145602
Attachment #8869090 - Flags: review?(kyle) → review+
Comment on attachment 8868494 [details]
Bug 1358395 - Part 1: Enable WebVR APIs of test_interfaces for the release build;

https://reviewboard.mozilla.org/r/140108/#review145600

> The name 'nonReleaseWindows' for this predicate is kind of hard to understand, and it sounds like you want it only on windows nightly/beta, similar to what the android nonReleaseAndroid is. Maybe releaseNonWindows?

Sounds good! Thanks for the suggestion.
Assignee: nobody → dmu
Pushed by dmu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/27d75c6c8b61
Part 1: Enable WebVR APIs of test_interfaces for the release build; r=kip,qdot
https://hg.mozilla.org/integration/autoland/rev/8de0faf80ef0
Part 2: Enable GamepadExtensions Web API the for the release build; r=kip,qdot
https://hg.mozilla.org/mozilla-central/rev/27d75c6c8b61
https://hg.mozilla.org/mozilla-central/rev/8de0faf80ef0
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.