1691625 - Improve MediaManager::mPendingGUMRequest logic

Status: ASSIGNED

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

Description

[Karl Tomlinson (:karlt)]

mPendingGUMRequest was implemented as an incomplete band-aid, with multiple flaws including the following.

The check on whether to send a new request is based on whether there is an existing request for the relevant (inner) window, but pending requests are dispatched when a request in the same process (for a potentially different window) is completed.

Some error handling paths trigger SendPendingGUMRequest():

• getUserMedia:response:deny
• AllocateDevices()

but others do not:

• getUserMedia:response:allow
• PrepareDOMStream()
• ~GetUserMediaTask()

The queue blocks even requests that do not need a prompt.

Comment 1

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Rename various things around "active windows" in MediaManager. r?karlt!

This is an attempt to make it clearer what an active window is. The term still
lives on through IsWindowStillActive and IsWindowListenerStillActive but it is
hopefully clearer that they only check whether a window listener is registered
to a window (id).

This hopefully reduces confusion with other terms, such as "active callbacks".

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Teach test_gUM_scarySources.html about the request queue. r?karlt!

Comment 3

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Notify of gesture activation before requesting display media in all webrtc browser-chrome tests. r?karlt!

At least a local failure has been observed in
browser_devices_get_user_media_in_xorigin_frame.js.

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Re-enable webrtc browser-tests. r?karlt!

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Don't look for "sharingbrowserwindow" window attribute in the legacy indicator. r?karlt!

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - In MediaManager refactor window and call related data into MediaManagerData. r?karlt!

This patch mainly moves window and call/request related data structures from
MediaManager into a new helper class called MediaManagerData. A lot of the
logic around these data structures moves along into MediaManagerData.

MediaManagerData stores the data in a natural way that better matches the
hierarchical relationships between calls, windows and MediaManager. This
improves readability of the affected code significantly, and makes it easier
to improve this code in the future.

It also takes over the responsibility to send some notifications to the
frontend, to declutter the code in MediaManager.

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Don't check pref "media.navigator.permission.force" twice before prompting. r?karlt!

It is already baked into askPermission and therefore has no effect.

Comment 8

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Route all GetUserMediaRequests through DataManager. r?karlt!

This patch removes the race by which MediaManager would check the registered
GetUserMediaTasks to determine whether to show a prompt, but pop the request
queue (thereby showing another prompt) at a later time than when unregistering
a GetUserMediaTask. This allowed another request to come in and show a prompt
in between these events, so that when popping the request queue there'd be two
simultaneous requests sent to the parent for prompting.

With this patch MediaManagerData is in charge of sending requests, and it will
only check the request queue when determining whether a request should be sent.

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Don't send a pending request if a call that never prompted told us to. r?karlt!

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - In MediaManagerData make the request queue per-window. r?karlt!

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1691625 - Send a pending GetUserMediaRequest (prompt) immediately after receiving a reply. r?karlt!, r?jib!