Closed Bug 1480281 Opened 2 years ago Closed 2 years ago

Add debug log for blocking autoplay

Categories

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

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Add MOZ_LOG=Autoplay:5 for debugging.
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."
Attachment #8996898 - Flags: review?(cpearce) → review+
Comment on attachment 8996899 [details]
Bug 1480281 - part2 : move exist log to autoplay log module.

https://reviewboard.mozilla.org/r/260890/#review267930
Attachment #8996899 - Flags: review?(cpearce) → review+
Pushed by alwu@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/4c0d86611660
https://hg.mozilla.org/mozilla-central/rev/4c8a057c6d54
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.