From e91d1ff3ecae1f2541b4a5bda945046b9ca61a0b Mon Sep 17 00:00:00 2001 From: Karl Tomlinson Date: Mon, 11 Oct 2021 20:44:39 +0000 Subject: [PATCH] Bug 1732410 wait for focus before proceeding for enumerateDevices() Promises r=jib,nika https://github.com/w3c/mediacapture-main/pull/574 Focus on browser chrome widgets is accepted provided the tab is fully active and foreground. https://github.com/w3c/mediacapture-main/issues/752#issuecomment-742036800 Differential Revision: https://phabricator.services.mozilla.com/D127051 --- docshell/base/BrowsingContext.cpp | 10 +++- dom/base/Navigator.h | 1 + dom/base/nsGlobalWindowInner.cpp | 16 +++++++ dom/base/nsGlobalWindowInner.h | 6 +++ dom/base/nsGlobalWindowOuter.cpp | 2 +- dom/base/nsPIDOMWindow.h | 2 + dom/media/MediaDevices.cpp | 46 ++++++++++++++++--- dom/media/MediaDevices.h | 8 ++++ ...rateDevices-with-navigation.https.html.ini | 4 -- ...merateDevices-in-background.https.html.ini | 2 - ...merateDevices-without-focus.https.html.ini | 3 -- 11 files changed, 81 insertions(+), 19 deletions(-) delete mode 100644 testing/web-platform/meta/mediacapture-streams/enumerateDevices-with-navigation.https.html.ini delete mode 100644 testing/web-platform/mozilla/meta/mediacapture-streams/enumerateDevices-without-focus.https.html.ini diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index e0956a07f7a8..2c34a2709f8c 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -33,6 +33,7 @@ #include "mozilla/dom/HTMLIFrameElement.h" #include "mozilla/dom/Location.h" #include "mozilla/dom/LocationBinding.h" +#include "mozilla/dom/MediaDevices.h" #include "mozilla/dom/PopupBlocker.h" #include "mozilla/dom/ScriptSettings.h" #include "mozilla/dom/SessionStorageManager.h" @@ -2930,14 +2931,19 @@ void BrowsingContext::DidSet(FieldIndex, if (RefPtr doc = aContext->GetExtantDocument()) { doc->UpdateDocumentStates(NS_DOCUMENT_STATE_WINDOW_INACTIVE, true); + RefPtr win = doc->GetInnerWindow(); + RefPtr devices; + if (isActivateEvent && (devices = win->GetExtantMediaDevices())) { + devices->BrowserWindowBecameActive(); + } + if (XRE_IsContentProcess() && (!aContext->GetParent() || !aContext->GetParent()->IsInProcess())) { // Send the inner window an activate/deactivate event if // the context is the top of a sub-tree of in-process // contexts. nsContentUtils::DispatchEventOnlyToChrome( - doc, doc->GetWindow()->GetCurrentInnerWindow(), - isActivateEvent ? u"activate"_ns : u"deactivate"_ns, + doc, win, isActivateEvent ? u"activate"_ns : u"deactivate"_ns, CanBubble::eYes, Cancelable::eYes, nullptr); } } diff --git a/dom/base/Navigator.h b/dom/base/Navigator.h index 986733342325..a949ecd2407a 100644 --- a/dom/base/Navigator.h +++ b/dom/base/Navigator.h @@ -178,6 +178,7 @@ class Navigator final : public nsISupports, public nsWrapperCache { already_AddRefed MozTCPSocket(); network::Connection* GetConnection(ErrorResult& aRv); MediaDevices* GetMediaDevices(ErrorResult& aRv); + MediaDevices* GetExtantMediaDevices() const { return mMediaDevices; }; void GetGamepads(nsTArray>& aGamepads, ErrorResult& aRv); GamepadServiceTest* RequestGamepadServiceTest(); diff --git a/dom/base/nsGlobalWindowInner.cpp b/dom/base/nsGlobalWindowInner.cpp index 6b43e41aadf9..bc405857782f 100644 --- a/dom/base/nsGlobalWindowInner.cpp +++ b/dom/base/nsGlobalWindowInner.cpp @@ -137,6 +137,7 @@ #include "mozilla/dom/LocalStorage.h" #include "mozilla/dom/LocalStorageCommon.h" #include "mozilla/dom/Location.h" +#include "mozilla/dom/MediaDevices.h" #include "mozilla/dom/MediaKeys.h" #include "mozilla/dom/NavigatorBinding.h" #include "mozilla/dom/Nullable.h" @@ -2310,6 +2311,10 @@ Navigator* nsPIDOMWindowInner::Navigator() { return mNavigator; } +MediaDevices* nsPIDOMWindowInner::GetExtantMediaDevices() const { + return mNavigator ? mNavigator->GetExtantMediaDevices() : nullptr; +} + VisualViewport* nsGlobalWindowInner::VisualViewport() { if (!mVisualViewport) { mVisualViewport = new mozilla::dom::VisualViewport(this); @@ -5570,6 +5575,10 @@ void nsGlobalWindowInner::Resume(bool aIncludeSubWindows) { mAudioContexts[i]->ResumeFromChrome(); } + if (RefPtr devices = GetExtantMediaDevices()) { + devices->WindowResumed(); + } + mTimeoutManager->Resume(); ResumeIdleRequests(); @@ -5721,6 +5730,13 @@ void nsGlobalWindowInner::SyncStateFromParentWindow() { } } +void nsGlobalWindowInner::UpdateBackgroundState() { + if (RefPtr devices = GetExtantMediaDevices()) { + devices->BackgroundStateChanged(); + } + mTimeoutManager->UpdateBackgroundState(); +} + template CallState nsGlobalWindowInner::CallOnInProcessDescendantsInternal( BrowsingContext* aBrowsingContext, bool aChildOnly, Method aMethod, diff --git a/dom/base/nsGlobalWindowInner.h b/dom/base/nsGlobalWindowInner.h index 3e7eb8558c78..42112c9c6570 100644 --- a/dom/base/nsGlobalWindowInner.h +++ b/dom/base/nsGlobalWindowInner.h @@ -325,6 +325,12 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget, virtual bool IsFrozen() const override; void SyncStateFromParentWindow(); + // Called on the current inner window of a browsing context when its + // background state changes according to selected tab or visibility of the + // browser window. Used with Suspend()/Resume() or Freeze()/Thaw() because + // background state may change while the inner window is not current. + void UpdateBackgroundState(); + mozilla::dom::DebuggerNotificationManager* GetOrCreateDebuggerNotificationManager() override; diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index f6d7f6c4c37d..2aa3f216930e 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -6757,7 +6757,7 @@ void nsGlobalWindowOuter::SetIsBackground(bool aIsBackground) { nsGlobalWindowInner* inner = GetCurrentInnerWindowInternal(); if (inner && changed) { - inner->mTimeoutManager->UpdateBackgroundState(); + inner->UpdateBackgroundState(); } if (aIsBackground) { diff --git a/dom/base/nsPIDOMWindow.h b/dom/base/nsPIDOMWindow.h index ca1b1137dfe8..74ef1aefd8f6 100644 --- a/dom/base/nsPIDOMWindow.h +++ b/dom/base/nsPIDOMWindow.h @@ -62,6 +62,7 @@ class DocGroup; class Document; class Element; class Location; +class MediaDevices; class MediaKeys; class Navigator; class Performance; @@ -577,6 +578,7 @@ class nsPIDOMWindowInner : public mozIDOMWindow { uint32_t GetMarkedCCGeneration() { return mMarkedCCGeneration; } mozilla::dom::Navigator* Navigator(); + mozilla::dom::MediaDevices* GetExtantMediaDevices() const; virtual mozilla::dom::Location* Location() = 0; virtual nsresult GetControllers(nsIControllers** aControllers) = 0; diff --git a/dom/media/MediaDevices.cpp b/dom/media/MediaDevices.cpp index f025a719369f..2be6f8c35102 100644 --- a/dom/media/MediaDevices.cpp +++ b/dom/media/MediaDevices.cpp @@ -5,6 +5,7 @@ #include "mozilla/dom/MediaDevices.h" #include "AudioDeviceInfo.h" +#include "mozilla/dom/BrowsingContext.h" #include "mozilla/dom/Document.h" #include "mozilla/dom/MediaStreamBinding.h" #include "mozilla/dom/MediaDeviceInfo.h" @@ -139,10 +140,39 @@ already_AddRefed MediaDevices::EnumerateDevices(ErrorResult& aRv) { if (NS_WARN_IF(aRv.Failed())) { return nullptr; } + mPendingEnumerateDevicesPromises.AppendElement(p); + MaybeResumeDeviceExposure(); + return p.forget(); +} + +void MediaDevices::MaybeResumeDeviceExposure() { + if (mPendingEnumerateDevicesPromises.IsEmpty()) { + return; + } + nsPIDOMWindowInner* window = GetOwner(); + if (!window || !window->IsFullyActive()) { + return; + } + BrowsingContext* bc = window->GetBrowsingContext(); + if (!bc->IsActive() || // not foreground tab + !bc->GetIsActiveBrowserWindow()) { // browser window does not have focus + return; + } + + auto pending = std::move(mPendingEnumerateDevicesPromises); + for (auto& promise : pending) { + ResumeEnumerateDevices(std::move(promise)); + } +} + +void MediaDevices::ResumeEnumerateDevices(RefPtr aPromise) { + nsCOMPtr window = GetOwner(); + MOZ_ASSERT(window, "Fully active document should have window"); RefPtr self(this); - MediaManager::Get()->EnumerateDevices(owner)->Then( + MediaManager::Get()->EnumerateDevices(window)->Then( GetCurrentSerialEventTarget(), __func__, - [this, self, p](RefPtr&& aDevices) { + [this, self, + aPromise](RefPtr&& aDevices) { nsPIDOMWindowInner* window = GetWindowIfCurrent(); if (!window) { return; // Leave Promise pending after navigation by design. @@ -189,16 +219,15 @@ already_AddRefed MediaDevices::EnumerateDevices(ErrorResult& aRv) { infos.AppendElement(MakeRefPtr( device->mID, device->mKind, label, device->mGroupID)); } - p->MaybeResolve(std::move(infos)); + aPromise->MaybeResolve(std::move(infos)); }, - [this, self, p](const RefPtr& error) { + [this, self, aPromise](const RefPtr& error) { nsPIDOMWindowInner* window = GetWindowIfCurrent(); if (!window) { return; // Leave Promise pending after navigation by design. } - error->Reject(p); + error->Reject(aPromise); }); - return p.forget(); } already_AddRefed MediaDevices::GetDisplayMedia( @@ -479,7 +508,10 @@ RefPtr MediaDevices::GetSinkDevice( }); } -NS_IMPL_ISUPPORTS_INHERITED0(MediaDevices, DOMEventTargetHelper) +NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(MediaDevices, + DOMEventTargetHelper) +NS_IMPL_CYCLE_COLLECTION_INHERITED(MediaDevices, DOMEventTargetHelper, + mPendingEnumerateDevicesPromises) void MediaDevices::OnDeviceChange() { MOZ_ASSERT(NS_IsMainThread()); diff --git a/dom/media/MediaDevices.h b/dom/media/MediaDevices.h index 5c1526be46e7..1240de9b41df 100644 --- a/dom/media/MediaDevices.h +++ b/dom/media/MediaDevices.h @@ -39,6 +39,7 @@ class MediaDevices final : public DOMEventTargetHelper { explicit MediaDevices(nsPIDOMWindowInner* aWindow); NS_DECL_ISUPPORTS_INHERITED + NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(MediaDevices, DOMEventTargetHelper) JSObject* WrapObject(JSContext* cx, JS::Handle aGivenProto) override; @@ -79,14 +80,21 @@ class MediaDevices final : public DOMEventTargetHelper { void EventListenerAdded(nsAtom* aType) override; using DOMEventTargetHelper::EventListenerAdded; + void BackgroundStateChanged() { MaybeResumeDeviceExposure(); } + void WindowResumed() { MaybeResumeDeviceExposure(); } + void BrowserWindowBecameActive() { MaybeResumeDeviceExposure(); } + private: class GumResolver; class EnumDevResolver; class GumRejecter; virtual ~MediaDevices(); + void MaybeResumeDeviceExposure(); + void ResumeEnumerateDevices(RefPtr aPromise); nsTHashSet mExplicitlyGrantedAudioOutputIds; + nsTArray> mPendingEnumerateDevicesPromises; nsCOMPtr mFuzzTimer; // Connect/Disconnect on main thread only diff --git a/testing/web-platform/meta/mediacapture-streams/enumerateDevices-with-navigation.https.html.ini b/testing/web-platform/meta/mediacapture-streams/enumerateDevices-with-navigation.https.html.ini deleted file mode 100644 index 90b6394a15da..000000000000 --- a/testing/web-platform/meta/mediacapture-streams/enumerateDevices-with-navigation.https.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[enumerateDevices-with-navigation.https.html] - expected: TIMEOUT - [enumerateDevices with navigation] - expected: TIMEOUT diff --git a/testing/web-platform/mozilla/meta/mediacapture-streams/enumerateDevices-in-background.https.html.ini b/testing/web-platform/mozilla/meta/mediacapture-streams/enumerateDevices-in-background.https.html.ini index 17201c5206a6..0b5b37ae28e9 100644 --- a/testing/web-platform/mozilla/meta/mediacapture-streams/enumerateDevices-in-background.https.html.ini +++ b/testing/web-platform/mozilla/meta/mediacapture-streams/enumerateDevices-in-background.https.html.ini @@ -1,5 +1,3 @@ [enumerateDevices-in-background.https.html] disabled: if buildapp != 'browser': uses Firefox-for-Desktop specific API - [enumerateDevices in background] - expected: FAIL diff --git a/testing/web-platform/mozilla/meta/mediacapture-streams/enumerateDevices-without-focus.https.html.ini b/testing/web-platform/mozilla/meta/mediacapture-streams/enumerateDevices-without-focus.https.html.ini deleted file mode 100644 index e86ac5310096..000000000000 --- a/testing/web-platform/mozilla/meta/mediacapture-streams/enumerateDevices-without-focus.https.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[enumerateDevices-without-focus.https.html] - [enumerateDevices without focus] - expected: FAIL