bug 1406831 don't tolerate just owning the monitor if AssertOnGraphThreadOrNotRunning() is not called on the correct thread r=pehrsons

Owning the monitor is not sufficient for consistent state if state can be
accessed without the monitor.

The requirements for SetCurrentDriver() are tighted to require both the
monitor and correct thread, as CurrentDriver() can be called either with the
monitor or on the graph thread.

MozReview-Commit-ID: 90q7Pfa8jxn

--HG--
extra : rebase_source : 6cbcc334dc2bd355d2e9afdebda45a9624edda4b

dom/media/MediaStreamGraph.cpp

@@ -1115,21 +1115,17 @@ MediaStreamGraph::NotifyOutputData(AudioDataValue* aBuffer, size_t aFrames,
   }
 }
 
-void
-MediaStreamGraph::AssertOnGraphThreadOrNotRunning() const
+bool
+MediaStreamGraph::OnGraphThreadOrNotRunning() const
 {
   // either we're on the right thread (and calling CurrentDriver() is safe),
-  // or we're going to assert anyways, so don't cross-check CurrentDriver
-#ifdef DEBUG
+  // or we're going to fail the assert anyway, so don't cross-check
+  // via CurrentDriver().
   MediaStreamGraphImpl const * graph =
     static_cast<MediaStreamGraphImpl const *>(this);
   // if all the safety checks fail, assert we own the monitor
-  if (!(graph->mDetectedNotRunning ?
-        NS_IsMainThread() : graph->mDriver->OnThread()))
-  {
-      graph->mMonitor.AssertCurrentThreadOwns();
-  }
-#endif
+  return graph->mDetectedNotRunning ?
+    NS_IsMainThread() : graph->mDriver->OnThread();
 }
 
 bool

dom/media/MediaStreamGraph.h

@@ -1367,7 +1367,10 @@ public:
   void NotifyOutputData(AudioDataValue* aBuffer, size_t aFrames,
                         TrackRate aRate, uint32_t aChannels);
 
-  void AssertOnGraphThreadOrNotRunning() const;
+  void AssertOnGraphThreadOrNotRunning() const
+  {
+    MOZ_ASSERT(OnGraphThreadOrNotRunning());
+  }
 
 protected:
   explicit MediaStreamGraph(TrackRate aSampleRate)
@@ -1380,6 +1383,10 @@ protected:
     MOZ_COUNT_DTOR(MediaStreamGraph);
   }
 
+  // Intended only for assertions, either on graph thread, not running, or
+  // with monitor held.
+  bool OnGraphThreadOrNotRunning() const;
+
   // Media graph thread only
   nsTArray<nsCOMPtr<nsIRunnable> > mPendingUpdateRunnables;
 

dom/media/MediaStreamGraphImpl.h

@@ -456,7 +456,11 @@ public:
    */
   GraphDriver* CurrentDriver() const
   {
-    AssertOnGraphThreadOrNotRunning();
+#ifdef DEBUG
+    if (!OnGraphThreadOrNotRunning()) {
+      mMonitor.AssertCurrentThreadOwns();
+    }
+#endif
     return mDriver;
   }
 
@@ -475,7 +479,10 @@ public:
    */
   void SetCurrentDriver(GraphDriver* aDriver)
   {
+#ifdef DEBUG
+    mMonitor.AssertCurrentThreadOwns();
     AssertOnGraphThreadOrNotRunning();
+#endif
     mDriver = aDriver;
   }