[e10s] Various media tests timeout on WinXP debug

RESOLVED FIXED in Firefox 47

Status

()

defect
P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: RyanVM, Assigned: jwwang)

Tracking

(Blocks 1 bug)

Trunk
mozilla48
Unspecified
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(e10s?, firefox46 wontfix, firefox47 fixed, firefox48 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

3 years ago
Posted file test logs
This is testing with e10s forced on (we're trying to get all platforms running e10s tests in CI). Only WinXP debug builds appear to be affected (opt builds ran the tests fine). I retriggered 5 times and hit the same failures each time (and the timeouts were always at the same point of each test, so I don't think this is just a "test run slow" issue).

I do see the following in the logs in each of the timing out tests, not sure if it's relevant or not, though. Any thoughts, Anthony?

08:25:41     INFO -  [Child 2532] WARNING: Decoder=bd86030 ReadMetadata failed, rv=80004005 HasValidMedia=0: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/MediaDecoderReader.cpp, line 219
08:25:41     INFO -  [Child 2532] WARNING: Decoder=bd86030 Decode metadata failed, shutting down decoder: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp, line 2005
08:25:41     INFO -  [Child 2532] WARNING: Decoder=bd86030 Decode error, changed state to ERROR: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp, line 1909

https://treeherder.mozilla.org/logviewer.html#?job_id=17670384&repo=try
Flags: needinfo?(ajones)
Blake - can we get some help from JW on this one?
Flags: needinfo?(bwu)
Assignee

Comment 2

3 years ago
I am on PTO today will need to go out soon.

I see lots of these messages in the logs:
WARNING: Failed to load WMF DLLs: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/platforms/wmf/WMFUtils.cpp, line 169

It looks like some DLLs are not installed on that machine.
Flags: needinfo?(cpearce)
Flags: needinfo?(bwu)
Flags: needinfo?(ajones)
(In reply to JW Wang [:jwwang] from comment #2)
> I am on PTO today will need to go out soon.
> 
> I see lots of these messages in the logs:
> WARNING: Failed to load WMF DLLs: file
> c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/media/
> platforms/wmf/WMFUtils.cpp, line 169
> 
> It looks like some DLLs are not installed on that machine.

Yes. WMF is not available on Windows XP. I imagine we're hitting that code path inside HTMLMediaElement.canPlayType(), when we query whether MP4/H.264/AAC is supported. I expected that we'd cache the result though so I wouldn't expect us to keep seeing error logging.

I do however expect DirectShow to be installed on those machines, so the errors for DirectShowReader failing to load are unexpected.
Flags: needinfo?(cpearce)
Assignee

Comment 4

3 years ago
I wonder why it happened on debug builds only. Could it be some kind of sandbox issue that fails to load the DLLs?
(In reply to JW Wang [:jwwang] from comment #4)
> I wonder why it happened on debug builds only. Could it be some kind of
> sandbox issue that fails to load the DLLs?

In the DirectShow case, the error is in a call stack inside an #ifdef DEBUG chunk:

http://mxr.mozilla.org/mozilla-central/source/dom/media/directshow/DirectShowReader.cpp#112

I wrote that code years ago, and IIRC its there so that you can attach the GraphEdit program to Firefox and see the filter graph that has been constructed. So we could probably just change that code to not error out on failure, or comment out or even remove that code entirely.
e10s -> p1
Priority: -- → P1
Assignee

Updated

3 years ago
Attachment #8727287 - Flags: review?(cpearce)
Reporter

Comment 8

3 years ago
Comment on attachment 8727287 [details]
MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce.

Looks like the patch works!
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b9aa7a1d715&group_state=expanded

Now I've got a leak, but at least the tests all run! This patch also fixed a permafail in the web-platform-tests previous runs were showing.
Attachment #8727287 - Flags: feedback+
Assignee

Comment 9

3 years ago
Comment on attachment 8727287 [details]
MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38403/diff/1-2/
Attachment #8727287 - Flags: feedback+ → review?(cpearce)
Comment on attachment 8727287 [details]
MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce.

https://reviewboard.mozilla.org/r/38403/#review35251

Would also accept just #ifdef'ing on something other than DEBUG, something that's not defined. That way the code isn't explicitly removed, just never built, as someday it may prove useful again.
Attachment #8727287 - Flags: review?(cpearce) → review+
Assignee

Comment 11

3 years ago
Thanks!
Assignee

Comment 12

3 years ago
How about |#ifdef DIRECTSHOW_REGISTER_GRAPH|?
Assignee

Comment 13

3 years ago
https://hg.mozilla.org/mozilla-central/file/be593a64d7c6a826260514fe758ef32a6ee580f7/dom/media/directshow/DirectShowUtils.cpp#l82

Somehow runningObjectTable->Register() returns hr=80070005 (E_ACCESSDENIED) only in e10s mode. We don't have such an error in non-e10s mode.

Hi Chris,
Do you have any idea about that?
Flags: needinfo?(cpearce)
Assignee

Comment 14

3 years ago
I will check the patch so we can move forward and open follow up bugs if necessary.
Assignee: nobody → jwwang
(In reply to JW Wang [:jwwang] from comment #12)
> How about |#ifdef DIRECTSHOW_REGISTER_GRAPH|?

Sounds good. Thanks!

(In reply to JW Wang [:jwwang] from comment #13)
> https://hg.mozilla.org/mozilla-central/file/
> be593a64d7c6a826260514fe758ef32a6ee580f7/dom/media/directshow/
> DirectShowUtils.cpp#l82
> 
> Somehow runningObjectTable->Register() returns hr=80070005 (E_ACCESSDENIED)
> only in e10s mode. We don't have such an error in non-e10s mode.
> 
> Hi Chris,
> Do you have any idea about that?

I don't know... Maybe there's some setup that we're not doing in e10s mode? I think we can just move on, we can fix it if and when we need to debug DirectShow again.
Flags: needinfo?(cpearce)

Comment 17

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/084c4f8baf48
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee

Comment 18

3 years ago
Comment on attachment 8727287 [details]
MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce.

Approval Request Comment
[Feature/regressing bug #]:unknown
[User impact if declined]:some audio/video won't play in e10s mode.
[Describe test coverage new/current, TreeHerder]:TreeHerder
[Risks and why]: low, the change is simple and just about disabling a feature that a user will never use.
[String/UUID change made/needed]:none
Attachment #8727287 - Flags: approval-mozilla-beta?
Attachment #8727287 - Flags: approval-mozilla-aurora?
Marking affected for 46 since we're testing e10s in 46 beta. 
Can we verify this fix on 48?
Flags: qe-verify+
Is this something that affects users playing audio/video, or just tests? Do we have a way to test/verify?
Flags: needinfo?(cpearce)
Reporter

Comment 21

3 years ago
This is debug-only code. It doesn't affect release users at all. Given that we aren't running WinXP e10s tests on 46, I think we should just uplift this a=NPOTB to Aurora and call it good.
Flags: needinfo?(cpearce)
Comment on attachment 8727287 [details]
MozReview Request: Bug 1253959 - per comment 5, remove code that causes ReadMetadata() to fail on Windows XP debug. r=cpearce.

Debug-only, OK to uplift to aurora
Attachment #8727287 - Flags: approval-mozilla-beta?
Attachment #8727287 - Flags: approval-mozilla-beta-
Attachment #8727287 - Flags: approval-mozilla-aurora?
Attachment #8727287 - Flags: approval-mozilla-aurora+
Thanks Ryan, I missed that it was xp only, and you're right, NPOTB should be ok to land anyway without approval.
[qe-] based on comment 21
Flags: qe-verify+ → qe-verify-
You need to log in before you can comment on or make changes to this bug.