Closed Bug 1365852 Opened 3 years ago Closed 3 years ago

Intermittent browser/base/content/test/webrtc/browser_devices_get_user_media_unprompted_access_queue_request.js | leaked 1 window(s) until shutdown [url = chrome://browser/content/webrtcIndicator.xul]

Categories

(Core :: WebRTC: Audio/Video, defect, P3)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed
Blocking Flags:
backlog tech-debt

People

(Reporter: mchiang, Assigned: mchiang)

References

Details

Attachments

(2 files)

No description provided.
Summary: Intermittent browser/base/content/test/webrtc/browser_devices_get_user_media_queue_request.js | leaked 1 window(s) until shutdown [url = chrome://browser/content/webrtcIndicator.xul] → Intermittent browser/base/content/test/webrtc/browser_devices_get_user_media_unprompted_access_queue_request.js | leaked 1 window(s) until shutdown [url = chrome://browser/content/webrtcIndicator.xul]
It is unclear how common this intermittent failure is...
backlog: --- → tech-debt
Rank: 35
Priority: -- → P3
(In reply to Munro Mengjue Chiang [:mchiang] from comment #2)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=4bbe25c2e4aef2b8f97d2748b592502c4781fe12&selectedJob=1
> 03304939

Looks like it's failing all the time. This leak can't exist on Mac because the webrtcIndicator.xul window doesn't exist there, the global sharing indicators are icons in the menubar instead.
Still have no clue. This bug only occurs on try machine.
Please refer to this log
https://treeherder.mozilla.org/logviewer.html#?job_id=104826384&repo=try&lineNumber=2933

When this leakage issue occurs, one thread is blocking on this function gIndicatorWindow.updateIndicatorState()
https://hg.mozilla.org/try/file/b29733b2def9/browser/modules/webrtcUI.jsm#l1090

Florian, do you have idea why the function gIndicatorWindow.updateIndicatorState() never returns?
Flags: needinfo?(florian)
(In reply to Munro Mengjue Chiang [:mchiang] from comment #5)
> Please refer to this log
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=104826384&repo=try&lineNumber=2933
> 
> When this leakage issue occurs, one thread is blocking on this function
> gIndicatorWindow.updateIndicatorState()
> https://hg.mozilla.org/try/file/b29733b2def9/browser/modules/webrtcUI.
> jsm#l1090
> 
> Florian, do you have idea why the function
> gIndicatorWindow.updateIndicatorState() never returns?

It likely throws an exception. You can put a try {...} catch(e) {ok(false, e);} around it to see what the exception is.
Flags: needinfo?(florian)
After adding try catch, this phenomenon suddenly disappears.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd5f0740aea50c582158dfe27ccfa9dee312997c&selectedJob=104956333
I check each job in the request and none of them throws an exception.
This might be a timing issue?
As mentioned in bug 861716 comment 101, there is a bug that we may call updateIndicatorState before webrtcIndicator is loaded & initialized and cause an exception.

Queuing gUM request in MediaManager (introduced in bug 861716) and unprompted access (introduced in bug 1270572) raise the reproduce rate of this bug.
Assignee: nobody → mchiang
Comment on attachment 8876971 [details] [diff] [review]
Bug1365852-ensure-webrtcIndicator-init-done-before-calling-updateIndicatorState.patch

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

Please provide patches with 8 lines of context.

I think the easiest fix here would be to just add:

// If stringBundle isn't set, the window hasn't finished loading.
if (!stringBundle)
  return;

at http://searchfox.org/mozilla-central/rev/61054508641ee76f9c49bcf7303ef3cfb6b410d2/browser/base/content/webrtcIndicator.js#42

::: browser/modules/webrtcUI.jsm
@@ +1085,5 @@
> +      if (AppConstants.platform == "macosx" || gIndicatorWindowInited) {
> +        updateIndicatorState();
> +      } else {
> +        gIndicatorWindow.addEventListener("load", () => {
> +          updateIndicatorState();

You don't need this, see
http://searchfox.org/mozilla-central/rev/61054508641ee76f9c49bcf7303ef3cfb6b410d2/browser/base/content/webrtcIndicator.xul#17
and
http://searchfox.org/mozilla-central/rev/61054508641ee76f9c49bcf7303ef3cfb6b410d2/browser/base/content/webrtcIndicator.js#32
Attachment #8876971 - Flags: review?(florian) → review-
Comment on attachment 8877384 [details]
Bug 1365852 - immediately return in updateIndicatorState() if webrtcIndicator hasn't finished loading.

https://reviewboard.mozilla.org/r/148764/#review153378
Attachment #8877384 - Flags: review?(florian) → review+
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev 85bf4deb567e needs "Bug N" or "No bug" in the commit message.
remote: Munro Mengjue Chiang <mchiang@mozilla.com>
remote: Bug1365852 - immediately return in updateIndicatorState() if webrtcIndicator hasn't finished loading. r=florian
remote: 
remote: MozReview-Commit-ID: D6MZn6khaJO
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/91908f42965c
immediately return in updateIndicatorState() if webrtcIndicator hasn't finished loading. r=florian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/91908f42965c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.