1185171 - GeckoMediaPlugins.GMPOutputProtection gtest fails when run from a windows tester

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

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?

Comment 2

[Bob Owen (:bobowen)]

(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.

Comment 3

[Bob Owen (:bobowen)]

Attachment: 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

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

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!

Comment 5

[Bob Owen (:bobowen)]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/245b24210e41d2f7197a78302431144a01cbd0a0
changeset:  245b24210e41d2f7197a78302431144a01cbd0a0
user:       Bob Owen <[e-mail]>
date:       Wed Jul 22 09:29:52 2015 +0100
description:
Bug 1185171: Add 0xc02625e5 as a valid failure code for GMPOutputProtection test. r=cpearce

Comment 6

[Chris Manchester (limited bugmail, email directly)]

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!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/245b24210e41

Comment 8

[Bob Owen (:bobowen)]

(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.

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 10

[Chris Manchester (limited bugmail, email directly)]

I'm going to take a look at a build and test slave and see if I can figure out what's going on.

Comment 11

[Chris Manchester (limited bugmail, email directly)]

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?

Comment 12

[Bob Owen (:bobowen)]

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.

Comment 13

[Chris Manchester (limited bugmail, email directly)]

Attachment: 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

Comment 14

[Bob Owen (:bobowen)]

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!

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/84e68150485c

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/84e68150485c