diff --git a/ipc/mscom/AgileReference.cpp b/ipc/mscom/AgileReference.cpp index 816d15df0c3c..c0fb458e5836 100644 --- a/ipc/mscom/AgileReference.cpp +++ b/ipc/mscom/AgileReference.cpp @@ -6,11 +6,10 @@ #include "mozilla/mscom/AgileReference.h" -#include "mozilla/Assertions.h" #include "mozilla/DebugOnly.h" #include "mozilla/DynamicallyLinkedFunctionPtr.h" +#include "mozilla/Assertions.h" #include "mozilla/Move.h" -#include "mozilla/mscom/Utils.h" #if NTDDI_VERSION < NTDDI_WINBLUE @@ -31,34 +30,10 @@ HRESULT WINAPI RoGetAgileReference(AgileReferenceOptions options, namespace mozilla { namespace mscom { -AgileReference::AgileReference() - : mIid() - , mGitCookie(0) -{ -} - AgileReference::AgileReference(REFIID aIid, IUnknown* aObject) : mIid(aIid) , mGitCookie(0) { - AssignInternal(aObject); -} - -void -AgileReference::Assign(REFIID aIid, IUnknown* aObject) -{ - Clear(); - mIid = aIid; - AssignInternal(aObject); -} - -void -AgileReference::AssignInternal(IUnknown* aObject) -{ - // We expect mIid to already be set - DebugOnly zeroIid = {}; - MOZ_ASSERT(mIid != zeroIid); - /* * There are two possible techniques for creating agile references. Starting * with Windows 8.1, we may use the RoGetAgileReference API, which is faster. @@ -71,7 +46,7 @@ AgileReference::AssignInternal(IUnknown* aObject) MOZ_ASSERT(aObject); if (pRoGetAgileReference && - SUCCEEDED(pRoGetAgileReference(AGILEREFERENCE_DEFAULT, mIid, aObject, + SUCCEEDED(pRoGetAgileReference(AGILEREFERENCE_DEFAULT, aIid, aObject, getter_AddRefs(mAgileRef)))) { return; } @@ -82,7 +57,7 @@ AgileReference::AssignInternal(IUnknown* aObject) return; } - DebugOnly hr = git->RegisterInterfaceInGlobal(aObject, mIid, + DebugOnly hr = git->RegisterInterfaceInGlobal(aObject, aIid, &mGitCookie); MOZ_ASSERT(SUCCEEDED(hr)); } @@ -97,16 +72,7 @@ AgileReference::AgileReference(AgileReference&& aOther) AgileReference::~AgileReference() { - Clear(); -} - -void -AgileReference::Clear() -{ - mIid = {}; - if (!mGitCookie) { - mAgileRef = nullptr; return; } @@ -118,27 +84,13 @@ AgileReference::Clear() DebugOnly hr = git->RevokeInterfaceFromGlobal(mGitCookie); MOZ_ASSERT(SUCCEEDED(hr)); - mGitCookie = 0; -} - -AgileReference& -AgileReference::operator=(AgileReference&& aOther) -{ - Clear(); - mIid = aOther.mIid; - aOther.mIid = {}; - mAgileRef = std::move(aOther.mAgileRef); - mGitCookie = aOther.mGitCookie; - aOther.mGitCookie = 0; - return *this; } HRESULT -AgileReference::Resolve(REFIID aIid, void** aOutInterface) const +AgileReference::Resolve(REFIID aIid, void** aOutInterface) { MOZ_ASSERT(aOutInterface); MOZ_ASSERT(mAgileRef || mGitCookie); - MOZ_ASSERT(IsCOMInitializedOnCurrentThread()); if (!aOutInterface) { return E_INVALIDARG; @@ -178,7 +130,7 @@ AgileReference::Resolve(REFIID aIid, void** aOutInterface) const return originalInterface->QueryInterface(aIid, aOutInterface); } -/* static */ IGlobalInterfaceTable* +IGlobalInterfaceTable* AgileReference::ObtainGit() { // Internally to COM, the Global Interface Table is a singleton, therefore we diff --git a/ipc/mscom/AgileReference.h b/ipc/mscom/AgileReference.h index fb2d6e16926f..e1ef9b3ed350 100644 --- a/ipc/mscom/AgileReference.h +++ b/ipc/mscom/AgileReference.h @@ -33,17 +33,9 @@ namespace mscom { * HRESULT hr = myAgileRef->Resolve(IID_IFoo, getter_AddRefs(foo)); * // Now foo may be called from the main thread */ -class AgileReference final +class AgileReference { public: - AgileReference(); - - template - explicit AgileReference(RefPtr& aObject) - : AgileReference(__uuidof(InterfaceT), aObject) - { - } - AgileReference(REFIID aIid, IUnknown* aObject); AgileReference(AgileReference&& aOther); @@ -54,31 +46,14 @@ public: return mAgileRef || mGitCookie; } - template - void Assign(const RefPtr& aOther) - { - Assign(__uuidof(T), aOther); - } - - template - AgileReference& operator=(const RefPtr& aOther) - { - Assign(aOther); - return *this; - } - - HRESULT Resolve(REFIID aIid, void** aOutInterface) const; + HRESULT Resolve(REFIID aIid, void** aOutInterface); AgileReference(const AgileReference& aOther) = delete; AgileReference& operator=(const AgileReference& aOther) = delete; - - AgileReference& operator=(AgileReference&& aOther); + AgileReference& operator=(AgileReference&& aOther) = delete; private: - void Assign(REFIID aIid, IUnknown* aObject); - void AssignInternal(IUnknown* aObject); - void Clear(); - static IGlobalInterfaceTable* ObtainGit(); + IGlobalInterfaceTable* ObtainGit(); private: IID mIid; @@ -89,26 +64,4 @@ private: } // namespace mscom } // namespace mozilla -template -RefPtr::RefPtr(const mozilla::mscom::AgileReference& aAgileRef) -{ - void* newRawPtr; - if (FAILED(aAgileRef.Resolve(__uuidof(T), &newRawPtr))) { - newRawPtr = nullptr; - } - mRawPtr = static_cast(newRawPtr); -} - -template -RefPtr& -RefPtr::operator=(const mozilla::mscom::AgileReference& aAgileRef) -{ - void* newRawPtr; - if (FAILED(aAgileRef.Resolve(__uuidof(T), &newRawPtr))) { - newRawPtr = nullptr; - } - assign_assuming_AddRef(static_cast(newRawPtr)); - return *this; -} - #endif // mozilla_mscom_AgileReference_h diff --git a/ipc/mscom/Utils.cpp b/ipc/mscom/Utils.cpp index 53da8540a528..6d4b32a4584d 100644 --- a/ipc/mscom/Utils.cpp +++ b/ipc/mscom/Utils.cpp @@ -32,15 +32,6 @@ extern "C" IMAGE_DOS_HEADER __ImageBase; namespace mozilla { namespace mscom { -bool -IsCOMInitializedOnCurrentThread() -{ - APTTYPE aptType; - APTTYPEQUALIFIER aptTypeQualifier; - HRESULT hr = CoGetApartmentType(&aptType, &aptTypeQualifier); - return hr != CO_E_NOTINITIALIZED; -} - bool IsCurrentThreadMTA() { diff --git a/ipc/mscom/Utils.h b/ipc/mscom/Utils.h index 5b6c0adfab18..b1ea77c77b8c 100644 --- a/ipc/mscom/Utils.h +++ b/ipc/mscom/Utils.h @@ -20,7 +20,6 @@ struct IUnknown; namespace mozilla { namespace mscom { -bool IsCOMInitializedOnCurrentThread(); bool IsCurrentThreadMTA(); bool IsProxy(IUnknown* aUnknown); bool IsValidGUID(REFGUID aCheckGuid); diff --git a/mfbt/RefPtr.h b/mfbt/RefPtr.h index e0b05f1a3e62..4ba2854f1487 100644 --- a/mfbt/RefPtr.h +++ b/mfbt/RefPtr.h @@ -22,11 +22,6 @@ class nsISupports; namespace mozilla { template class OwningNonNull; template class StaticRefPtr; -#if defined(XP_WIN) -namespace mscom { -class AgileReference; -} // namespace mscom -#endif // defined(XP_WIN) // Traditionally, RefPtr supports automatic refcounting of any pointer type // with AddRef() and Release() methods that follow the traditional semantics. @@ -157,9 +152,6 @@ public: MOZ_IMPLICIT RefPtr(const nsQueryReferent& aHelper); MOZ_IMPLICIT RefPtr(const nsCOMPtr_helper& aHelper); -#if defined(XP_WIN) - MOZ_IMPLICIT RefPtr(const mozilla::mscom::AgileReference& aAgileRef); -#endif // defined(XP_WIN) // Defined in OwningNonNull.h template @@ -223,9 +215,6 @@ public: RefPtr& operator=(const nsQueryReferent& aQueryReferent); RefPtr& operator=(const nsCOMPtr_helper& aHelper); -#if defined(XP_WIN) - RefPtr& operator=(const mozilla::mscom::AgileReference& aAgileRef); -#endif // defined(XP_WIN) RefPtr& operator=(RefPtr && aRefPtr) diff --git a/widget/windows/JumpListBuilder.cpp b/widget/windows/JumpListBuilder.cpp index 8fce9560f74f..1f66c4a97d2c 100644 --- a/widget/windows/JumpListBuilder.cpp +++ b/widget/windows/JumpListBuilder.cpp @@ -20,10 +20,8 @@ #include "nsThreadUtils.h" #include "mozilla/LazyIdleThread.h" #include "nsIObserverService.h" -#include "mozilla/ScopeExit.h" #include "mozilla/Unused.h" #include "mozilla/dom/Promise.h" -#include "mozilla/mscom/COMApartmentRegion.h" #include #include "WinUtils.h" @@ -109,22 +107,10 @@ JumpListBuilder::JumpListBuilder() : mHasCommit(false), mMonitor("JumpListBuilderMonitor") { - MOZ_ASSERT(NS_IsMainThread()); + ::CoInitialize(nullptr); - RefPtr jumpListMgr; - HRESULT hr = CoCreateInstance(CLSID_DestinationList, nullptr, - CLSCTX_INPROC_SERVER, IID_ICustomDestinationList, - getter_AddRefs(jumpListMgr)); - if (FAILED(hr)) { - return; - } - - // Since we are accessing mJumpListMgr across different threads - // (ie, different apartments), mJumpListMgr must be an agile reference. - mJumpListMgr = jumpListMgr; - if (!mJumpListMgr) { - return; - } + CoCreateInstance(CLSID_DestinationList, nullptr, CLSCTX_INPROC_SERVER, + IID_ICustomDestinationList, getter_AddRefs(mJumpListMgr)); // Make a lazy thread for any IO mIOThread = new LazyIdleThread(DEFAULT_THREAD_TIMEOUT_MS, @@ -143,6 +129,8 @@ JumpListBuilder::JumpListBuilder() : JumpListBuilder::~JumpListBuilder() { Preferences::RemoveObserver(this, kPrefTaskbarEnabled); + mJumpListMgr = nullptr; + ::CoUninitialize(); } NS_IMETHODIMP JumpListBuilder::GetAvailable(int16_t *aAvailable) @@ -176,19 +164,14 @@ NS_IMETHODIMP JumpListBuilder::GetMaxListItems(int16_t *aMaxItems) return NS_OK; } - RefPtr jumpListMgr = mJumpListMgr; - if (!jumpListMgr) { - return NS_ERROR_UNEXPECTED; - } - IObjectArray *objArray; - if (SUCCEEDED(jumpListMgr->BeginList(&mMaxItems, IID_PPV_ARGS(&objArray)))) { + if (SUCCEEDED(mJumpListMgr->BeginList(&mMaxItems, IID_PPV_ARGS(&objArray)))) { *aMaxItems = mMaxItems; if (objArray) objArray->Release(); - jumpListMgr->AbortList(); + mJumpListMgr->AbortList(); } return NS_OK; @@ -229,55 +212,36 @@ NS_IMETHODIMP JumpListBuilder::InitListBuild(JSContext* aCx, void JumpListBuilder::DoInitListBuild(RefPtr&& aPromise) { - // Since we're invoking COM interfaces to talk to the shell on a background - // thread, we need to be running inside a single-threaded apartment. - mscom::STARegion sta; - MOZ_ASSERT(sta.IsValid()); - ReentrantMonitorAutoEnter lock(mMonitor); MOZ_ASSERT(mJumpListMgr); - if (sBuildingList) { + if(sBuildingList) { AbortListBuild(); } - HRESULT hr = E_UNEXPECTED; - auto errorHandler = MakeScopeExit([&aPromise, &hr]() { - if (SUCCEEDED(hr)) { - return; - } - - NS_DispatchToMainThread(NS_NewRunnableFunction("InitListBuildReject", - [promise = std::move(aPromise)]() { - promise->MaybeReject(NS_ERROR_FAILURE); - })); - }); - - RefPtr jumpListMgr = mJumpListMgr; - if (!jumpListMgr) { - return; - } - nsTArray urisToRemove; RefPtr objArray; - hr = jumpListMgr->BeginList(&mMaxItems, - IID_PPV_ARGS(static_cast - (getter_AddRefs(objArray)))); - if (FAILED(hr)) { - return; - } - + HRESULT hr = mJumpListMgr->BeginList(&mMaxItems, + IID_PPV_ARGS(static_cast + (getter_AddRefs(objArray)))); // The returned objArray of removed items are for manually removed items. // This does not return items which are removed because they were previously // part of the jump list but are no longer part of the jump list. - sBuildingList = true; - RemoveIconCacheAndGetJumplistShortcutURIs(objArray, urisToRemove); + if (SUCCEEDED(hr)) { + sBuildingList = true; + RemoveIconCacheAndGetJumplistShortcutURIs(objArray, urisToRemove); - NS_DispatchToMainThread(NS_NewRunnableFunction("InitListBuildResolve", - [urisToRemove = std::move(urisToRemove), - promise = std::move(aPromise)]() { - promise->MaybeResolve(urisToRemove); - })); + NS_DispatchToMainThread(NS_NewRunnableFunction("InitListBuildResolve", + [urisToRemove = std::move(urisToRemove), + promise = std::move(aPromise)]() { + promise->MaybeResolve(urisToRemove); + })); + } else { + NS_DispatchToMainThread(NS_NewRunnableFunction("InitListBuildReject", + [promise = std::move(aPromise)]() { + promise->MaybeReject(NS_ERROR_FAILURE); + })); + } } // Ensures that we have no old ICO files left in the jump list cache @@ -330,11 +294,6 @@ NS_IMETHODIMP JumpListBuilder::AddListToBuild(int16_t aCatType, nsIArray *items, if (!mJumpListMgr) return NS_ERROR_NOT_AVAILABLE; - RefPtr jumpListMgr = mJumpListMgr; - if (!jumpListMgr) { - return NS_ERROR_UNEXPECTED; - } - switch(aCatType) { case nsIJumpListBuilder::JUMPLIST_CATEGORY_TASKS: { @@ -379,7 +338,7 @@ NS_IMETHODIMP JumpListBuilder::AddListToBuild(int16_t aCatType, nsIArray *items, return NS_ERROR_UNEXPECTED; // Add the tasks - hr = jumpListMgr->AddUserTasks(pArray); + hr = mJumpListMgr->AddUserTasks(pArray); if (SUCCEEDED(hr)) *_retval = true; return NS_OK; @@ -387,14 +346,14 @@ NS_IMETHODIMP JumpListBuilder::AddListToBuild(int16_t aCatType, nsIArray *items, break; case nsIJumpListBuilder::JUMPLIST_CATEGORY_RECENT: { - if (SUCCEEDED(jumpListMgr->AppendKnownCategory(KDC_RECENT))) + if (SUCCEEDED(mJumpListMgr->AppendKnownCategory(KDC_RECENT))) *_retval = true; return NS_OK; } break; case nsIJumpListBuilder::JUMPLIST_CATEGORY_FREQUENT: { - if (SUCCEEDED(jumpListMgr->AppendKnownCategory(KDC_FREQUENT))) + if (SUCCEEDED(mJumpListMgr->AppendKnownCategory(KDC_FREQUENT))) *_retval = true; return NS_OK; } @@ -461,7 +420,7 @@ NS_IMETHODIMP JumpListBuilder::AddListToBuild(int16_t aCatType, nsIArray *items, return NS_ERROR_UNEXPECTED; // Add the tasks - hr = jumpListMgr->AppendCategory(reinterpret_cast(catName.BeginReading()), pArray); + hr = mJumpListMgr->AppendCategory(reinterpret_cast(catName.BeginReading()), pArray); if (SUCCEEDED(hr)) *_retval = true; @@ -483,12 +442,7 @@ NS_IMETHODIMP JumpListBuilder::AbortListBuild() if (!mJumpListMgr) return NS_ERROR_NOT_AVAILABLE; - RefPtr jumpListMgr = mJumpListMgr; - if (!jumpListMgr) { - return NS_ERROR_UNEXPECTED; - } - - jumpListMgr->AbortList(); + mJumpListMgr->AbortList(); sBuildingList = false; return NS_OK; @@ -517,33 +471,20 @@ NS_IMETHODIMP JumpListBuilder::CommitListBuild(nsIJumpListCommittedCallback* aCa void JumpListBuilder::DoCommitListBuild(RefPtr aCallback) { - // Since we're invoking COM interfaces to talk to the shell on a background - // thread, we need to be running inside a single-threaded apartment. - mscom::STARegion sta; - MOZ_ASSERT(sta.IsValid()); - ReentrantMonitorAutoEnter lock(mMonitor); MOZ_ASSERT(mJumpListMgr); MOZ_ASSERT(aCallback); - HRESULT hr = E_UNEXPECTED; - auto onExit = MakeScopeExit([&hr, &aCallback]() { - // XXX We might want some specific error data here. - aCallback->SetResult(SUCCEEDED(hr)); - Unused << NS_DispatchToMainThread(aCallback); - }); - - RefPtr jumpListMgr = mJumpListMgr; - if (!jumpListMgr) { - return; - } - - hr = jumpListMgr->CommitList(); + HRESULT hr = mJumpListMgr->CommitList(); sBuildingList = false; if (SUCCEEDED(hr)) { mHasCommit = true; } + + // XXX We might want some specific error data here. + aCallback->SetResult(SUCCEEDED(hr)); + Unused << NS_DispatchToMainThread(aCallback); } NS_IMETHODIMP JumpListBuilder::DeleteActiveList(bool *_retval) @@ -554,22 +495,15 @@ NS_IMETHODIMP JumpListBuilder::DeleteActiveList(bool *_retval) if (!mJumpListMgr) return NS_ERROR_NOT_AVAILABLE; - if (sBuildingList) { + if(sBuildingList) AbortListBuild(); - } nsAutoString uid; if (!WinTaskbar::GetAppUserModelID(uid)) return NS_OK; - RefPtr jumpListMgr = mJumpListMgr; - if (!jumpListMgr) { - return NS_ERROR_UNEXPECTED; - } - - if (SUCCEEDED(jumpListMgr->DeleteList(uid.get()))) { + if (SUCCEEDED(mJumpListMgr->DeleteList(uid.get()))) *_retval = true; - } return NS_OK; } @@ -620,7 +554,7 @@ void JumpListBuilder::RemoveIconCacheAndGetJumplistShortcutURIs(IObjectArray *aO int32_t numArgs; arglist = ::CommandLineToArgvW(buf, &numArgs); - if (arglist && numArgs > 0) { + if(arglist && numArgs > 0) { nsString spec(arglist[0]); aURISpecs.AppendElement(std::move(spec)); ::LocalFree(arglist); diff --git a/widget/windows/JumpListBuilder.h b/widget/windows/JumpListBuilder.h index 064437e476f3..5140359a1cd8 100644 --- a/widget/windows/JumpListBuilder.h +++ b/widget/windows/JumpListBuilder.h @@ -23,7 +23,6 @@ #include "nsIObserver.h" #include "nsTArray.h" #include "mozilla/Attributes.h" -#include "mozilla/mscom/AgileReference.h" #include "mozilla/ReentrantMonitor.h" namespace mozilla { @@ -49,7 +48,7 @@ protected: static Atomic sBuildingList; private: - mscom::AgileReference mJumpListMgr; + RefPtr mJumpListMgr; uint32_t mMaxItems; bool mHasCommit; nsCOMPtr mIOThread;