Closed Bug 1154019 Opened 9 years ago Closed 9 years ago

Intermittent browser_devices_get_user_media.js | Test timed out | Found a Browser:WebRTCGlobalIndicator

Categories

(Firefox :: Site Permissions, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- fixed

People

(Reporter: RyanVM, Assigned: florian)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 3 obsolete files)

Attached image test screenshot
Just a one-off so far, so I'm inclined to keep the other patches in and deal with this one separately.

08:33:25 INFO - 527 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | webRTC-shareDevices notification shown
08:33:25 INFO - 528 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | notification panel open
08:33:25 INFO - 529 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | notification panel populated
08:33:25 INFO - 530 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | expected notification getUserMedia:request
08:33:25 INFO - 531 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | microphone selector visible
08:33:25 INFO - 532 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | camera selector visible
08:33:25 INFO - 533 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | received ok
08:33:25 INFO - 534 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | expected notification getUserMedia:response:allow
08:33:25 INFO - 535 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | expected notification recording-device-events
08:33:25 INFO - 536 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | expected microphone to be shared
08:33:25 INFO - 537 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | webRTC-sharingDevices notification appeared
08:33:25 INFO - 538 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | WebRTC indicator visible
08:33:25 INFO - 539 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | camera global indicator as expected
08:33:25 INFO - 540 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | microphone global indicator as expected
08:33:25 INFO - 541 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | screen global indicator as expected
08:33:25 INFO - 542 INFO TEST-PASS | browser/base/content/test/general/browser_devices_get_user_media.js | WebRTC menu should be visible
08:33:25 INFO - 543 INFO waiting for a chrome://browser/content/webrtcIndicator.xul window
08:33:25 INFO - 544 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_devices_get_user_media.js | Test timed out - expected PASS
08:33:25 INFO - [1116] WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\netwerk\base\nsSimpleURI.cpp, line 264
08:33:25 INFO - [1116] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) failed with result 0x80040111: file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\docshell\base\nsDocShell.cpp, line 4580
08:33:25 INFO - MEMORY STAT vsize after test: 988454912
08:33:25 INFO - MEMORY STAT vsizeMaxContiguous after test: 572260352
08:33:25 INFO - MEMORY STAT residentFast after test: 343478272
08:33:25 INFO - MEMORY STAT heapAllocated after test: 73503366
08:33:25 INFO - 545 INFO TEST-OK | browser/base/content/test/general/browser_devices_get_user_media.js | took 45289ms
08:33:25 INFO - ++DOCSHELL 0D3E0400 == 15 [pid = 1116] [id = 637]
08:33:25 INFO - ++DOMWINDOW == 29 (0E039800) [pid = 1116] [serial = 1661] [outer = 00000000]
08:33:25 INFO - ++DOMWINDOW == 30 (0E3C4000) [pid = 1116] [serial = 1662] [outer = 0E039800]
08:33:25 INFO - [1116] WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\netwerk\base\nsSimpleURI.cpp, line 264
08:33:25 INFO - [1116] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) failed with result 0x80040111: file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\docshell\base\nsDocShell.cpp, line 4580
08:33:25 INFO - 546 INFO checking window state
08:33:25 INFO - 547 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_devices_get_user_media.js | Found a Browser:WebRTCGlobalIndicator after previous test timed out - expected PASS
08:33:25 INFO - [1116] WARNING: Should've notified blocked=true for a fully finished stream: 'stream->mNotifiedBlocked', file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\dom\media\MediaStreamGraph.cpp, line 447
08:33:25 INFO - [1116] WARNING: Should've notified blocked=true for a fully finished stream: 'stream->mNotifiedBlocked', file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\dom\media\MediaStreamGraph.cpp, line 447
08:33:25 INFO - [1116] WARNING: Shouldn't have already notified of finish *and* have output!: '!streamHasOutput[i] || !stream->mNotifiedFinished', file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\dom\media\MediaStreamGraph.cpp, line 418
08:33:25 INFO - [1116] WARNING: Shouldn't have already notified of finish *and* have output!: '!streamHasOutput[i] || !stream->mNotifiedFinished', file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\dom\media\MediaStreamGraph.cpp, line 418
08:33:25 INFO - [1116] WARNING: Shouldn't have already notified of finish *and* have output!: '!streamHasOutput[i] || !stream->mNotifiedFinished', file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\dom\media\MediaStreamGraph.cpp, line 418
08:33:25 INFO - [1116] WARNING: Shouldn't have already notified of finish *and* have output!: '!streamHasOutput[i] || !stream->mNotifiedFinished', file c:\builds\moz2_slave\fx-team-w32-d-0000000000000000\build\src\dom\media\MediaStreamGraph.cpp, line 418
The new error is coming from the patch from bug 1109728.
Blocks: 1109728
No longer blocks: 1099095
In order to debug this, I made the global indicator appear after a delay of one second... and that made me discover that my code using the domwindowopened notification didn't work at all: it just prints a message saying it's ignoring an about:blank window. After looking around in the tree, it seems other consummers of this notification wait for a load event before doing anything with the window.

The attached patch does just this, and that's enough for the test to pass if the global indicator window is opened from a 1s setTimeout.

Unfortunately, the 2 test failures that have been stared here don't have an "ignoring about:blank" line, so I don't think this patch will fix the failure.
Any ideas on how to solve this one?  It continues to show up... any chance the patch here would actually fix it?  (Or help in general?
Flags: needinfo?(florian)
(In reply to Randell Jesup [:jesup] from comment #51)
> Any ideas on how to solve this one?

Unfortunately, I don't have any idea for a _real_ solution here. The patch I'm attaching now will hide the problem by waiting for the indicator window to exist with a timer. I'm relatively confident this will make the intermittent failures go away, as the failures here do have an indicator window when the test eventually timeouts.

> It continues to show up... any chance
> the patch here would actually fix it?  (Or help in general?

The patch that I attached here could possibly have marginally helped, but like I said in comment 5, it wouldn't help with failures that don't have an "ignoring about:blank" line in the log... and none of the failures I looked at here had that line.
Assignee: nobody → florian
Attachment #8592279 - Attachment is obsolete: true
Flags: needinfo?(florian)
Attachment #8616624 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8616624 [details] [diff] [review]
Wait for the window using promiseWaitForCondition

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

waitForCondition is evil and needs to die in a fire (because it relies on setTimeout). I am worried that wouldn't actually fix the intermittents.

It seems to me that what should happen is that the code should be listening for the window to open earlier. checkSharingUI is the only caller that passes non-null aExpected, so could you just make that pass a promise for the window opening as well, and create that promise in the checkSharingUI callsites *before* doing whatever triggers this (usually, clicking "Ok" in the permissions prompt) ? In fact, that code (ie, the callers) could probably be made to yield for that as well, which would solve the problem without having to pass the promise around.
Attachment #8616624 - Flags: review?(gijskruitbosch+bugs)
Attached patch Patch v3 (obsolete) — Splinter Review
I couldn't push to try because of the tree closure, but I've tested it locally on Mac opt and Linux debug.
Attachment #8616624 - Attachment is obsolete: true
Attachment #8617224 - Flags: review?(gijskruitbosch+bugs)
Attached patch Patch v3.1Splinter Review
Actually, we don't need the promiseWindow call in assertWebRTCIndicatorStatus anymore.
Attachment #8617224 - Attachment is obsolete: true
Attachment #8617224 - Flags: review?(gijskruitbosch+bugs)
Attachment #8617230 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8617230 [details] [diff] [review]
Patch v3.1

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

Nice! Let's hope this finally gets it... :-)
Attachment #8617230 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/d378f3087fca
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.