1480281 - Add debug log for blocking autoplay

Status: RESOLVED FIXED

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

Description

[Alastor Wu [:alwu]]

Add MOZ_LOG=Autoplay:5 for debugging.

Comment 1

[Alastor Wu [:alwu]]

Attachment: Bug 1480281 - part1 : add autoplay debug log module.

Add new log module which allow us to debug by using "MOZ_LOG=Autoplay:5".

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

Comment 2

[Alastor Wu [:alwu]]

Attachment: Bug 1480281 - part2 : move exist log to autoplay log module.

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

Comment 3

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

Comment on attachment 8996898 [details]
Bug 1480281 - part1 : add autoplay debug log module.

https://reviewboard.mozilla.org/r/260888/#review267928

::: dom/html/HTMLMediaElement.cpp:3069
(Diff revision 1)
>  {
>    if (GetPaused()) {
>      return;
>    }
>    if (AutoplayPolicy::IsAllowedToPlay(*this) != nsIAutoplay::ALLOWED) {
> +    AUTOPLAY_LOG("pause because if should not be playing, element=%p\n", this);

"pause because not allowed to play, element=%p".

You don't need the \n there?

::: dom/media/AutoplayPolicy.cpp:29
(Diff revision 1)
> +mozilla::LazyLogModule gAutoplayPermissionLog("Autoplay");
> +
> +#define AUTOPLAY_LOG(msg, ...)                                             \
> +  MOZ_LOG(gAutoplayPermissionLog, LogLevel::Debug, (msg, ##__VA_ARGS__))
> +
> +const char*

AllowAutoplayToStr can be static, as it's not used outside of this translation unit.

::: dom/media/AutoplayPolicy.cpp:87
(Diff revision 1)
>  
>    nsIDocument* approver = ApproverDocOf(*aWindow->GetExtantDoc());
>    if (nsContentUtils::IsExactSitePermAllow(approver->NodePrincipal(),
>                                             "autoplay-media")) {
>      // Autoplay permission has been granted already.
> +    AUTOPLAY_LOG("Allow autoplay for document owns autoplay permission.");

"Allow autoplay as document has autoplay permission."

::: dom/media/AutoplayPolicy.cpp:92
(Diff revision 1)
> +    AUTOPLAY_LOG("Allow autoplay for document owns autoplay permission.");
>      return true;
>    }
>  
>    if (approver->HasBeenUserGestureActivated()) {
> -    // Document has been activated by user gesture.
> +    AUTOPLAY_LOG("Allow autoplay for document activated by user gesture.");

s/for/as/

::: dom/media/AutoplayPolicy.cpp:97
(Diff revision 1)
> -    // Document has been activated by user gesture.
> +    AUTOPLAY_LOG("Allow autoplay for document activated by user gesture.");
>      return true;
>    }
>  
>    if (approver->IsExtensionPage()) {
> -    // Always allow extension page to autoplay.
> +    AUTOPLAY_LOG("Allow autoplay for extension document.");

s/for/as in/

::: dom/media/AutoplayPolicy.cpp:140
(Diff revision 1)
> -          IsWindowAllowedToPlay(aElement.OwnerDoc()->GetInnerWindow()) ||
> -          (aElement.OwnerDoc()->MediaDocumentKind() == nsIDocument::MediaDocumentKind::Video);
> +    AUTOPLAY_LOG("Allow muted media %p to autoplay.", &aElement);
> +    return true;
> +  }
> +
> +  if (IsWindowAllowedToPlay(aElement.OwnerDoc()->GetInnerWindow())) {
> +    AUTOPLAY_LOG("Allow activated window for media %p to autoplay.", &aElement);

IsWindowAllowedToPlay() also returns true for windows that have an origin with autoplay permission. So:

"Autoplay allowed as activated/whitelisted window, media %p."

Comment 4

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

Comment on attachment 8996899 [details]
Bug 1480281 - part2 : move exist log to autoplay log module.

https://reviewboard.mozilla.org/r/260890/#review267930

Comment 5

[Alastor Wu [:alwu]]

Comment on attachment 8996898 [details]
Bug 1480281 - part1 : add autoplay debug log module.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/260888/diff/1-2/

Comment 6

[Alastor Wu [:alwu]]

Comment on attachment 8996899 [details]
Bug 1480281 - part2 : move exist log to autoplay log module.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/260890/diff/1-2/

Comment 7

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c47380dbb98822876d2b4d3dba757a5f1efb761e

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c0d86611660
part1 : add autoplay debug log module. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/4c8a057c6d54
part2 : move exist log to autoplay log module. r=cpearce

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/4c0d86611660
https://hg.mozilla.org/mozilla-central/rev/4c8a057c6d54