Backed out changeset baed7a5596e1 (bug 1760662) for causing build bustages on WorkerPrivate.h CLOSED TREE

dom/serviceworkers/ServiceWorkerRegistrar.cpp

@@ -874,18 +874,13 @@ nsresult ServiceWorkerRegistrar::ReadData() {
       }
     }
   }
+
   // Overwrite previous version.
   // Cannot call SaveData directly because gtest uses main-thread.
-
-  // XXX NOTE: if we could be accessed multi-threaded here, we would need to
-  // find a way to lock around access to mData.  Since we can't, suppress the
-  // thread-safety warnings.
-  PUSH_IGNORE_THREAD_SAFETY
   if (overwrite && NS_FAILED(WriteData(mData))) {
     NS_WARNING("Failed to write data for the ServiceWorker Registations.");
     DeleteData();
   }
-  POP_THREAD_SAFETY
 
   return NS_OK;
 }

dom/serviceworkers/ServiceWorkerRegistrar.h

@@ -75,8 +75,8 @@ class ServiceWorkerRegistrar : public nsIObserver,
   nsresult WriteData(const nsTArray<ServiceWorkerRegistrationData>& aData);
   void DeleteData();
 
-  void RegisterServiceWorkerInternal(const ServiceWorkerRegistrationData& aData)
+  void RegisterServiceWorkerInternal(
-      REQUIRES(mMonitor);
+      const ServiceWorkerRegistrationData& aData);
 
   ServiceWorkerRegistrar();
   virtual ~ServiceWorkerRegistrar();
@@ -97,14 +97,12 @@ class ServiceWorkerRegistrar : public nsIObserver,
   bool IsSupportedVersion(const nsACString& aVersion) const;
 
  protected:
-  mozilla::Monitor mMonitor;
+  mozilla::Monitor mMonitor MOZ_UNANNOTATED;
 
   // protected by mMonitor.
-  nsCOMPtr<nsIFile> mProfileDir GUARDED_BY(mMonitor);
+  nsCOMPtr<nsIFile> mProfileDir;
-  // Read on mainthread, modified on background thread EXCEPT for
+  nsTArray<ServiceWorkerRegistrationData> mData;
-  // ReloadDataForTest() AND for gtest, which modifies this on MainThread.
+  bool mDataLoaded;
-  nsTArray<ServiceWorkerRegistrationData> mData GUARDED_BY(mMonitor);
-  bool mDataLoaded GUARDED_BY(mMonitor);
 
   // PBackground thread only
   uint32_t mDataGeneration;

dom/serviceworkers/test/gtest/TestReadWrite.cpp

@@ -37,9 +37,7 @@ class ServiceWorkerRegistrarTest : public ServiceWorkerRegistrar {
   }
 
   nsresult TestReadData() { return ReadData(); }
-  nsresult TestWriteData() NO_THREAD_SAFETY_ANALYSIS {
+  nsresult TestWriteData() { return WriteData(mData); }
-    return WriteData(mData);
-  }
   void TestDeleteData() { DeleteData(); }
 
   void TestRegisterServiceWorker(const ServiceWorkerRegistrationData& aData) {

dom/workers/JSExecutionManager.h

@@ -104,16 +104,16 @@ class JSExecutionManager {
 
   // Workers waiting to be given permission for execution.
   // Guarded by mExecutionQueueMutex.
-  std::deque<WorkerPrivate*> mExecutionQueue GUARDED_BY(mExecutionQueueMutex);
+  std::deque<WorkerPrivate*> mExecutionQueue;
 
   // Number of threads currently executing concurrently for this manager.
   // Guarded by mExecutionQueueMutex.
-  int32_t mRunning GUARDED_BY(mExecutionQueueMutex) = 0;
+  int32_t mRunning = 0;
 
   // Number of threads allowed to run concurrently for environments managed
   // by this manager.
   // Guarded by mExecutionQueueMutex.
-  int32_t mMaxRunning GUARDED_BY(mExecutionQueueMutex) = 1;
+  int32_t mMaxRunning = 1;
 
   // Mutex that guards the execution queue and associated state.
   Mutex mExecutionQueueMutex =
@@ -130,7 +130,7 @@ class JSExecutionManager {
   // Whether the main thread is currently awaiting permission to execute. Main
   // thread execution is always prioritized.
   // Guarded by mExecutionQueueMutex.
-  bool mMainThreadAwaitingExecution GUARDED_BY(mExecutionQueueMutex) = false;
+  bool mMainThreadAwaitingExecution = false;
 };
 
 // Helper for managing execution requests and allowing re-entrant permission

dom/workers/Queue.h

@@ -53,26 +53,26 @@ struct StorageWithTArray {
   static void Compact(StorageType& aStorage) { aStorage.Compact(); }
 };
 
-class CAPABILITY LockingWithMutex {
+class LockingWithMutex {
-  mozilla::Mutex mMutex;
+  mozilla::Mutex mMutex MOZ_UNANNOTATED;
 
  protected:
   LockingWithMutex() : mMutex("LockingWithMutex::mMutex") {}
 
-  void Lock() CAPABILITY_ACQUIRE() { mMutex.Lock(); }
+  PUSH_IGNORE_THREAD_SAFETY
+  void Lock() { mMutex.Lock(); }
 
-  void Unlock() CAPABILITY_RELEASE() { mMutex.Unlock(); }
+  void Unlock() { mMutex.Unlock(); }
 
-  class SCOPED_CAPABILITY AutoLock {
+  class AutoLock {
     LockingWithMutex& mHost;
 
    public:
-    explicit AutoLock(LockingWithMutex& aHost) CAPABILITY_ACQUIRE(aHost) : mHost(aHost) {
+    explicit AutoLock(LockingWithMutex& aHost) : mHost(aHost) { mHost.Lock(); }
-      mHost.Lock();
-    }
 
-    ~AutoLock() CAPABILITY_RELEASE() { mHost.Unlock(); }
+    ~AutoLock() { mHost.Unlock(); }
   };
+  POP_THREAD_SAFETY
 
   friend class AutoLock;
 };

dom/workers/RuntimeService.h

@@ -60,14 +60,13 @@ class RuntimeService final : public nsIObserver {
     mozilla::TimeStamp mExpirationTime;
   };
 
-  mozilla::Mutex mMutex;
+  mozilla::Mutex mMutex MOZ_UNANNOTATED;
 
   // Protected by mMutex.
-  nsClassHashtable<nsCStringHashKey, WorkerDomainInfo> mDomainMap
+  nsClassHashtable<nsCStringHashKey, WorkerDomainInfo> mDomainMap;
-      GUARDED_BY(mMutex);
 
   // Protected by mMutex.
-  nsTArray<IdleThreadInfo> mIdleThreadArray GUARDED_BY(mMutex);
+  nsTArray<IdleThreadInfo> mIdleThreadArray;
 
   // *Not* protected by mMutex.
   nsClassHashtable<nsPtrHashKey<const nsPIDOMWindowInner>,
@@ -195,8 +194,7 @@ class RuntimeService final : public nsIObserver {
 
   void Cleanup();
 
-  void AddAllTopLevelWorkersToArray(nsTArray<WorkerPrivate*>& aWorkers)
+  void AddAllTopLevelWorkersToArray(nsTArray<WorkerPrivate*>& aWorkers);
-      REQUIRES(mMutex);
 
   nsTArray<WorkerPrivate*> GetWorkersForWindow(
       const nsPIDOMWindowInner& aWindow) const;

dom/workers/WorkerCSPEventListener.h

@@ -30,10 +30,10 @@ class WorkerCSPEventListener final : public nsICSPEventListener {
   WorkerCSPEventListener();
   ~WorkerCSPEventListener() = default;
 
-  Mutex mMutex;
+  Mutex mMutex MOZ_UNANNOTATED;
 
   // Protected by mutex.
-  RefPtr<WeakWorkerRef> mWorkerRef GUARDED_BY(mMutex);
+  RefPtr<WeakWorkerRef> mWorkerRef;
 };
 
 }  // namespace dom

dom/workers/WorkerEventTarget.h

@@ -28,9 +28,9 @@ class WorkerEventTarget final : public nsISerialEventTarget {
   enum class Behavior : uint8_t { Hybrid, ControlOnly };
 
  private:
-  mozilla::Mutex mMutex;
+  mozilla::Mutex mMutex MOZ_UNANNOTATED;
-  CheckedUnsafePtr<WorkerPrivate> mWorkerPrivate GUARDED_BY(mMutex);
+  CheckedUnsafePtr<WorkerPrivate> mWorkerPrivate;
-  const Behavior mBehavior GUARDED_BY(mMutex);
+  const Behavior mBehavior;
 
   ~WorkerEventTarget() = default;
 

dom/workers/WorkerPrivate.cpp

@@ -656,9 +656,8 @@ class DebuggerImmediateRunnable : public WorkerRunnable {
   }
 };
 
-// GetJSContext() is safe on the worker thread
+void PeriodicGCTimerCallback(nsITimer* aTimer, void* aClosure) {
-void PeriodicGCTimerCallback(nsITimer* aTimer,
+  // GetJSContext() is safe on the worker thread
-                             void* aClosure) NO_THREAD_SAFETY_ANALYSIS {
   auto* workerPrivate = static_cast<WorkerPrivate*>(aClosure);
   MOZ_DIAGNOSTIC_ASSERT(workerPrivate);
   workerPrivate->AssertIsOnWorkerThread();
@@ -667,8 +666,7 @@ void PeriodicGCTimerCallback(nsITimer* aTimer,
                                         false /* collect children */);
 }
 
-void IdleGCTimerCallback(nsITimer* aTimer,
+void IdleGCTimerCallback(nsITimer* aTimer, void* aClosure) {
-                         void* aClosure) NO_THREAD_SAFETY_ANALYSIS {
   auto* workerPrivate = static_cast<WorkerPrivate*>(aClosure);
   MOZ_DIAGNOSTIC_ASSERT(workerPrivate);
   workerPrivate->AssertIsOnWorkerThread();
@@ -937,8 +935,8 @@ nsString ComputeWorkerPrivateId() {
 class WorkerPrivate::EventTarget final : public nsISerialEventTarget {
   // This mutex protects mWorkerPrivate and must be acquired *before* the
   // WorkerPrivate's mutex whenever they must both be held.
-  mozilla::Mutex mMutex;
+  mozilla::Mutex mMutex MOZ_UNANNOTATED;
-  WorkerPrivate* mWorkerPrivate GUARDED_BY(mMutex);
+  WorkerPrivate* mWorkerPrivate;
   nsIEventTarget* mWeakNestedEventTarget;
   nsCOMPtr<nsIEventTarget> mNestedEventTarget;
 
@@ -2975,6 +2973,8 @@ void WorkerPrivate::UnrootGlobalScopes() {
 
 void WorkerPrivate::DoRunLoop(JSContext* aCx) {
   auto data = mWorkerThreadAccessible.Access();
+  MOZ_ASSERT(mThread);
+
   MOZ_RELEASE_ASSERT(!GetExecutionManager());
 
   RefPtr<WorkerThread> thread;
@@ -3620,13 +3620,10 @@ void WorkerPrivate::ScheduleDeletion(WorkerRanOrNot aRanOrNot) {
   }
 }
 
-bool WorkerPrivate::CollectRuntimeStats(
+bool WorkerPrivate::CollectRuntimeStats(JS::RuntimeStats* aRtStats,
-    JS::RuntimeStats* aRtStats, bool aAnonymize) NO_THREAD_SAFETY_ANALYSIS {
+                                        bool aAnonymize) {
-  // We don't have a lock to access mJSContext, but it's safe to access on this
-  // thread.
   AssertIsOnWorkerThread();
   NS_ASSERTION(aRtStats, "Null RuntimeStats!");
-  // We don't really own it, but it's safe to access on this thread
   NS_ASSERTION(mJSContext, "This must never be null!");
 
   return JS::CollectRuntimeStats(mJSContext, aRtStats, nullptr, aAnonymize);
@@ -4054,8 +4051,6 @@ bool WorkerPrivate::RunCurrentSyncLoop() {
   {
     MutexAutoLock lock(mMutex);
     // Copy to local so we don't trigger mutex analysis lower down
-    // mThread is set before we enter, and is never changed during
-    // RunCurrentSyncLoop copy to local so we don't trigger mutex analysis
     thread = mThread;
   }
 

dom/workers/WorkerPrivate.h

@@ -84,10 +84,10 @@ class WorkerThread;
 // SharedMutex is a small wrapper around an (internal) reference-counted Mutex
 // object. It exists to avoid changing a lot of code to use Mutex* instead of
 // Mutex&.
-class CAPABILITY SharedMutex {
+class SharedMutex {
   using Mutex = mozilla::Mutex;
 
-  class CAPABILITY RefCountedMutex final : public Mutex {
+  class RefCountedMutex final : public Mutex {
    public:
     explicit RefCountedMutex(const char* aName) : Mutex(aName) {}
 
@@ -105,17 +105,11 @@ class CAPABILITY SharedMutex {
 
   SharedMutex(const SharedMutex& aOther) = default;
 
-  operator Mutex&() RETURN_CAPABILITY(this) { return *mMutex; }
+  operator Mutex&() RETURN_CAPABILITY(*mMutex) { return *mMutex; }
 
-  operator const Mutex&() const RETURN_CAPABILITY(this) { return *mMutex; }
+  operator const Mutex&() const RETURN_CAPABILITY(*mMutex) { return *mMutex; }
 
-  // We need these to make thread-safety analysis work
+  void AssertCurrentThreadOwns() const ASSERT_CAPABILITY(*mMutex) {
-  void Lock() CAPABILITY_ACQUIRE() { mMutex->Lock(); }
-  void Unlock() CAPABILITY_RELEASE() { mMutex->Unlock(); }
-
-  // We can assert we own 'this', but we can't assert we hold mMutex
-  void AssertCurrentThreadOwns() const
-      ASSERT_CAPABILITY(this) NO_THREAD_SAFETY_ANALYSIS {
     mMutex->AssertCurrentThreadOwns();
   }
 };
@@ -172,18 +166,18 @@ class WorkerPrivate final
 
   bool Cancel() { return Notify(Canceling); }
 
-  bool Close() REQUIRES(mMutex);
+  bool Close();
 
   // The passed principal must be the Worker principal in case of a
   // ServiceWorker and the loading principal for any other type.
   static void OverrideLoadInfoLoadGroup(WorkerLoadInfo& aLoadInfo,
                                         nsIPrincipal* aPrincipal);
 
-  bool IsDebuggerRegistered() NO_THREAD_SAFETY_ANALYSIS {
+  bool IsDebuggerRegistered() {
     AssertIsOnMainThread();
 
     // No need to lock here since this is only ever modified by the same thread.
-    return mDebuggerRegistered;  // would give a thread-safety warning
+    return mDebuggerRegistered;
   }
 
   bool ExtensionAPIAllowed() {
@@ -367,7 +361,7 @@ class WorkerPrivate final
     return mFetchHandlerWasAdded;
   }
 
-  JSContext* GetJSContext() const NO_THREAD_SAFETY_ANALYSIS {
+  JSContext* GetJSContext() const {
     // mJSContext is only modified on the worker thread, so workerthread code
     // can safely read it without a lock
     AssertIsOnWorkerThread();
@@ -545,7 +539,7 @@ class WorkerPrivate final
     return mParentStatus;
   }
 
-  WorkerStatus ParentStatus() const REQUIRES(mMutex) {
+  WorkerStatus ParentStatus() const {
     mMutex.AssertCurrentThreadOwns();
     return mParentStatus;
   }
@@ -1044,8 +1038,6 @@ class WorkerPrivate final
   void IncreaseWorkerFinishedRunnableCount() { ++mWorkerFinishedRunnableCount; }
   void DecreaseWorkerFinishedRunnableCount() { --mWorkerFinishedRunnableCount; }
 
-  Mutex& Mutex() RETURN_CAPABILITY(mMutex) { return mMutex; }
-
  private:
   WorkerPrivate(
       WorkerPrivate* aParent, const nsAString& aScriptURL, bool aIsChromeWorker,
@@ -1098,14 +1090,13 @@ class WorkerPrivate final
     return ProcessAllControlRunnablesLocked();
   }
 
-  ProcessAllControlRunnablesResult ProcessAllControlRunnablesLocked()
+  ProcessAllControlRunnablesResult ProcessAllControlRunnablesLocked();
-      REQUIRES(mMutex);
 
   void EnableMemoryReporter();
 
   void DisableMemoryReporter();
 
-  void WaitForWorkerEvents() REQUIRES(mMutex);
+  void WaitForWorkerEvents();
 
   // If the worker shutdown status is equal or greater then aFailStatus, this
   // operation will fail and nullptr will be returned. See WorkerStatus.h for
@@ -1142,7 +1133,7 @@ class WorkerPrivate final
   // to allow runnables to be atomically dispatched in bulk.
   nsresult DispatchLockHeld(already_AddRefed<WorkerRunnable> aRunnable,
                             nsIEventTarget* aSyncLoopTarget,
-                            const MutexAutoLock& aProofOfLock) REQUIRES(mMutex);
+                            const MutexAutoLock& aProofOfLock);
 
   // This method dispatches a simple runnable that starts the shutdown procedure
   // after a self.close(). This method is called after a ClearMainEventQueue()
@@ -1181,7 +1172,7 @@ class WorkerPrivate final
   friend class mozilla::dom::WorkerThread;
 
   SharedMutex mMutex;
-  mozilla::CondVar mCondVar GUARDED_BY(mMutex);
+  mozilla::CondVar mCondVar;
 
   // We cannot make this CheckedUnsafePtr<WorkerPrivate> as this would violate
   // our static assert
@@ -1215,18 +1206,18 @@ class WorkerPrivate final
   LocationInfo mLocationInfo;
 
   // Protected by mMutex.
-  workerinternals::JSSettings mJSSettings GUARDED_BY(mMutex);
+  workerinternals::JSSettings mJSSettings;
 
   WorkerDebugger* mDebugger;
 
   workerinternals::Queue<WorkerControlRunnable*, 4> mControlQueue;
   workerinternals::Queue<WorkerRunnable*, 4> mDebuggerQueue;
 
+  JSContext* mJSContext;
+  RefPtr<WorkerThread> mThread;
   // Touched on multiple threads, protected with mMutex. Only modified on the
   // worker thread
-  JSContext* mJSContext GUARDED_BY(mMutex);
   // mThread is only modified on the Worker thread, before calling DoRunLoop
-  RefPtr<WorkerThread> mThread GUARDED_BY(mMutex);
   // mPRThread is only modified on another thread in ScheduleWorker(), and is
   // constant for the duration of DoRunLoop.  Static mutex analysis doesn't help
   // here
@@ -1270,7 +1261,7 @@ class WorkerPrivate final
   RefPtr<WorkerCSPEventListener> mCSPEventListener;
 
   // Protected by mMutex.
-  nsTArray<RefPtr<WorkerRunnable>> mPreStartRunnables GUARDED_BY(mMutex);
+  nsTArray<RefPtr<WorkerRunnable>> mPreStartRunnables;
 
   // Only touched on the parent thread. This is set only if IsSharedWorker().
   RefPtr<RemoteWorkerChild> mRemoteWorkerController;
@@ -1280,8 +1271,8 @@ class WorkerPrivate final
 
   JS::UniqueChars mDefaultLocale;  // nulled during worker JSContext init
   TimeStamp mKillTime;
-  WorkerStatus mParentStatus GUARDED_BY(mMutex);
+  WorkerStatus mParentStatus;
-  WorkerStatus mStatus GUARDED_BY(mMutex);
+  WorkerStatus mStatus;
 
   // This is touched on parent thread only, but it can be read on a different
   // thread before crashing because hanging.
@@ -1414,7 +1405,7 @@ class WorkerPrivate final
   // use our global object's secure state there.
   const bool mIsSecureContext;
 
-  bool mDebuggerRegistered GUARDED_BY(mMutex);
+  bool mDebuggerRegistered;
 
   // During registration, this worker may be marked as not being ready to
   // execute debuggee runnables or content.