Bug 1376760 - Fix race condition for ICustomDestinationList usage. r?jimm draft
authorWei-Cheng Pan <wpan@mozilla.com>
Thu, 06 Jul 2017 14:06:09 +0800
changeset 605909 40be010ed2cf1e4ad8af3d31d9e50c273f6a6b6f
parent 605863 a418121d46250f91728b86d9eea331029c264c30
child 636624 0409b40d82a99b0734c63dac1a25651da6c7422b
push id67549
push userbmo:wpan@mozilla.com
push dateMon, 10 Jul 2017 02:56:53 +0000
reviewersjimm
bugs1376760
milestone56.0a1
Bug 1376760 - Fix race condition for ICustomDestinationList usage. r?jimm MozReview-Commit-ID: fgZjrHqiek
widget/windows/JumpListBuilder.cpp
widget/windows/JumpListBuilder.h
--- a/widget/windows/JumpListBuilder.cpp
+++ b/widget/windows/JumpListBuilder.cpp
@@ -99,17 +99,18 @@ private:
 
 NS_IMPL_ISUPPORTS(DoneCommitListBuildCallback, nsIRunnable);
 
 } // namespace detail
 
 
 JumpListBuilder::JumpListBuilder() :
   mMaxItems(0),
-  mHasCommit(false)
+  mHasCommit(false),
+  mMonitor("JumpListBuilderMonitor")
 {
   ::CoInitialize(nullptr);
 
   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,
@@ -131,31 +132,33 @@ JumpListBuilder::~JumpListBuilder()
   mJumpListMgr = nullptr;
   ::CoUninitialize();
 }
 
 NS_IMETHODIMP JumpListBuilder::GetAvailable(int16_t *aAvailable)
 {
   *aAvailable = false;
 
+  ReentrantMonitorAutoEnter lock(mMonitor);
   if (mJumpListMgr)
     *aAvailable = true;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP JumpListBuilder::GetIsListCommitted(bool *aCommit)
 {
   *aCommit = mHasCommit;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP JumpListBuilder::GetMaxListItems(int16_t *aMaxItems)
 {
+  ReentrantMonitorAutoEnter lock(mMonitor);
   if (!mJumpListMgr)
     return NS_ERROR_NOT_AVAILABLE;
 
   *aMaxItems = 0;
 
   if (sBuildingList) {
     *aMaxItems = mMaxItems;
     return NS_OK;
@@ -175,16 +178,17 @@ NS_IMETHODIMP JumpListBuilder::GetMaxLis
 }
 
 NS_IMETHODIMP JumpListBuilder::InitListBuild(nsIMutableArray *removedItems, bool *_retval)
 {
   NS_ENSURE_ARG_POINTER(removedItems);
 
   *_retval = false;
 
+  ReentrantMonitorAutoEnter lock(mMonitor);
   if (!mJumpListMgr)
     return NS_ERROR_NOT_AVAILABLE;
 
   if(sBuildingList)
     AbortListBuild();
 
   IObjectArray *objArray;
 
@@ -305,16 +309,17 @@ nsresult JumpListBuilder::RemoveIconCach
 }
 
 NS_IMETHODIMP JumpListBuilder::AddListToBuild(int16_t aCatType, nsIArray *items, const nsAString &catName, bool *_retval)
 {
   nsresult rv;
 
   *_retval = false;
 
+  ReentrantMonitorAutoEnter lock(mMonitor);
   if (!mJumpListMgr)
     return NS_ERROR_NOT_AVAILABLE;
 
   switch(aCatType) {
     case nsIJumpListBuilder::JUMPLIST_CATEGORY_TASKS:
     {
       NS_ENSURE_ARG_POINTER(items);
 
@@ -452,27 +457,29 @@ NS_IMETHODIMP JumpListBuilder::AddListTo
     }
     break;
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP JumpListBuilder::AbortListBuild()
 {
+  ReentrantMonitorAutoEnter lock(mMonitor);
   if (!mJumpListMgr)
     return NS_ERROR_NOT_AVAILABLE;
 
   mJumpListMgr->AbortList();
   sBuildingList = false;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP JumpListBuilder::CommitListBuild(nsIJumpListCommittedCallback* aCallback)
 {
+  ReentrantMonitorAutoEnter lock(mMonitor);
   if (!mJumpListMgr)
     return NS_ERROR_NOT_AVAILABLE;
 
   // Also holds a strong reference to this to prevent use-after-free.
   RefPtr<detail::DoneCommitListBuildCallback> callback =
     new detail::DoneCommitListBuildCallback(aCallback, this);
 
   // The builder has a strong reference in the callback already, so we do not
@@ -483,16 +490,17 @@ NS_IMETHODIMP JumpListBuilder::CommitLis
        &JumpListBuilder::DoCommitListBuild, Move(callback));
   Unused << mIOThread->Dispatch(event, NS_DISPATCH_NORMAL);
 
   return NS_OK;
 }
 
 void JumpListBuilder::DoCommitListBuild(RefPtr<detail::DoneCommitListBuildCallback> aCallback)
 {
+  ReentrantMonitorAutoEnter lock(mMonitor);
   MOZ_ASSERT(mJumpListMgr);
   MOZ_ASSERT(aCallback);
 
   HRESULT hr = mJumpListMgr->CommitList();
   sBuildingList = false;
 
   if (SUCCEEDED(hr)) {
     mHasCommit = true;
@@ -502,16 +510,17 @@ void JumpListBuilder::DoCommitListBuild(
   aCallback->SetResult(SUCCEEDED(hr));
   Unused << NS_DispatchToMainThread(aCallback);
 }
 
 NS_IMETHODIMP JumpListBuilder::DeleteActiveList(bool *_retval)
 {
   *_retval = false;
 
+  ReentrantMonitorAutoEnter lock(mMonitor);
   if (!mJumpListMgr)
     return NS_ERROR_NOT_AVAILABLE;
 
   if(sBuildingList)
     AbortListBuild();
 
   nsAutoString uid;
   if (!WinTaskbar::GetAppUserModelID(uid))
--- a/widget/windows/JumpListBuilder.h
+++ b/widget/windows/JumpListBuilder.h
@@ -17,16 +17,17 @@
 #include "nsString.h"
 #include "nsIMutableArray.h"
 
 #include "nsIJumpListBuilder.h"
 #include "nsIJumpListItem.h"
 #include "JumpListItem.h"
 #include "nsIObserver.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/ReentrantMonitor.h"
 
 namespace mozilla {
 namespace widget {
 
 namespace detail {
 class DoneCommitListBuildCallback;
 } // namespace detail
 
@@ -45,16 +46,17 @@ public:
 protected:
   static Atomic<bool> sBuildingList;
 
 private:
   RefPtr<ICustomDestinationList> mJumpListMgr;
   uint32_t mMaxItems;
   bool mHasCommit;
   nsCOMPtr<nsIThread> mIOThread;
+  ReentrantMonitor mMonitor;
 
   bool IsSeparator(nsCOMPtr<nsIJumpListItem>& item);
   nsresult TransferIObjectArrayToIMutableArray(IObjectArray *objArray, nsIMutableArray *removedItems);
   nsresult RemoveIconCacheForItems(nsIMutableArray *removedItems);
   nsresult RemoveIconCacheForAllItems();
   void DoCommitListBuild(RefPtr<detail::DoneCommitListBuildCallback> aCallback);
 
   friend class WinTaskbar;