User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0 Build ID: 20170520072017 Steps to reproduce: Tried to share a window from my computer using WebRTC but the list is incomplete, and only shows Mozilla software. Tested in a Jitsi instance and with the gUM Test Page Actual results: Windows not listed. Look at the attached screeshot. A lot of windows are open in the task bar but do not show in Firefox's list. Expected results: Full list of available windows.

There is no filter to show only Mozilla software. Can you do us a favor and type up a list of the windows shown and a list of windows that are not shown? It's hard to tell from the screenshot. Thanks.

Hello Jan, Sorry for not providing enough details. In the example from my screenshot I have following windows/programs opened: -Dolphin -Firefox -Kate -KCalc -Konsole -Thunderbird The only shown windows are Firefox and Thunderbird.

Thanks! Is there anyone with linux who can reproduce this? It would also help to know if this is a regression. Do we know if this worked in earlier versions of Firefox on linux? Window sharing has been implemented for a long time, going back to version 33, though it required a whitelist before 52. If this used to work, the next step is to establish a regression-range with https://mozilla.github.io/mozregression/

Michael, I see you verified window sharing on linux in bug 1336723, could you see if you can reproduce this?

I just tested on TrueOS and I am not seeing all the windows either.

(In reply to Jan-Ivar Bruaroey [:jib] from comment #4) > Michael, I see you verified window sharing on linux in bug 1336723, could > you see if you can reproduce this? My linux box is temporarily offline, but I'll bring it up as soon as possible and check. Leaving needinfo on for me as a reminder. Should be later today or tomorrow.

I'm on Ubuntu 16.04, Firefox 53.0.3. Using the gUM Test Page I see non-Mozilla windows in the list. However, in order to get the list to update, I had to restart Firefox. Simply opening a new tab, or a new window did not cause the list of windows to update. I'll attach a screen shot to show my window list.

Here is the screen shot showing non-Mozilla windows in the list

Johan, does comment 7 explain your symptom? Did you open the other windows after Firefox, and do you see them in the list if you open them before opening Firefox? Note I'm not saying this is ok, it just helps triage this.

I attempted to repro this in on Ubuntu 16.04.2 LTS with Firefox 53.03, and found the same behavior with the added caveat that closed windows still appeared in the list. Selecting a closed window failed to capture.

The same behavior exists in the webrtc.org 57 merge Firefox branch.

Hello Jan, We are on a way to a solution here. While restarting Firefox helps a lot - and shows non Mozilla windows - I'm still missing a few. I am approximately missing a third of the windows.

Are the missing windows minimized by any chance?

Minimized windows don't seem to have an impact. I have been making some tests yesterday with very weird results. If I close and open Firefox without even touching my windows, some are not shown anymore, and some others appear.

I checked on my laptop and this behavior also occurs on OS X. I suspect it is platform independent.

Could it be X11 related? Last time I used OS X it was still the default server.

Comment on attachment 8877362 [details] Bug 1371000 - add expiration to noncamera device info; https://reviewboard.mozilla.org/r/148748/#review153188 Not the best design perhaps to have 184 IPC calls on a single gUM call, but until we change it, the design very much relies on this being cached. ::: dom/media/systemservices/VideoEngine.cpp:75 (Diff revision 1) > std::shared_ptr<webrtc::VideoCaptureModule::DeviceInfo> > VideoEngine::GetOrCreateVideoCaptureDeviceInfo() { > + switch (mCaptureDevInfo.type) { > + case webrtc::CaptureDeviceType::Camera: { > - if (mDeviceInfo) { > + if (mDeviceInfo) { > - return mDeviceInfo; > + return mDeviceInfo; > - } > + } GetOrCreateVideoCaptureDeviceInfo() is called from RecvGetCaptureCapability() which is called 184 times on a single gUM call.

Oh wait, I tested camera, not screenshare, so it may not be a problem after all, but I'd still like to have a better understanding on how often this is called for screenshare.

It's called a lot less for screensharing, ~10 times, but that's still too high. Basically it's called for every IPC method to CamerasParent. Someday we should rethink that design.

I tested on Fedora 25 with nightly from ~ 5 days ago. All windows, mozilla and non-mozilla, are shown. Opening a new non-mozilla window and trying Window on the gum_test page again shows the new window (and sharing it works). I rebuilt inbound on 6/13, and I see the problem - new windows don't show up.

Downloading a m-c build from 6-5 shows the problem too. Regression-range says between 2016-12-17 and 2017-1-5,which points strongly to the v49 landing. Note sure why it worked for me earlier

Randell, are you able to get a narrower range with mozregression?

Comment on attachment 8877362 [details] Bug 1371000 - add expiration to noncamera device info; https://reviewboard.mozilla.org/r/148748/#review154002 This could work, though I have a question about the cache window logic, as it seems more elaborate than necessary to me, though I may be missing something. ::: dom/media/systemservices/VideoEngine.h:30 (Diff revision 2) > + // Note because cameras use HW plug event detection, this > + // only applies to screen based modes. nit: odd indent ::: dom/media/systemservices/VideoEngine.h:34 (Diff revision 2) > + // Base cache validity period > + // Note because cameras use HW plug event detection, this > + // only applies to screen based modes. > + static const int64_t kBaseExpiryPeriodMs = 500; > + // The number of times the expiry window can slide before forced expiration > + static const int kMaxEpiryExtensions = 10; typo ::: dom/media/systemservices/VideoEngine.h:53 (Diff revision 2) > - /** Returns or creates a new new DeviceInfo. > - * It is cached to prevent repeated lengthy polling for "realness" > - * of the hardware devices. This could be handled in a more elegant > - * way in the future. > + /** Returns an existing or creates a new new DeviceInfo. > + * Camera info is cached to prevent repeated lengthy polling for "realness" > + * of the hardware devices. Other types of capture, e.g. screen share info, > + * is not cached. This could be handled in a more elegant way in the future. update "not cached" language ::: dom/media/systemservices/VideoEngine.cpp:89 (Diff revision 2) > + // Screen sharing cache is invalidated by expiry over a sliding window > + // with an enforced maximum size. > + currentTime = webrtc::Clock::GetRealTimeClock()->TimeInMilliseconds(); > + if (currentTime <= mExpiryTimeInMs) { > + if (++mExpiryExtensions <= kMaxEpiryExtensions) { > + mExpiryTimeInMs += kExpiryExtentionMs; > + } From what I can tell, this extends the "caching" window ("expiry window" seems like a misnomer) from 500 ms to 1000 ms if I call the method 10 or more times within the first 500 ms, which we know is typical. What problem does this logic solve over a simpler 1 second max?

(In reply to Jan-Ivar Bruaroey [:jib] from comment #24) > Randell, are you able to get a narrower range with mozregression? 12/27 works v49 landed 12/28 12/29 fails

Comment on attachment 8877362 [details] Bug 1371000 - add expiration to noncamera device info; https://reviewboard.mozilla.org/r/148748/#review154640 lgtm. ::: dom/media/systemservices/VideoEngine.cpp:89 (Diff revision 3) > + // Screen sharing cache is invalidated by expiry over a sliding window > + // with an enforced maximum size. window is no longer sliding.

Comment on attachment 8877362 [details] Bug 1371000 - add expiration to noncamera device info; https://reviewboard.mozilla.org/r/148748/#review155306

I'm seeing similar problem with Windows 7 and Windows 10. Window sharing list is much shorter than application sharing.

Nico/jib: can we land this? We've missed possible uplift to 55.

Yes, I have it running on try now.

Pushed by na-g@nostrum.com: https://hg.mozilla.org/integration/autoland/rev/3b3d54ec727b add expiration to noncamera device info;r=jib

(In reply to Randell Jesup [:jesup] from comment #32) > Nico/jib: can we land this? We've missed possible uplift to 55. There's still two more betas in this cycle (one more week). Is this too risky for uplift or?

question for jib as well. Not a couple-liner, though pretty straightforward if it's careful about holding refs needed by the timer and there aren't any thread races, and I think there aren't. I'd be ok with this perhaps with some manual checks.

Looks ok to me. No threads or timer references here since it just checks against previous GetRealTimeClock() values. Provided those counters are never reset for any reason, we should be safe. The change seems well-contained to screensharing, as the more common camera case bypasses the new logic, and works as before. Definitely needs manual testing, since there's no automated coverage. TESTING TIPS: Set MOZ_LOG="timestamp:1,VideoEngine:4" and verify log messages from the patch in the log. We should test both camera and screensharing (multiple types) with e.g. http://mozilla.github.io/webrtc-landing/ In short, since this low-level code is called several times during a single request, we pretty much want the cache to work all the time for cameras, and within the same request for screensharing (but not across multiple screensharing requests). Ensure: 1. the cache is never reset for cameras, 2. the cache is never reset within a single screensharing request (<1s) 3. the cache is always reset between individual screensharing requests (>1s) 4. User sees new windows in drop down 5. camera capture is not sluggish

I am running through the "Ensure" list above using https://mozilla.github.io/webrtc-landing/gum_test.html 1. Tested by selecting the "video" option on the "getUserMedia Test Page", allowing access to the camera, then refreshing and repeating. In 14 consecutive tries only the first resulted in a new device info being created. 2. & 3. Tested by running the "window" test on the "getUserMedia Test Page", then hitting refresh and running the "window" test again. Afterwards I inspected the logs. This appears to be working as intended.* 4. Tested by closing most windows on the system, triggering the window sharing gUM dialog by selecting "window", and checking the drop down list. Then I opened a number of new windows and refreshed the "getUserMedia Test Page", selected "window" again, and checked the drop down list. The VideoEngine logs show a device info creation event followed by a number of cache hits. Then some seconds later another device info creation event occurs followed a larger number of cache hits than the first. 5. I noticed no unexpected sluggishness when testing #1 * Something I did not expect is that a device info is being created right before a VideoEngine is deleted. I don't have any evidence that this is related to the patch, I will investigate further tomorrow.

The device info creation before deletion of the VideoEngine is done so that any event handlers can be removed. It is not related to this patch.

Was someone going to request Beta uplift on this patch?

Comment on attachment 8877362 [details] Bug 1371000 - add expiration to noncamera device info; Approval Request Comment [Feature/Bug causing the regression]: bug 1371000 [User impact if declined]: Users will have to restart the browser to refresh the windows available in gUM (getUserMedia) dialogs [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: I conducted manual testing as outlined in detail in comment 40 [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: medium risk [Why is the change risky/not risky?]: it is not a trival patch, but it is well contained. It could cause regressions in gUM behavior. [String changes made/needed]: none

Comment on attachment 8877362 [details] Bug 1371000 - add expiration to noncamera device info; Seems too last minute for 55, we're building release candidates today

I reproduced this issue on Ubuntu 16.04 32bit using old Nightly from 2017-07-20, verified that the newly opened windows (or recently closed don't) appear in latest Firefox 56 beta 11 after reloading the website or if clicking for the first time on window sharing after first opening Fx and then opening other apps.