Bug 1411808. P2 - don't call mClient->IsSuspended() off the main thread in Update().
By mirroring the suspend status of the client, Update() is able to make
decisions on reading streams without calling mClient->IsSuspended().
MozReview-Commit-ID: G4gS2VGiMjj
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -26,30 +26,30 @@ namespace mozilla {
ChannelMediaResource::ChannelMediaResource(MediaResourceCallback* aCallback,
nsIChannel* aChannel,
nsIURI* aURI,
bool aIsPrivateBrowsing)
: BaseMediaResource(aCallback, aChannel, aURI)
, mReopenOnError(false)
, mCacheStream(this, aIsPrivateBrowsing)
- , mSuspendAgent(mChannel)
+ , mSuspendAgent(mChannel, mCacheStream)
{
}
ChannelMediaResource::ChannelMediaResource(
MediaResourceCallback* aCallback,
nsIChannel* aChannel,
nsIURI* aURI,
const MediaChannelStatistics& aStatistics)
: BaseMediaResource(aCallback, aChannel, aURI)
, mReopenOnError(false)
, mCacheStream(this, /* aIsPrivateBrowsing = */ false)
, mChannelStatistics(aStatistics)
- , mSuspendAgent(mChannel)
+ , mSuspendAgent(mChannel, mCacheStream)
{
}
ChannelMediaResource::~ChannelMediaResource()
{
MOZ_ASSERT(mClosed);
MOZ_ASSERT(!mChannel);
MOZ_ASSERT(!mListener);
@@ -599,18 +599,20 @@ ChannelMediaResource::CloneData(MediaRes
new ChannelMediaResource(aCallback, nullptr, mURI, mChannelStatistics);
// Initially the clone is treated as suspended by the cache, because
// we don't have a channel. If the cache needs to read data from the clone
// it will call CacheClientResume (or CacheClientSeek with aResume true)
// which will recreate the channel. This way, if all of the media data
// is already in the cache we don't create an unnecessary HTTP channel
// and perform a useless HTTP transaction.
+ resource->mCacheStream.InitAsClone(&mCacheStream);
+ // mSuspendAgent.Suspend() accesses mCacheStream which is not ready
+ // until InitAsClone() is done.
resource->mSuspendAgent.Suspend();
- resource->mCacheStream.InitAsClone(&mCacheStream);
resource->mChannelStatistics.Stop();
return resource.forget();
}
void ChannelMediaResource::CloseChannel()
{
NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
@@ -1015,17 +1017,21 @@ ChannelMediaResource::GetDebugInfo()
// ChannelSuspendAgent
bool
ChannelSuspendAgent::Suspend()
{
MOZ_ASSERT(NS_IsMainThread());
SuspendInternal();
- return (++mSuspendCount == 1);
+ if (++mSuspendCount == 1) {
+ mCacheStream.NotifyClientSuspended(true);
+ return true;
+ }
+ return false;
}
void
ChannelSuspendAgent::SuspendInternal()
{
MOZ_ASSERT(NS_IsMainThread());
if (mChannel) {
bool isPending = false;
@@ -1037,23 +1043,23 @@ ChannelSuspendAgent::SuspendInternal()
}
}
bool
ChannelSuspendAgent::Resume()
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(IsSuspended(), "Resume without suspend!");
- --mSuspendCount;
- if (mSuspendCount == 0) {
+ if (--mSuspendCount == 0) {
if (mChannel && mIsChannelSuspended) {
mChannel->Resume();
mIsChannelSuspended = false;
}
+ mCacheStream.NotifyClientSuspended(false);
return true;
}
return false;
}
void
ChannelSuspendAgent::UpdateSuspendedStatusIfNeeded()
{
--- a/dom/media/ChannelMediaResource.h
+++ b/dom/media/ChannelMediaResource.h
@@ -19,19 +19,19 @@ namespace mozilla {
/**
* This class is responsible for managing the suspend count and report suspend
* status of channel.
**/
class ChannelSuspendAgent
{
public:
- explicit ChannelSuspendAgent(nsIChannel* aChannel)
+ ChannelSuspendAgent(nsIChannel* aChannel, MediaCacheStream& aCacheStream)
: mChannel(aChannel)
- , mIsChannelSuspended(false)
+ , mCacheStream(aCacheStream)
{
}
// True when the channel has been suspended or needs to be suspended.
bool IsSuspended();
// Return true when the channel is logically suspended, i.e. the suspend
// count goes from 0 to 1.
@@ -50,18 +50,19 @@ public:
// Check whether we need to suspend the channel.
void UpdateSuspendedStatusIfNeeded();
private:
// Only suspends channel but not changes the suspend count.
void SuspendInternal();
nsIChannel* mChannel;
+ MediaCacheStream& mCacheStream;
uint32_t mSuspendCount = 0;
- bool mIsChannelSuspended;
+ bool mIsChannelSuspended = false;
};
/**
* This is the MediaResource implementation that wraps Necko channels.
* Much of its functionality is actually delegated to MediaCache via
* an underlying MediaCacheStream.
*
* All synchronization is performed by MediaCacheStream; all off-main-
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -1442,17 +1442,17 @@ MediaCache::Update()
enableReading = predictedNewDataUse < latestNextUse;
}
}
if (enableReading) {
for (uint32_t j = 0; j < i; ++j) {
MediaCacheStream* other = mStreams[j];
if (other->mResourceID == stream->mResourceID && !other->mClosed &&
- !other->mClient->IsSuspended() &&
+ !other->mClientSuspended &&
OffsetToBlockIndexUnchecked(other->mSeekTarget != -1
? other->mSeekTarget
: other->mChannelOffset) ==
OffsetToBlockIndexUnchecked(desiredOffset)) {
// This block is already going to be read by the other stream.
// So don't try to read it from this stream as well.
enableReading = false;
LOG("Stream %p waiting on same block (%" PRId32 ") from stream %p",
@@ -2144,16 +2144,34 @@ void
MediaCacheStream::NotifyChannelRecreated()
{
NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
mChannelEnded = false;
mDidNotifyDataEnded = false;
}
+void
+MediaCacheStream::NotifyClientSuspended(bool aSuspended)
+{
+ MOZ_ASSERT(NS_IsMainThread());
+
+ RefPtr<ChannelMediaResource> client = mClient;
+ nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
+ "MediaCacheStream::NotifyClientSuspended", [client, this, aSuspended]() {
+ if (mClientSuspended != aSuspended) {
+ mClientSuspended = aSuspended;
+ // mClientSuspended changes the decision of reading streams.
+ ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
+ mMediaCache->QueueUpdate();
+ }
+ });
+ OwnerThread()->Dispatch(r.forget());
+}
+
MediaCacheStream::~MediaCacheStream()
{
NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
NS_ASSERTION(!mPinCount, "Unbalanced Pin");
if (mMediaCache) {
NS_ASSERTION(mClosed, "Stream was not closed");
mMediaCache->ReleaseStream(this);
@@ -2671,16 +2689,19 @@ MediaCacheStream::Init(int64_t aContentL
void
MediaCacheStream::InitAsClone(MediaCacheStream* aOriginal)
{
MOZ_ASSERT(aOriginal->IsAvailableForSharing());
MOZ_ASSERT(!mMediaCache, "Has been initialized.");
MOZ_ASSERT(aOriginal->mMediaCache, "Don't clone an uninitialized stream.");
+ // This needs to be done before OpenStream() to avoid data race.
+ mClientSuspended = true;
+
// Use the same MediaCache as our clone.
mMediaCache = aOriginal->mMediaCache;
mMediaCache->OpenStream(this);
mResourceID = aOriginal->mResourceID;
// Grab cache blocks from aOriginal as readahead blocks for our stream
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -270,16 +270,20 @@ public:
void FlushPartialBlock();
// Notifies the cache that the channel has closed with the given status.
void NotifyDataEnded(nsresult aStatus);
// Notifies the stream that the channel is reopened. The stream should
// reset variables such as |mDidNotifyDataEnded|.
void NotifyChannelRecreated();
+ // Notifies the stream that the suspend status of the client has changed.
+ // Main thread only.
+ void NotifyClientSuspended(bool aSuspended);
+
// These methods can be called on any thread.
// Cached blocks associated with this stream will not be evicted
// while the stream is pinned.
void Pin();
void Unpin();
// See comments above for NotifyDataLength about how the length
// can vary over time. Returns -1 if no length is known. Returns the
// reported length if we haven't got any better information. If
@@ -513,13 +517,16 @@ private:
// Heap allocate this buffer since the exact power-of-2 will cause allocation
// slop when combined with the rest of the object members.
// This partial buffer should always be read/write within the cache's monitor.
const UniquePtr<uint8_t[]> mPartialBlockBuffer =
MakeUnique<uint8_t[]>(BLOCK_SIZE);
// True if associated with a private browsing window.
const bool mIsPrivateBrowsing;
+
+ // True if the client is suspended. Accessed on the owner thread only.
+ bool mClientSuspended = false;
};
} // namespace mozilla
#endif