From 9d394c09bbd1ca5941fc4bf7ce4d8bf4e782a868 Mon Sep 17 00:00:00 2001 From: Kearwood Gilbert Date: Tue, 14 Feb 2017 11:21:09 -0800 Subject: [PATCH] Bug 1335606 - Add 'display' value to Navigator.vrdisplayconnect, Navigator.vrdisplaydisconnect, and Navigator.vrdisplaypresentchange events r=smaug MozReview-Commit-ID: FLZ7u98mqqi --HG-- extra : rebase_source : dd783f3c085efc50c4114ea45ac3f695670079fd --- dom/base/nsGlobalWindow.cpp | 107 ++++++++++++++++++++++++++++++++-- dom/base/nsGlobalWindow.h | 5 +- dom/vr/VREventObserver.cpp | 41 ++++++++----- dom/vr/VREventObserver.h | 13 +++-- gfx/vr/VRDisplayClient.cpp | 2 +- gfx/vr/ipc/VRManagerChild.cpp | 80 +++++++++++++++---------- gfx/vr/ipc/VRManagerChild.h | 15 +++-- 7 files changed, 197 insertions(+), 66 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index a2faa730eff3..4341588e3539 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -10444,8 +10444,10 @@ void nsGlobalWindow::DisableVRUpdates() { MOZ_ASSERT(IsInnerWindow()); - - mVREventObserver = nullptr; + if (mVREventObserver) { + mVREventObserver->DisconnectFromOwner(); + mVREventObserver = nullptr; + } } void @@ -13472,6 +13474,8 @@ void nsGlobalWindow::DispatchVRDisplayActivate(uint32_t aDisplayID, mozilla::dom::VRDisplayEventReason aReason) { + // Search for the display identified with aDisplayID and fire the + // event if found. for (auto display : mVRDisplays) { if (display->DisplayId() == aDisplayID && !display->IsAnyPresenting()) { @@ -13479,7 +13483,7 @@ nsGlobalWindow::DispatchVRDisplayActivate(uint32_t aDisplayID, // display already. VRDisplayEventInit init; - init.mBubbles = true; + init.mBubbles = false; init.mCancelable = false; init.mDisplay = display; init.mReason.Construct(aReason); @@ -13494,7 +13498,9 @@ nsGlobalWindow::DispatchVRDisplayActivate(uint32_t aDisplayID, event->SetTrusted(true); bool defaultActionEnabled; Unused << DispatchEvent(event, &defaultActionEnabled); - break; + // Once we dispatch the event, we must not access any members as an event + // listener can do anything, including closing windows. + return; } } } @@ -13503,13 +13509,15 @@ void nsGlobalWindow::DispatchVRDisplayDeactivate(uint32_t aDisplayID, mozilla::dom::VRDisplayEventReason aReason) { + // Search for the display identified with aDisplayID and fire the + // event if found. for (auto display : mVRDisplays) { if (display->DisplayId() == aDisplayID && display->IsPresenting()) { // We only want to trigger this event to content that is presenting to // the display already. VRDisplayEventInit init; - init.mBubbles = true; + init.mBubbles = false; init.mCancelable = false; init.mDisplay = display; init.mReason.Construct(aReason); @@ -13518,9 +13526,96 @@ nsGlobalWindow::DispatchVRDisplayDeactivate(uint32_t aDisplayID, VRDisplayEvent::Constructor(this, NS_LITERAL_STRING("vrdisplaydeactivate"), init); + event->SetTrusted(true); bool defaultActionEnabled; Unused << DispatchEvent(event, &defaultActionEnabled); - break; + // Once we dispatch the event, we must not access any members as an event + // listener can do anything, including closing windows. + return; + } + } +} + +void +nsGlobalWindow::DispatchVRDisplayConnect(uint32_t aDisplayID) +{ + // Search for the display identified with aDisplayID and fire the + // event if found. + for (auto display : mVRDisplays) { + if (display->DisplayId() == aDisplayID) { + // Fire event even if not presenting to the display. + VRDisplayEventInit init; + init.mBubbles = false; + init.mCancelable = false; + init.mDisplay = display; + // VRDisplayEvent.reason is not set for vrdisplayconnect + + RefPtr event = + VRDisplayEvent::Constructor(this, + NS_LITERAL_STRING("vrdisplayconnect"), + init); + event->SetTrusted(true); + bool defaultActionEnabled; + Unused << DispatchEvent(event, &defaultActionEnabled); + // Once we dispatch the event, we must not access any members as an event + // listener can do anything, including closing windows. + return; + } + } +} + +void +nsGlobalWindow::DispatchVRDisplayDisconnect(uint32_t aDisplayID) +{ + // Search for the display identified with aDisplayID and fire the + // event if found. + for (auto display : mVRDisplays) { + if (display->DisplayId() == aDisplayID) { + // Fire event even if not presenting to the display. + VRDisplayEventInit init; + init.mBubbles = false; + init.mCancelable = false; + init.mDisplay = display; + // VRDisplayEvent.reason is not set for vrdisplaydisconnect + + RefPtr event = + VRDisplayEvent::Constructor(this, + NS_LITERAL_STRING("vrdisplaydisconnect"), + init); + event->SetTrusted(true); + bool defaultActionEnabled; + Unused << DispatchEvent(event, &defaultActionEnabled); + // Once we dispatch the event, we must not access any members as an event + // listener can do anything, including closing windows. + return; + } + } +} + +void +nsGlobalWindow::DispatchVRDisplayPresentChange(uint32_t aDisplayID) +{ + // Search for the display identified with aDisplayID and fire the + // event if found. + for (auto display : mVRDisplays) { + if (display->DisplayId() == aDisplayID) { + // Fire event even if not presenting to the display. + VRDisplayEventInit init; + init.mBubbles = false; + init.mCancelable = false; + init.mDisplay = display; + // VRDisplayEvent.reason is not set for vrdisplaypresentchange + + RefPtr event = + VRDisplayEvent::Constructor(this, + NS_LITERAL_STRING("vrdisplaypresentchange"), + init); + event->SetTrusted(true); + bool defaultActionEnabled; + Unused << DispatchEvent(event, &defaultActionEnabled); + // Once we dispatch the event, we must not access any members as an event + // listener can do anything, including closing windows. + return; } } } diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index 0ea7f2c27095..05fed83d9f65 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -768,6 +768,9 @@ public: mozilla::dom::VRDisplayEventReason aReason); void DispatchVRDisplayDeactivate(uint32_t aDisplayID, mozilla::dom::VRDisplayEventReason aReason); + void DispatchVRDisplayConnect(uint32_t aDisplayID); + void DispatchVRDisplayDisconnect(uint32_t aDisplayID); + void DispatchVRDisplayPresentChange(uint32_t aDisplayID); #define EVENT(name_, id_, type_, struct_) \ mozilla::dom::EventHandlerNonNull* GetOn##name_() \ @@ -2027,7 +2030,7 @@ protected: // The VR Displays for this window nsTArray> mVRDisplays; - nsAutoPtr mVREventObserver; + RefPtr mVREventObserver; friend class nsDOMScriptableHelper; friend class nsDOMWindowUtils; diff --git a/dom/vr/VREventObserver.cpp b/dom/vr/VREventObserver.cpp index 67f1fe062002..eaa33a70693c 100644 --- a/dom/vr/VREventObserver.cpp +++ b/dom/vr/VREventObserver.cpp @@ -33,6 +33,18 @@ VREventObserver::VREventObserver(nsGlobalWindow* aGlobalWindow) VREventObserver::~VREventObserver() { + DisconnectFromOwner(); +} + +void +VREventObserver::DisconnectFromOwner() +{ + // In the event that nsGlobalWindow is deallocated, VREventObserver may + // still be AddRef'ed elsewhere. Ensure that we don't UAF by + // dereferencing mWindow. + mWindow = nullptr; + + // Unregister from VRManagerChild VRManagerChild* vmc = VRManagerChild::Get(); if (vmc) { vmc->RemoveListener(this); @@ -42,7 +54,7 @@ VREventObserver::~VREventObserver() void VREventObserver::NotifyVRDisplayMounted(uint32_t aDisplayID) { - if (mWindow->AsInner()->IsCurrentInnerWindow()) { + if (mWindow && mWindow->AsInner()->IsCurrentInnerWindow()) { MOZ_ASSERT(nsContentUtils::IsSafeToRunScript()); mWindow->DispatchVRDisplayActivate(aDisplayID, VRDisplayEventReason::Mounted); @@ -52,7 +64,7 @@ VREventObserver::NotifyVRDisplayMounted(uint32_t aDisplayID) void VREventObserver::NotifyVRDisplayNavigation(uint32_t aDisplayID) { - if (mWindow->AsInner()->IsCurrentInnerWindow()) { + if (mWindow && mWindow->AsInner()->IsCurrentInnerWindow()) { MOZ_ASSERT(nsContentUtils::IsSafeToRunScript()); mWindow->DispatchVRDisplayActivate(aDisplayID, VRDisplayEventReason::Navigation); @@ -62,7 +74,7 @@ VREventObserver::NotifyVRDisplayNavigation(uint32_t aDisplayID) void VREventObserver::NotifyVRDisplayRequested(uint32_t aDisplayID) { - if (mWindow->AsInner()->IsCurrentInnerWindow()) { + if (mWindow && mWindow->AsInner()->IsCurrentInnerWindow()) { MOZ_ASSERT(nsContentUtils::IsSafeToRunScript()); mWindow->DispatchVRDisplayActivate(aDisplayID, VRDisplayEventReason::Requested); @@ -72,7 +84,7 @@ VREventObserver::NotifyVRDisplayRequested(uint32_t aDisplayID) void VREventObserver::NotifyVRDisplayUnmounted(uint32_t aDisplayID) { - if (mWindow->AsInner()->IsCurrentInnerWindow()) { + if (mWindow && mWindow->AsInner()->IsCurrentInnerWindow()) { MOZ_ASSERT(nsContentUtils::IsSafeToRunScript()); mWindow->DispatchVRDisplayDeactivate(aDisplayID, VRDisplayEventReason::Unmounted); @@ -80,39 +92,36 @@ VREventObserver::NotifyVRDisplayUnmounted(uint32_t aDisplayID) } void -VREventObserver::NotifyVRDisplayConnect() +VREventObserver::NotifyVRDisplayConnect(uint32_t aDisplayID) { /** * We do not call nsGlobalWindow::NotifyActiveVRDisplaysChanged here, as we * can assume that a newly enumerated display is not presenting WebVR * content. */ - if (mWindow->AsInner()->IsCurrentInnerWindow()) { + if (mWindow && mWindow->AsInner()->IsCurrentInnerWindow()) { MOZ_ASSERT(nsContentUtils::IsSafeToRunScript()); - mWindow->GetOuterWindow()->DispatchCustomEvent( - NS_LITERAL_STRING("vrdisplayconnected")); + mWindow->DispatchVRDisplayConnect(aDisplayID); } } void -VREventObserver::NotifyVRDisplayDisconnect() +VREventObserver::NotifyVRDisplayDisconnect(uint32_t aDisplayID) { - if (mWindow->AsInner()->IsCurrentInnerWindow()) { + if (mWindow && mWindow->AsInner()->IsCurrentInnerWindow()) { mWindow->NotifyActiveVRDisplaysChanged(); MOZ_ASSERT(nsContentUtils::IsSafeToRunScript()); - mWindow->GetOuterWindow()->DispatchCustomEvent( - NS_LITERAL_STRING("vrdisplaydisconnected")); + mWindow->DispatchVRDisplayDisconnect(aDisplayID); } } void -VREventObserver::NotifyVRDisplayPresentChange() +VREventObserver::NotifyVRDisplayPresentChange(uint32_t aDisplayID) { - if (mWindow->AsInner()->IsCurrentInnerWindow()) { + if (mWindow && mWindow->AsInner()->IsCurrentInnerWindow()) { mWindow->NotifyActiveVRDisplaysChanged(); MOZ_ASSERT(nsContentUtils::IsSafeToRunScript()); - mWindow->GetOuterWindow()->DispatchCustomEvent( - NS_LITERAL_STRING("vrdisplaypresentchange")); + mWindow->DispatchVRDisplayPresentChange(aDisplayID); } } diff --git a/dom/vr/VREventObserver.h b/dom/vr/VREventObserver.h index 10333f163793..536862495358 100644 --- a/dom/vr/VREventObserver.h +++ b/dom/vr/VREventObserver.h @@ -8,6 +8,7 @@ #define mozilla_dom_VREventObserver_h #include "mozilla/dom/VRDisplayEventBinding.h" +#include "nsISupportsImpl.h" // for NS_INLINE_DECL_REFCOUNTING class nsGlobalWindow; @@ -17,18 +18,22 @@ namespace dom { class VREventObserver final { public: - ~VREventObserver(); + NS_INLINE_DECL_REFCOUNTING(VREventObserver) explicit VREventObserver(nsGlobalWindow* aGlobalWindow); void NotifyVRDisplayMounted(uint32_t aDisplayID); void NotifyVRDisplayUnmounted(uint32_t aDisplayID); void NotifyVRDisplayNavigation(uint32_t aDisplayID); void NotifyVRDisplayRequested(uint32_t aDisplayID); - void NotifyVRDisplayConnect(); - void NotifyVRDisplayDisconnect(); - void NotifyVRDisplayPresentChange(); + void NotifyVRDisplayConnect(uint32_t aDisplayID); + void NotifyVRDisplayDisconnect(uint32_t aDisplayID); + void NotifyVRDisplayPresentChange(uint32_t aDisplayID); + + void DisconnectFromOwner(); private: + ~VREventObserver(); + // Weak pointer, instance is owned by mWindow. nsGlobalWindow* MOZ_NON_OWNING_REF mWindow; }; diff --git a/gfx/vr/VRDisplayClient.cpp b/gfx/vr/VRDisplayClient.cpp index 6229da04f603..e14484564d4e 100644 --- a/gfx/vr/VRDisplayClient.cpp +++ b/gfx/vr/VRDisplayClient.cpp @@ -113,7 +113,7 @@ VRDisplayClient::NotifyVsync() // Check if we need to trigger onVRDisplayPresentChange event if (bLastEventWasPresenting != isPresenting) { bLastEventWasPresenting = isPresenting; - vm->FireDOMVRDisplayPresentChangeEvent(); + vm->FireDOMVRDisplayPresentChangeEvent(mDisplayInfo.mDisplayID); } // Check if we need to trigger onvrdisplayactivate event diff --git a/gfx/vr/ipc/VRManagerChild.cpp b/gfx/vr/ipc/VRManagerChild.cpp index d427409b8e9a..c61c0cb7a7e5 100644 --- a/gfx/vr/ipc/VRManagerChild.cpp +++ b/gfx/vr/ipc/VRManagerChild.cpp @@ -203,8 +203,8 @@ VRManagerChild::DeallocPVRLayerChild(PVRLayerChild* actor) void VRManagerChild::UpdateDisplayInfo(nsTArray& aDisplayUpdates) { - bool bDisplayConnected = false; - bool bDisplayDisconnected = false; + nsTArray disconnectedDisplays; + nsTArray connectedDisplays; // Check if any displays have been disconnected for (auto& display : mDisplays) { @@ -217,7 +217,7 @@ VRManagerChild::UpdateDisplayInfo(nsTArray& aDisplayUpdates) } if (!found) { display->NotifyDisconnected(); - bDisplayDisconnected = true; + disconnectedDisplays.AppendElement(display->GetDisplayInfo().GetDisplayID()); } } @@ -230,10 +230,10 @@ VRManagerChild::UpdateDisplayInfo(nsTArray& aDisplayUpdates) const VRDisplayInfo& prevInfo = display->GetDisplayInfo(); if (prevInfo.GetDisplayID() == displayUpdate.GetDisplayID()) { if (displayUpdate.GetIsConnected() && !prevInfo.GetIsConnected()) { - bDisplayConnected = true; + connectedDisplays.AppendElement(displayUpdate.GetDisplayID()); } if (!displayUpdate.GetIsConnected() && prevInfo.GetIsConnected()) { - bDisplayDisconnected = true; + disconnectedDisplays.AppendElement(displayUpdate.GetDisplayID()); } display->UpdateDisplayInfo(displayUpdate); displays.AppendElement(display); @@ -243,17 +243,19 @@ VRManagerChild::UpdateDisplayInfo(nsTArray& aDisplayUpdates) } if (isNewDisplay) { displays.AppendElement(new VRDisplayClient(displayUpdate)); - bDisplayConnected = true; + connectedDisplays.AppendElement(displayUpdate.GetDisplayID()); } } mDisplays = displays; - if (bDisplayConnected) { - FireDOMVRDisplayConnectEvent(); + // We wish to fire the events only after mDisplays is updated + for (uint32_t displayID : disconnectedDisplays) { + FireDOMVRDisplayDisconnectEvent(displayID); } - if (bDisplayDisconnected) { - FireDOMVRDisplayDisconnectEvent(); + + for (uint32_t displayID : connectedDisplays) { + FireDOMVRDisplayConnectEvent(displayID); } mDisplaysInitialized = true; @@ -522,30 +524,36 @@ VRManagerChild::FireDOMVRDisplayUnmountedEvent(uint32_t aDisplayID) } void -VRManagerChild::FireDOMVRDisplayConnectEvent() +VRManagerChild::FireDOMVRDisplayConnectEvent(uint32_t aDisplayID) { - nsContentUtils::AddScriptRunner(NewRunnableMethod(this, - &VRManagerChild::FireDOMVRDisplayConnectEventInternal)); + nsContentUtils::AddScriptRunner(NewRunnableMethod(this, + &VRManagerChild::FireDOMVRDisplayConnectEventInternal, + aDisplayID)); } void -VRManagerChild::FireDOMVRDisplayDisconnectEvent() +VRManagerChild::FireDOMVRDisplayDisconnectEvent(uint32_t aDisplayID) { - nsContentUtils::AddScriptRunner(NewRunnableMethod(this, - &VRManagerChild::FireDOMVRDisplayDisconnectEventInternal)); + nsContentUtils::AddScriptRunner(NewRunnableMethod(this, + &VRManagerChild::FireDOMVRDisplayDisconnectEventInternal, + aDisplayID)); } void -VRManagerChild::FireDOMVRDisplayPresentChangeEvent() +VRManagerChild::FireDOMVRDisplayPresentChangeEvent(uint32_t aDisplayID) { - nsContentUtils::AddScriptRunner(NewRunnableMethod(this, - &VRManagerChild::FireDOMVRDisplayPresentChangeEventInternal)); + nsContentUtils::AddScriptRunner(NewRunnableMethod(this, + &VRManagerChild::FireDOMVRDisplayPresentChangeEventInternal, + aDisplayID)); } void VRManagerChild::FireDOMVRDisplayMountedEventInternal(uint32_t aDisplayID) { - for (auto& listener : mListeners) { + // Iterate over a copy of mListeners, as dispatched events may modify it. + nsTArray> listeners; + listeners = mListeners; + for (auto& listener : listeners) { listener->NotifyVRDisplayMounted(aDisplayID); } } @@ -553,32 +561,44 @@ VRManagerChild::FireDOMVRDisplayMountedEventInternal(uint32_t aDisplayID) void VRManagerChild::FireDOMVRDisplayUnmountedEventInternal(uint32_t aDisplayID) { - for (auto& listener : mListeners) { + // Iterate over a copy of mListeners, as dispatched events may modify it. + nsTArray> listeners; + listeners = mListeners; + for (auto& listener : listeners) { listener->NotifyVRDisplayUnmounted(aDisplayID); } } void -VRManagerChild::FireDOMVRDisplayConnectEventInternal() +VRManagerChild::FireDOMVRDisplayConnectEventInternal(uint32_t aDisplayID) { - for (auto& listener : mListeners) { - listener->NotifyVRDisplayConnect(); + // Iterate over a copy of mListeners, as dispatched events may modify it. + nsTArray> listeners; + listeners = mListeners; + for (auto& listener : listeners) { + listener->NotifyVRDisplayConnect(aDisplayID); } } void -VRManagerChild::FireDOMVRDisplayDisconnectEventInternal() +VRManagerChild::FireDOMVRDisplayDisconnectEventInternal(uint32_t aDisplayID) { - for (auto& listener : mListeners) { - listener->NotifyVRDisplayDisconnect(); + // Iterate over a copy of mListeners, as dispatched events may modify it. + nsTArray> listeners; + listeners = mListeners; + for (auto& listener : listeners) { + listener->NotifyVRDisplayDisconnect(aDisplayID); } } void -VRManagerChild::FireDOMVRDisplayPresentChangeEventInternal() +VRManagerChild::FireDOMVRDisplayPresentChangeEventInternal(uint32_t aDisplayID) { - for (auto& listener : mListeners) { - listener->NotifyVRDisplayPresentChange(); + // Iterate over a copy of mListeners, as dispatched events may modify it. + nsTArray> listeners; + listeners = mListeners; + for (auto& listener : listeners) { + listener->NotifyVRDisplayPresentChange(aDisplayID); } } diff --git a/gfx/vr/ipc/VRManagerChild.h b/gfx/vr/ipc/VRManagerChild.h index b52eeca69e5c..ae02d098611a 100644 --- a/gfx/vr/ipc/VRManagerChild.h +++ b/gfx/vr/ipc/VRManagerChild.h @@ -81,9 +81,9 @@ public: void UpdateDisplayInfo(nsTArray& aDisplayUpdates); void FireDOMVRDisplayMountedEvent(uint32_t aDisplayID); void FireDOMVRDisplayUnmountedEvent(uint32_t aDisplayID); - void FireDOMVRDisplayConnectEvent(); - void FireDOMVRDisplayDisconnectEvent(); - void FireDOMVRDisplayPresentChangeEvent(); + void FireDOMVRDisplayConnectEvent(uint32_t aDisplayID); + void FireDOMVRDisplayDisconnectEvent(uint32_t aDisplayID); + void FireDOMVRDisplayPresentChangeEvent(uint32_t aDisplayID); virtual void HandleFatalError(const char* aName, const char* aMsg) const override; @@ -141,9 +141,9 @@ private: void FireDOMVRDisplayMountedEventInternal(uint32_t aDisplayID); void FireDOMVRDisplayUnmountedEventInternal(uint32_t aDisplayID); - void FireDOMVRDisplayConnectEventInternal(); - void FireDOMVRDisplayDisconnectEventInternal(); - void FireDOMVRDisplayPresentChangeEventInternal(); + void FireDOMVRDisplayConnectEventInternal(uint32_t aDisplayID); + void FireDOMVRDisplayDisconnectEventInternal(uint32_t aDisplayID); + void FireDOMVRDisplayPresentChangeEventInternal(uint32_t aDisplayID); /** * Notify id of Texture When host side end its use. Transaction id is used to * make sure if there is no newer usage. @@ -167,8 +167,7 @@ private: int32_t mFrameRequestCallbackCounter; mozilla::TimeStamp mStartTimeStamp; - // Array of Weak pointers, instance is owned by nsGlobalWindow::mVREventObserver. - nsTArray mListeners; + nsTArray> mListeners; /** * Hold TextureClients refs until end of their usages on host side.