1477881 - Ensure AutoplayPermissionRequest can't double report a response

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P2)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

As pointed out in bug 1477273 comment 1:

1. (Gecko/C++) We cancel the permission request in the AutoplayPermissionRequest destructor [1], and if we get a genuine cancel from the doorhanger. The Request reports the cancel to the AutoplayPermissionManager, but we reuse the same manager across different requests. So if a second request for permission comes in, we create a new AutoplayPermissionRequest and fire that off to content, but the first could be destroyed after the second is dispatched, and the cancel in the first's destructor could be reported to the manager as the second's result. We should clear the AutoplayPermissionRequest's reference to the Manager in Approve() and Cancel() so that we can't mixup the responses like this.


[1] https://searchfox.org/mozilla-central/rev/ad36eff63e208b37bc9441b91b7cea7291d82890/dom/html/AutoplayPermissionRequest.cpp#37

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1477881 - Ensure AutoplayPermissionRequest can't double report a response.

We cancel the permission request in the AutoplayPermissionRequest destructor,
and if we get a genuine cancel from the doorhanger. The Request reports the
cancel to the AutoplayPermissionManager, but we reuse the same manager across
different requests. So if a second request for permission comes in, we create a
new AutoplayPermissionRequest and fire that off to the front end code, but the
first request could be destroyed after the second request is dispatched but
before the response for the second request has retuned. Thus and the cancel in
the first's destructor could be reported to the manager as the second's result.

We should clear the AutoplayPermissionRequest's reference to the Manager in
Approve() and Cancel() so that we can't mixup the responses like this.

Review commit: https://reviewboard.mozilla.org/r/258946/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/258946/

Comment 2

[Alastor Wu [:alwu]]

Comment on attachment 8994376 [details]
Bug 1477881 - Ensure AutoplayPermissionRequest can't double report a response.

https://reviewboard.mozilla.org/r/258946/#review266176

LGTM, thank you!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4e4541f3c1c
Ensure AutoplayPermissionRequest can't double report a response. r=alwu

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/f4e4541f3c1c