Closed Bug 1472580 Opened 6 years ago Closed 6 years ago

Clicking the audio tab indicator should unnblock autoplay

Categories

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

enhancement

Tracking

()

VERIFIED FIXED
mozilla63
Tracking Status
firefox63 --- verified

People

(Reporter: cpearce, Assigned: cpearce)

References

Details

Attachments

(3 files)

If the user opens a tab in the background, and that tab tries to play audible media, we'll show the audio tab indicator in the tab bar. If the user clicks on it to start playback, we'll then hit the block autoplay logic and maybe show a doorhanger in that tab asking for permission. The doorhanger won't show until that tab is brought to the foreground, so in other words, when the user clicks the "play" button, we'll actually not play. So we need the tab audio indicator to override the autoplay blocking logic. This is spun off from bug 1463919.
Comment on attachment 8989096 [details] Bug 1472580 - Gesture activate documents which are played via the tab audio indicator. https://reviewboard.mozilla.org/r/254160/#review261364 ::: toolkit/content/browser-content.js:533 (Diff revision 1) > case "resumeMedia": > + // User has clicked the tab audio indicator to play a delayed > + // media. That's clear user intent to play, so gesture activate > + // the content document tree so that the block-autoplay logic > + // allows the media to autoplay. > + content.window.document.notifyUserGestureActivation(); I _think_ `content.window` is redundant, and you can just do `content.document` here.
Attachment #8989096 - Flags: review?(mconley) → review+
Comment on attachment 8989097 [details] Bug 1472580 - Test that starting play from tab audio indicator overrides block autoplay. https://reviewboard.mozilla.org/r/254162/#review261366 Thanks for the test! ::: toolkit/content/tests/browser/browser_block_autoplay_media.js:11 (Diff revision 1) > SUSPENDED_BLOCK: 2, > SUSPENDED_PAUSE_DISPOSABLE: 3 > }; > > -function check_audio_suspended(suspendedType) { > +function check_audio_suspended(browser, suspendedType) { > + return ContentTask.spawn(browser, suspendedType, (suspendedType) => { For single arguments in arrow-functions, you can forgo the parantheses. Same below.
Attachment #8989097 - Flags: review?(mconley) → review+
Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4913ce5ab6cb Gesture activate documents which are played via the tab audio indicator. r=mconley https://hg.mozilla.org/integration/autoland/rev/d3d4619e8133 Test that starting play from tab audio indicator overrides block autoplay. r=mconley
Backed out 2 changesets (bug 1472580) for browser chrome LeakSanitizer failures Backout: https://hg.mozilla.org/integration/autoland/rev/cdfb7cd94980a0e50b275d076b1ed8f5edc1c797 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d3d4619e8133d286727a1062e27ebeb962450647&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&selectedJob=186295717 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=186295717&repo=autoland&lineNumber=80894 [task 2018-07-03T22:20:19.050Z] INFO - GECKO(4837) | Suppressions used: [task 2018-07-03T22:20:19.051Z] INFO - GECKO(4837) | count bytes template [task 2018-07-03T22:20:19.052Z] INFO - GECKO(4837) | 2028 182543 libc.so [task 2018-07-03T22:20:19.053Z] INFO - GECKO(4837) | 2066 120234 nsComponentManagerImpl [task 2018-07-03T22:20:19.055Z] INFO - GECKO(4837) | 201 15675 mozJSComponentLoader::LoadModule [task 2018-07-03T22:20:19.056Z] INFO - GECKO(4837) | 611 17713 libfontconfig.so [task 2018-07-03T22:20:19.057Z] INFO - GECKO(4837) | 1 29 libglib-2.0.so [task 2018-07-03T22:20:19.058Z] INFO - GECKO(4837) | ----------------------------------------------------- [task 2018-07-03T22:20:19.059Z] INFO - GECKO(4837) | SUMMARY: AddressSanitizer: 277997 byte(s) leaked in 4134 allocation(s). [task 2018-07-03T22:20:19.061Z] INFO - GECKO(4837) | ----------------------------------------------------- [task 2018-07-03T22:20:19.062Z] INFO - GECKO(4837) | Suppressions used: [task 2018-07-03T22:20:19.063Z] INFO - GECKO(4837) | count bytes template [task 2018-07-03T22:20:19.064Z] INFO - GECKO(4837) | 670 21304 nsComponentManagerImpl [task 2018-07-03T22:20:19.066Z] INFO - GECKO(4837) | 43 8944 mozJSComponentLoader::LoadModule [task 2018-07-03T22:20:19.067Z] INFO - GECKO(4837) | 611 17509 libfontconfig.so [task 2018-07-03T22:20:19.069Z] INFO - GECKO(4837) | 12 528 _PR_Getfd [task 2018-07-03T22:20:19.070Z] INFO - GECKO(4837) | 1 29 libglib-2.0.so [task 2018-07-03T22:20:19.071Z] INFO - GECKO(4837) | ----------------------------------------------------- [task 2018-07-03T22:20:19.073Z] INFO - TEST-INFO | Main app process: exit 0 [task 2018-07-03T22:20:19.074Z] INFO - TEST-INFO | LeakSanitizer | To show the addresses of leaked objects add report_objects=1 to LSAN_OPTIONS [task 2018-07-03T22:20:19.075Z] INFO - TEST-INFO | LeakSanitizer | This can be done in testing/mozbase/mozrunner/mozrunner/utils.py [task 2018-07-03T22:20:19.076Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, js::Shape::hashify, maybeCreateTableForLookup [task 2018-07-03T22:20:19.078Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_calloc, maybe_pod_calloc, AllocScriptData, js::detail::CopyScript [task 2018-07-03T22:20:19.079Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::Exception_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.080Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, NewEmptyScopeData, js::GlobalScope::create [task 2018-07-03T22:20:19.081Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::Document_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.082Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, js::RegExpStatics::create [task 2018-07-03T22:20:19.083Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::ChromeUtils_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.084Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::DOMParser_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.085Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, CopyScopeData, js::LexicalScope::create [task 2018-07-03T22:20:19.086Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, createTable, js::detail::HashTable [task 2018-07-03T22:20:19.087Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, js::ForOfPIC::createForOfPICObject [task 2018-07-03T22:20:19.088Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, mozilla::UniquePtr, js::FunctionScope::create [task 2018-07-03T22:20:19.089Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_realloc, maybe_pod_realloc, js::Nursery::reallocateBuffer, ReallocateObjectBuffer [task 2018-07-03T22:20:19.090Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, js::LazyScript::CreateRaw [task 2018-07-03T22:20:19.091Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, NewEmptyScopeData, js::FunctionScope::create [task 2018-07-03T22:20:19.092Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, make_unique [task 2018-07-03T22:20:19.093Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at mozilla::dom::Performance::CreateForMainThread, nsPIDOMWindowInner::CreatePerformanceObjectIfNeeded, nsPIDOMWindowInner::GetPerformance, mozilla::dom::Window_Binding::get_performance [task 2018-07-03T22:20:19.094Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::MessageListenerManager_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.095Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, createTable, init [task 2018-07-03T22:20:19.096Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at AllocateProtoAndIfaceCache, mozilla::dom::CreateGlobal, mozilla::dom::Window_Binding::Wrap, nsGlobalWindowOuter::SetNewDocument [task 2018-07-03T22:20:19.097Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at Alloc, nsTSubstring, nsTSubstring, nsTSubstring [task 2018-07-03T22:20:19.103Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_new, MakeUnique, HashChildren, js::PropertyTree::insertChild [task 2018-07-03T22:20:19.104Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, CopyScopeData, js::GlobalScope::create [task 2018-07-03T22:20:19.105Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, tryNewTenuredObject, js::Allocate [task 2018-07-03T22:20:19.106Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, AllocChars [task 2018-07-03T22:20:19.107Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::InspectorUtils_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.108Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::URLSearchParams_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.109Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, CopyScopeData, js::Scope::clone [task 2018-07-03T22:20:19.110Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, mozilla::UniquePtr, js::FunctionScope::clone [task 2018-07-03T22:20:19.111Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::MatchPatternSet_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.112Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_calloc, maybe_pod_calloc, js::ShapeTable::init, js::Shape::hashify [task 2018-07-03T22:20:19.114Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, js::ObjectGroup::newPlainObject [task 2018-07-03T22:20:19.115Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::Blob_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.116Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, pod_malloc [task 2018-07-03T22:20:19.117Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::XMLDocument_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.118Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at AllocateProtoAndIfaceCache, xpc::CreateGlobalObject, XPCWrappedNative::WrapNewGlobal, xpc::InitClassesWithNewWrappedGlobal [task 2018-07-03T22:20:19.119Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, make_pod_array [task 2018-07-03T22:20:19.120Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::MozQueryInterface_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.121Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_calloc, maybe_pod_calloc, AllocScriptData, JSScript::partiallyInit [task 2018-07-03T22:20:19.122Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, js::Nursery::allocateBuffer [task 2018-07-03T22:20:19.123Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, js::TypeNewScript::make [task 2018-07-03T22:20:19.125Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_calloc, maybe_pod_calloc, js::TypeNewScript::maybeAnalyze, js::CreateThisForFunctionWithProto [task 2018-07-03T22:20:19.125Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::NodeFilter_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.126Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at ProtoAndIfaceCache, AllocateProtoAndIfaceCache, mozilla::dom::CreateGlobal, mozilla::dom::Window_Binding::Wrap [task 2018-07-03T22:20:19.127Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at Realloc, nsTArray_base, AppendElement, mozilla::dom::ChromeUtils::GenerateQI [task 2018-07-03T22:20:19.128Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at ProtoAndIfaceCache, AllocateProtoAndIfaceCache, xpc::CreateGlobalObject, XPCWrappedNative::WrapNewGlobal [task 2018-07-03T22:20:19.129Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::ChildSHistory_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.129Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_calloc, maybe_pod_calloc, partiallyInit, JSScript::initFunctionPrototype [task 2018-07-03T22:20:19.130Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at EntrySlotOrCreate, EntrySlotOrCreate, mozilla::dom::File_Binding::CreateInterfaceObjects, mozilla::dom::GetPerInterfaceObjectHandle [task 2018-07-03T22:20:19.131Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, CreateTraceList, DefineSimpleTypeDescr [task 2018-07-03T22:20:19.131Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, CreateFunctionPrototype, js::GlobalObject::resolveConstructor [task 2018-07-03T22:20:19.132Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, js::frontend::CreateScriptSourceObject [task 2018-07-03T22:20:19.132Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, CopyScopeData, js::VarScope::create [task 2018-07-03T22:20:19.133Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at mozilla::dom::ChromeUtils::GenerateQI, mozilla::dom::ChromeUtils_Binding::generateQI, CallJSNative, js::InternalCallOrConstruct [task 2018-07-03T22:20:19.134Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, pod_malloc, js::ObjectGroup::allocationSiteGroup [task 2018-07-03T22:20:19.134Z] ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, make_pod_array, JSExternalString::ensureFlat [task 2018-07-03T22:20:19.135Z] INFO - runtests.py | Application ran for: 0:05:11.974015
Flags: needinfo?(cpearce)
Comment on attachment 8991572 [details] Bug 1472580 - Ensure we always get a allow/cancel response to an autoplay media permission request. https://reviewboard.mozilla.org/r/256500/#review263380 I think this needs some minor tweaks to be easier to follow. ::: dom/base/nsGlobalWindowInner.cpp:8169 (Diff revision 1) > } > return rootTreeItem->GetDocument()->GetInnerWindow(); > } > > -already_AddRefed<mozilla::AutoplayRequest> > +already_AddRefed<mozilla::AutoplayPermissionManager> > nsPIDOMWindowInner::GetAutoplayRequest() This is weird. Method named GetAutoplayRequest returns something totally different. ::: dom/base/nsGlobalWindowInner.cpp:8177 (Diff revision 1) > nsPIDOMWindowInner* window = GetTopLevelInnerWindow(this); > if (!window) { > return nullptr; > } > if (!window->mAutoplayRequest) { > - window->mAutoplayRequest = AutoplayRequest::Create(nsGlobalWindowInner::Cast(window)); > + window->mAutoplayRequest = similar here. mAutoplayRequest stores some different kind of object in it. ::: dom/html/AutoplayPermissionManager.h:51 (Diff revision 1) > + // rejects in the content process after an IPC round trip). > RefPtr<GenericPromise> RequestWithPrompt(); > > + // Called by AutoplayPermissionRequest to approve the play request, > + // and resolve the promise. > + void Resolve(); Since this code doesn't use Promises but MozPromises which are totally different things, I'd prefer if at least the comment would describe precisely what is being resolved, so MozPromise, not promise. But I think also the names of the methods, Resolve() and Reject() should be something else, describing what they really do.
Attachment #8991572 - Flags: review?(bugs) → review-
(interesting how totally unreadable MozReview manages to make that patch. Downloading the patch works)
Comment on attachment 8991572 [details] Bug 1472580 - Ensure we always get a allow/cancel response to an autoplay media permission request. https://reviewboard.mozilla.org/r/256500/#review263850 ::: dom/html/AutoplayPermissionRequest.h:21 (Diff revision 2) > +class AutoplayPermissionManager; > + > +// The AutoplayPermissionRequest is the object we pass off to the chrome JS > +// code to represent a request for permission to autoplay. Unfortunately the > +// front end code doesn't guarantee to give us an approve/cancel callback in > +// all cases. So the AutoplayPermissionManager keeps a weak reference to the AutoplayPermissionManager does not seem to keep any reference to the request, but request keeps a weak reference to the manager. ::: dom/html/AutoplayPermissionRequest.h:27 (Diff revision 2) > +// AutoplayPermissionRequest. If chrome JS dismisses the permission request for > +// whatever reason (tab closed, user presses ESC, navigation, etc), the > +// permission UI code will drop its reference to the AutoplayPermissionRequest > +// and it will be destroyed. AutoplayPermissionRequest's destructor calls back > +// the AutoplayPermissionManager with a cancel operation, so we can thus > +// guarantee to always get an approve or cancel callback. Doesn't seem to be quite right. mManager is WeakPtr, so the manager may actually go away before approving or canceling.
Attachment #8991572 - Flags: review?(bugs) → review+
Comment on attachment 8991572 [details] Bug 1472580 - Ensure we always get a allow/cancel response to an autoplay media permission request. https://reviewboard.mozilla.org/r/256500/#review263850 > AutoplayPermissionManager does not seem to keep any reference to the request, but request keeps a weak reference to the manager. I've adjusted the comments here to match reality. Thanks for the review!
Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a2c0973c0454 Gesture activate documents which are played via the tab audio indicator. r=mconley https://hg.mozilla.org/integration/autoland/rev/a1a2ee0ee145 Test that starting play from tab audio indicator overrides block autoplay. r=mconley https://hg.mozilla.org/integration/autoland/rev/9b4b53a14538 Ensure we always get a allow/cancel response to an autoplay media permission request. r=smaug
Flags: needinfo?(cpearce)
Verified on Nightly 63.0a1(20180823100106), media with sound is played if the 'play tab' button is clicked.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: