GeckoMediaPlugins.GMPOutputProtection gtest fails when run from a windows tester

RESOLVED FIXED in Firefox 42

Status

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: chmanchester, Assigned: bobowen)

Tracking

unspecified
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed, firefox43 fixed)

Details

Attachments

(2 attachments)

I have a set of patches for bug 992983 that get gtests running from the test package. The only hiccup is a failing test on windows:


22:46:25     INFO -  TEST-S[GMP 728] WARNING: pipe error: 109: file c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 333
22:46:25     INFO -  TART | GeckoMediaPlugins.GMPOutputProtection
22:46:25     INFO -  [GMP 728] WARNING: pipe error: 109: file c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 333
22:46:25  WARNING -  TEST-UNEXPECTED-FAIL | GeckoMediaPlugins.GMPOutputProtection | Value of: msg.get()
22:46:25     INFO -    Actual: "FAIL EnumDisplayDevicesA call failed"
22:46:25     INFO -  Expected: mExpected[0].mMessage.get()
22:46:25     INFO -  Which is: "OP tests completed" @ c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/dom/media/gtest/TestGMPCrossOrigin.cpp:1321
22:46:25  WARNING -  TEST-UNEXPECTED-FAIL | GeckoMediaPlugins.GMPOutputProtection | Value of: msg.get()
22:46:25     INFO -    Actual: "FAIL OPMGetVideoOutputsFromHMONITOR call failed: HRESULT=0xc02625e5"
22:46:25     INFO -  Expected: mExpected[0].mMessage.get()
22:46:25     INFO -  Which is: "OP tests completed" @ c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/dom/media/gtest/TestGMPCrossOrigin.cpp:1321
22:46:25  WARNING -  TEST-UNEXPECTED-FAIL | GeckoMediaP[GMP 3024] WARNING: pipe error: 109: file c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 333
22:46:25     INFO -  lugins.GMPOutputProtection | test completed (time: 20ms)


The same test runs and passes when run from a builder during make check.
(Reporter)

Comment 1

4 years ago
This appears to be a result of missing a physical monitor on our test machines (HRESULT=0xc02625e5 is ERROR_GRAPHICS_NO_MONITORS_CORRESPOND_TO_DISPLAY_DEVICE), but I'm not really sure why this test passes when it runs from a build machine.

Bob, do you have any input on how to resolve this? Is it possible to test this meaningfully without an actual monitor?
Flags: needinfo?(bobowen.code)
(Assignee)

Comment 2

4 years ago
(In reply to Chris Manchester [:chmanchester] from comment #1)
> This appears to be a result of missing a physical monitor on our test
> machines (HRESULT=0xc02625e5 is
> ERROR_GRAPHICS_NO_MONITORS_CORRESPOND_TO_DISPLAY_DEVICE), but I'm not really
> sure why this test passes when it runs from a build machine.
> 
> Bob, do you have any input on how to resolve this? Is it possible to test
> this meaningfully without an actual monitor?

Yes, we could only test so much of the output protection setup, because it is not well documented and because of these sort of device issues on test machines.
I had to add a couple of valid failure codes, which is why it works (up to a point) on the build machines.
Not very satisfactory, but it was the best I could do.

This sounds like another valid failure, so I'll add it in.
Flags: needinfo?(bobowen.code)
(Assignee)

Comment 3

4 years ago
Created attachment 8636511 [details]
MozReview Request: Bug 1185171: Add 0xc02625e5 as a valid failure code for GMPOutputProtection test. r?cpearce

Bug 1185171: Add 0xc02625e5 as a valid failure code for GMPOutputProtection test. r?cpearce
Attachment #8636511 - Flags: review?(cpearce)
Comment on attachment 8636511 [details]
MozReview Request: Bug 1185171: Add 0xc02625e5 as a valid failure code for GMPOutputProtection test. r?cpearce

https://reviewboard.mozilla.org/r/13683/#review12345

Ship It!
Attachment #8636511 - Flags: review?(cpearce) → review+
(Assignee)

Comment 5

3 years ago
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/245b24210e41d2f7197a78302431144a01cbd0a0
changeset:  245b24210e41d2f7197a78302431144a01cbd0a0
user:       Bob Owen <bobowencode@gmail.com>
date:       Wed Jul 22 09:29:52 2015 +0100
description:
Bug 1185171: Add 0xc02625e5 as a valid failure code for GMPOutputProtection test. r=cpearce
(Assignee)

Updated

3 years ago
Assignee: nobody → bobowen.code
Status: NEW → ASSIGNED
(Reporter)

Comment 6

3 years ago
I ran this through try and found EnumDisplayDevices is still failing and failing the test. Is this a valid failure as well? Thanks for looking into this!
https://hg.mozilla.org/mozilla-central/rev/245b24210e41
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
(Assignee)

Comment 8

3 years ago
(In reply to Chris Manchester [:chmanchester] from comment #6)
> I ran this through try and found EnumDisplayDevices is still failing and
> failing the test. Is this a valid failure as well? Thanks for looking into
> this!

Ah, didn't realise there was an earlier failure.
That one returns a bool, so maybe this test doesn't really make any sense for this particular set-up.
I don't understand why it would fail on the test machines, but not the build ones.

Chris (P) - maybe we could look at disabling this test now? Or maybe add a check for display devices somehow.
Disabling would be quicker and I don't have time to look at this at the moment.
Status: RESOLVED → REOPENED
Flags: needinfo?(cpearce)
Resolution: FIXED → ---
I'm not keen on disabling this test; we really don't want Output Protection to regress, especially without us knowing about it, our EME solution would be uncompetitive without it.
Flags: needinfo?(cpearce)
(Reporter)

Updated

3 years ago
Depends on: 1189048
(Reporter)

Updated

3 years ago
Depends on: 1189049
(Reporter)

Comment 10

3 years ago
I'm going to take a look at a build and test slave and see if I can figure out what's going on.
(Reporter)

Comment 11

3 years ago
I made a little progress comparing build/test machines. Here's the result of calling EnumDisplayDevices in a loop on the builder (where this test runs today):

> Device Name: \\.\DISPLAY1
> Device String: Standard VGA Graphics Adapter
> State Flags: 15
> DeviceID: PCI\VEN_102B&DEV_0532&SUBSYS_060515D9&REV_0A
> DeviceKey: ...\Control\Video\{B003ECEE-2EB0-4275-B246-79D8E21FA25D}\0000

>    Device Name: \\.\DISPLAY1\Monitor0
>    Device String: Generic Non-PnP Monitor
>    State Flags: 3
>    DeviceID: Monitor\Default_Monitor\{4d36e96e-e325-11ce-bfc1-08002be10318
>    DeviceKey: ...\Control\Class\{4d36e96e-e325-11ce-bfc1-08002be10318}\000


> Device Name: \\.\DISPLAYV1
> Device String: RDPDD Chained DD
> State Flags: 8
> DeviceID:
> DeviceKey: ...\Control\Video\{DEB039CC-B704-4F53-B43E-9DD4432FA2E9}\0000


> Device Name: \\.\DISPLAYV2
> Device String: RDP Encoder Mirror Driver
> State Flags: 200008
> DeviceID:
> DeviceKey: ...\Control\Video\{42cf9257-1d96-4c9d-87f3-0d8e74595f78}\0000


> Device Name: \\.\DISPLAYV3
> Device String: RDP Reflector Display Driver
> State Flags: 200008
> DeviceID:
> DeviceKey: ...\Control\Video\{b043b95c-5670-4f10-b934-8ed0c8eb59a8}\0000

The EnumDisplayMonitors callback is called once during the test for "DISPLAY1", so EnumDisplayDevices for this adapter finds "DISPLAY1\Monitor0" and succeeds.

And here's what calling EnumDisplayDevices in a loop looks like on the tester:

> Device Name: \\.\DISPLAY1
> Device String: Microsoft Basic Display Adapter
> sState Flags: 0
> sDeviceID: PCI\VEN_102B&DEV_0532&SUBSYS_060E15D9&REV_0A
> sDeviceKey: ...\Control\Video\{A222B962-CD8B-4E55-9A0C-BA990B1C207E}\0000

>     Device Name: \\.\DISPLAY1\Monitor0
>     Device String: Generic Non-PnP Monitor
>     sState Flags: 2
>     sDeviceID: MONITOR\Default_Monitor\{4d36e96e-e325-11ce-bfc1-08002be10318}\00
00
>     sDeviceKey: ...\Control\Class\{4d36e96e-e325-11ce-bfc1-08002be10318}\0000


> Device Name: \\.\DISPLAY2
> Device String: NVIDIA GeForce GT 610
> sState Flags: 5
> sDeviceID: PCI\VEN_10DE&DEV_104A&SUBSYS_26153842&REV_A1
> sDeviceKey: ...\Control\Video\{6EFFFF15-30BD-4129-947E-2D539603C80C}\0000


> Device Name: \\.\DISPLAY3
> Device String: NVIDIA GeForce GT 610
> sState Flags: 0
> sDeviceID: PCI\VEN_10DE&DEV_104A&SUBSYS_26153842&REV_A1
> sDeviceKey: ...\Control\Video\{6EFFFF15-30BD-4129-947E-2D539603C80C}\0001

On this machine the EnumDisplayMonitors callback is called once during the test for "DISPLAY2", and the subsequent EnumDisplayDevices call fails because there aren't any devices associated with the adapter. This video card is selected as the primary display (the "Show desktop only on 2" option is selected in control panel).

My question at this point is whether the call to EnumDisplayDevices as it happens today on the builder is providing significant coverage. If not, it looks like we could move it after the call to sOPMGetVideoOutputsFromHMONITORProc (we would give up after that due to the lack of monitor, but before failing on EnumDisplayDevices).

Does this seem like an acceptable change?
Flags: needinfo?(bobowen.code)
(Assignee)

Comment 12

3 years ago
Thanks for doing this investigation and sorry that I've not had any more time to look into it.

(In reply to Chris Manchester [:chmanchester] from comment #11)

> My question at this point is whether the call to EnumDisplayDevices as it
> happens today on the builder is providing significant coverage. If not, it
> looks like we could move it after the call to
> sOPMGetVideoOutputsFromHMONITORProc (we would give up after that due to the
> lack of monitor, but before failing on EnumDisplayDevices).
> 
> Does this seem like an acceptable change?

This sounds perfectly reasonable to me in the circumstances.
They are only done in this order because of an example and I think the OPMGetVideoOutputsFromHMONITOR call is probably more important.
We would still get the more full coverage when the test was run manually.

It would be great to try and revisit this test and make it less fragile and also cover more of the OPM APIs, but other things are more pressing at the moment.
Flags: needinfo?(bobowen.code)
(Reporter)

Comment 13

3 years ago
Created attachment 8647105 [details]
MozReview Request: Bug 1185171 - Modify gmp-test-output-protection.h to prevent failure on machines without a physical monitor attached. r=bobowen

Bug 1185171 - Modify gmp-test-output-protection.h to prevent failure on machines without a physical monitor attached. r=bobowen
Attachment #8647105 - Flags: review?(bobowen.code)
(Assignee)

Comment 14

3 years ago
Comment on attachment 8647105 [details]
MozReview Request: Bug 1185171 - Modify gmp-test-output-protection.h to prevent failure on machines without a physical monitor attached. r=bobowen

https://reviewboard.mozilla.org/r/15899/#review14239

Ship It!
Attachment #8647105 - Flags: review?(bobowen.code) → review+
https://hg.mozilla.org/mozilla-central/rev/84e68150485c
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.