http://searchfox.org/mozilla-central/rev/7e090b227f7a0ec44d4ded604823d48823158c51/dom/media/ChannelMediaResource.cpp#387-396 Calling GetLength() and GetOffset() will need to acquire the cache lock and might block the main thread. We will move the code to the owner thread of MediaCache so we won't block the main thread.

Comment on attachment 8929325 [details] Bug 1415090. P1 - always move the channel back to the foreground when OnStopRequest() is fired. https://reviewboard.mozilla.org/r/200642/#review205826

Comment on attachment 8929326 [details] Bug 1415090. P2 - move the "reopen on error" code from ChannelMediaResource::OnStopRequest() to MediaCacheStream::NotifyDataEnded(). https://reviewboard.mozilla.org/r/200644/#review205838

Comment on attachment 8929329 [details] Bug 1415090. P5 - remove MediaCacheStream::NotifyChannelRecreated(). https://reviewboard.mozilla.org/r/200650/#review205854

Comment on attachment 8929327 [details] Bug 1415090. P3 - run MediaCacheStream::NotifyDataEnded() off the main thread. https://reviewboard.mozilla.org/r/200646/#review205874

Comment on attachment 8929328 [details] Bug 1415090. P4 - don't modify mResourceID off the main thread. https://reviewboard.mozilla.org/r/200648/#review205876 ::: dom/media/MediaCache.cpp (Diff revision 1) > - if (NS_FAILED(aStatus)) { > - // Disconnect from other streams sharing our resource, since they > - // should continue trying to load. Our load might have been deliberately > - // canceled and that shouldn't affect other streams. > - mResourceID = mMediaCache->AllocateResourceID(); > - } Why remove AllocateResourceID()? Is the code useless?

Comment on attachment 8929328 [details] Bug 1415090. P4 - don't modify mResourceID off the main thread. https://reviewboard.mozilla.org/r/200648/#review205876 > Why remove AllocateResourceID()? Is the code useless? mResourceID can only be modified on the main thread while NotifyDataEndedInternal() runs off the main thread. As the commit message said, this patch is about safely making a stream whose download ends abnormally to continue sharing the resource.

Comment on attachment 8929328 [details] Bug 1415090. P4 - don't modify mResourceID off the main thread. https://reviewboard.mozilla.org/r/200648/#review205878 Is is possible to merge the MediaCacheStream::mResourceID and MediaCacheStream::mLoadID?

Comment on attachment 8929328 [details] Bug 1415090. P4 - don't modify mResourceID off the main thread. https://reviewboard.mozilla.org/r/200648/#review205878 No. They are totally different ideas. mResourceID: MediaCacheStreams with the same resource id will share the same resource. That is, when one MediaCacheStream commit a block to the cache, this block will be also visible to others streams sharing the same resource id. mLoadID: Used to identify different channels within a single MediaCacheStream. Since NotifyDataStarted() runs on the main thread while NotifyDataReceived() runs off the main thread, it is possible for NotifyDataStarted() from the new channel to run before NotifyDataReceived() from the old channel. mLoadID is used to check whether the callbacks are from the old channel and should be aborted.

Comment on attachment 8929325 [details] Bug 1415090. P1 - always move the channel back to the foreground when OnStopRequest() is fired. https://reviewboard.mozilla.org/r/200642/#review206148

Comment on attachment 8929326 [details] Bug 1415090. P2 - move the "reopen on error" code from ChannelMediaResource::OnStopRequest() to MediaCacheStream::NotifyDataEnded(). https://reviewboard.mozilla.org/r/200644/#review206150

Comment on attachment 8929327 [details] Bug 1415090. P3 - run MediaCacheStream::NotifyDataEnded() off the main thread. https://reviewboard.mozilla.org/r/200646/#review206152

Comment on attachment 8929328 [details] Bug 1415090. P4 - don't modify mResourceID off the main thread. https://reviewboard.mozilla.org/r/200648/#review206154

Comment on attachment 8929329 [details] Bug 1415090. P5 - remove MediaCacheStream::NotifyChannelRecreated(). https://reviewboard.mozilla.org/r/200650/#review206156

Thanks for the reviews!

Pushed by jwwang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/846d1bec4f36 P1 - always move the channel back to the foreground when OnStopRequest() is fired. r=bechen,gerald https://hg.mozilla.org/integration/autoland/rev/94287653dc31 P2 - move the "reopen on error" code from ChannelMediaResource::OnStopRequest() to MediaCacheStream::NotifyDataEnded(). r=bechen,gerald https://hg.mozilla.org/integration/autoland/rev/d712d60574b6 P3 - run MediaCacheStream::NotifyDataEnded() off the main thread. r=bechen,gerald https://hg.mozilla.org/integration/autoland/rev/222597bfd33b P4 - don't modify mResourceID off the main thread. r=bechen,gerald https://hg.mozilla.org/integration/autoland/rev/fdb2a41005f2 P5 - remove MediaCacheStream::NotifyChannelRecreated(). r=bechen,gerald

https://hg.mozilla.org/mozilla-central/rev/846d1bec4f36 https://hg.mozilla.org/mozilla-central/rev/94287653dc31 https://hg.mozilla.org/mozilla-central/rev/d712d60574b6 https://hg.mozilla.org/mozilla-central/rev/222597bfd33b https://hg.mozilla.org/mozilla-central/rev/fdb2a41005f2