NotifyDataEnded() runs off the main thread which might set mChannelEnded
wrongly after NotifyChannelRecreated reset it on the main thread.
We should reset the flags in NotifyDataStarted() which indicate a new load has begun.
MozReview-Commit-ID: Gi6PFXwMJqc
--HG--
extra : rebase_source : 85bb2c25a55cce4b3c3f023bf4c02fe5d1de7552
extra : source : 2f8c5518bf615f9190f87032568fc53037bc6fc1
There are some works to do when we allow a stream whose download ends abnormally
to continue sharing the resource:
1. Abort Read() when download error happens. We might still have a chance to
get all the data successfully. However, it doesn't really matter since
the stream data is incomplete and we will encounter decode errors sooner
or later.
2. Update() needs to check mChannelEnded since an ended stream will not
download data needed by other streams.
MozReview-Commit-ID: LGCecQ5rpzq
--HG--
extra : rebase_source : 17a91a1cfd145344c3c0a29b80665cb99ce20746
extra : source : 0947c12b035acc9fba02e89dc87b3a17f84cf2e5
Since NotifyDataEnded() run its code asynchronously, it is possible that a new
channel is created and NotifyDataStarted() is called before NotifyDataEndedInternal()
has a chance to run. We check the load ID to exit the function if necessary.
We also need to fix data races when running NotifyDataEndedInternal() off the
main thread in next patches.
MozReview-Commit-ID: IIAc7dxHike
--HG--
extra : rebase_source : 58e45f924058a986b8d86bfaeff2791ee8a5f4bc
extra : intermediate-source : b2a7fa7514723e214b8da40cfc0ec40b1de9a345
extra : source : 1ff93dc8f8c451b804133c780cedef2ee3d348e5
We will need to modify these members off the main thead while IsAvailableForSharing()
is a main thread only function.
InitAsClone() will return an error if the original stream ends abnormally.
MozReview-Commit-ID: 1qRyboca2YZ
--HG--
extra : rebase_source : 4617a911a1de052833bd0085b883a8ae4d639c7d
This will remove the need to retry reading for the callers.
Note since data is usually downloaded faster than being consumed, we don't
benefit much in reading data from a partial block in the memory. Chances are
we still need to wait for the block to commit to the cache so the reader can
continue. So we change the code to always read data from the cache or from
the last block when it is completed (reaching EOS).
This change allows up to somewhat optimize NotifyDataReceived() which won't
have to wake up readers if no blocks are committed to the cache.
MozReview-Commit-ID: KwgNSOawuAE
--HG--
extra : rebase_source : af29b9f5d8b7ee1ed41bda5d23e1e94209e323b7
extra : intermediate-source : 863cb113d20b9cc1222de001bacbefa7eb8ac5c1
extra : source : 09683d9ffe477c27164769dc93e9eb9ee0af21bd
So it doesn't need to call mCacheStream.IsTransportSeekable() which needs to
take the lock and might block the main thread.
MozReview-Commit-ID: 99QVcSxzjCz
--HG--
extra : rebase_source : be71b065ce0334987efbfb67a5cf010ab0373d80
extra : source : 2de3f0baf1475e8ae3228a33cf4cf139cf923c37
Per P2 changes, a reader will always read data from the cache or from the last
block in the memory. NotifyDataReceived() will be slightly more efficient
if we don't wake up readers unnecessarily when there are no new blocks committed
to the cache.
MozReview-Commit-ID: 3UWHbvtEUmu
--HG--
extra : rebase_source : d8c97d275ca5df7deb56447ef55092ac3d110e7f
extra : source : 8acc253fb322c4b6defb03bbc5489b5b1877f375
This will remove the need to retry reading for the callers.
Note since data is usually downloaded faster than being consumed, we don't
benefit much in reading data from a partial block in the memory. Chances are
we still need to wait for the block to commit to the cache so the reader can
continue. So we change the code to always read data from the cache or from
the last block when it is completed (reaching EOS).
This change allows up to somewhat optimize NotifyDataReceived() which won't
have to wake up readers if no blocks are committed to the cache.
MozReview-Commit-ID: KwgNSOawuAE
--HG--
extra : rebase_source : dcf61b2c43a7c030a0265979d75d18c63a3c41d0
extra : intermediate-source : 62e5895d3684b6fd00df7703156af5ea1f08bef3
extra : source : 09683d9ffe477c27164769dc93e9eb9ee0af21bd
Evicting the block will result in a gap in the current cached range starting
from mStreamOffset to mChannelOffset. Then we have
|GetCachedDataEndInternal(mStreamOffset) < mChannelOffset| and MediaCache
will open a new channel to fill the gap which is bad.
This is exactly the issue described in bug 1347174 where we limit the readahead
size to prevent the problem above from happening.
However bug 1347174 is indeed a workaround instead of a fix. It works around
the issue by suspending the download before the cache is full and has to evict
some blocks to allow new data to come in.
We should let MediaCache suspend the channel if it is full or evict played
blocks if possible.
MozReview-Commit-ID: HuUsZLdHGuh
--HG--
extra : rebase_source : cdd9bdb5bc63589550bafd49f6e313244037d8dd
extra : intermediate-source : 90aaf942dfbfdff98ef3df412ed49141f3f50e55
extra : source : fc8bc1c456677b92884c80ecfe4d5074a610f81a
We don't want MediaCache to use a half-initialized stream.
MozReview-Commit-ID: LjPLOYwy0Wd
--HG--
extra : rebase_source : a52a23fc6dce2a87ef2829254dcabe8176b2c28c
extra : intermediate-source : eb9c3d068ff4c3496831d80398012daebe8a5d52
extra : source : d980eb79f7a5219651ef710630cdf6dc3bd96195
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
--HG--
extra : rebase_source : bcdc1010fce47965c999df61666983c87e189670
extra : intermediate-source : 9dd8cfb80e29677e8cae866b2326dfb0aec5b6ae
extra : source : d20f640bf99478c9ba581e4979ec8091ef94e0f3
Note we will fix bugs required to run Update() off the main thread
in the following patches.
MozReview-Commit-ID: CYwT5kDjD9R
--HG--
extra : rebase_source : edef59353b5f56d5aecb6d58f5cc2a8dfb09c317
extra : intermediate-source : 995a34fb6af54d3f4ad2de479d7777860a957f98
extra : source : 8d86a14f877cf83584cf1e57020568037a35154a
MediaCache::Update() depends on mStreamOffset to make decision whether to
read streams or not. It is important to update mStreamOffset before releasing
the monitor so Update() won't make wrong decisions based on the stale value.
MozReview-Commit-ID: 40jUk5xd6GR
--HG--
extra : rebase_source : 40f4a64d0b66886b3a12c374f1bda874f8853750
extra : intermediate-source : 20ad1ad39a1e0c128ffd16e3fa669c9db1bb9c98
extra : source : 04f82ee89d93d5c008e51746c9686b4298e5f44f
This is a fix to P3.
Since seek is performed asynchronously by CacheClientSeek(), it is possible
for OnStopRequest() to come before Seek(). Changing mChannelOffset will
cause MediaCacheStream::NotifyDataEnded() to update mStreamLength incorrectly.
mChannelOffset should only be changed in response to channel activities such
as NotifyDataStarted() and NotifyDataReceived().
However, if MediaCache::Update() calls CacheClientSeek() without updating
mChannelOffset, next Update() might make a wrong decision and call CacheClientSeek()
again which is bad. So we add a member mSeekTarget to track if there is a pending
seek on which the stream reading decisions will be made.
MozReview-Commit-ID: VWP0vdlEYM
--HG--
extra : rebase_source : ea0d85bcbcc5d14f1554ebff3d10981a5b17e18a
extra : source : 339b9323b583849ac88e39da19670f6b26772877
For it is not safe to access mStreams without the lock off the main thread.
MozReview-Commit-ID: DjVlhxgjVj5
--HG--
extra : rebase_source : b584fe59712430acd4528e6b6cd01ae86dc5761f
extra : source : d7fd550934bfe6967638e42acb076882611792dd
SetTransportSeekable() is always called after NotifyDataStarted().
This is slightly more efficient for we don't acquire the lock twice.
MozReview-Commit-ID: 9myolomriIQ
--HG--
extra : rebase_source : f33c3be978edacf45d8144af43f45c8ad5e7b53e
extra : source : 2cefaeb1adae7238b77d5e2d1287ae0d96d9f671
To avoid leaks caused by Dispatch() failures. See comment 0 for the detail.
MozReview-Commit-ID: 3lYxQNj1GPl
--HG--
extra : rebase_source : 8df990476a49544b475df39be789e3cb27853609
extra : intermediate-source : 012f42a1a9d46b0dafa31ca03da1c2bc4fc76d2e
extra : source : c6acb9362de9ab8d130104aaf102de2ecb27dc8f
To avoid leaks caused by Dispatch() failures. See comment 0 for the detail.
MozReview-Commit-ID: 3lYxQNj1GPl
--HG--
extra : rebase_source : 3ea4958ba21e691f83dbdf36fd820e597d6c5d68
extra : intermediate-source : 012f42a1a9d46b0dafa31ca03da1c2bc4fc76d2e
extra : source : c6acb9362de9ab8d130104aaf102de2ecb27dc8f
It's only used in MediaCache.cpp, so it may as well be defined there.
MozReview-Commit-ID: HcA499xFOUg
--HG--
extra : rebase_source : f25c30d6e9be10549af80d5dfa963ef66abac576
So it is easier to run Update() loops off the main thread in the future.
MozReview-Commit-ID: LdxzQf6B3GK
--HG--
extra : rebase_source : 157984edf8ea08270fe61376e67183715b5bd4d4
extra : intermediate-source : f4045ce626977d392c799fae8f3d4f19efe3039f
extra : source : 778256b7055f4a470889eeae063660595d34337f
mStreamLength is always accessed within the lock. So it is safe to read/write
mStreamLength on all threads.
MozReview-Commit-ID: 9zJ2cwRrL5L
--HG--
extra : rebase_source : 10f282aa1c2fce2b9c0f431afb85e9d8ec7fab74
extra : intermediate-source : 38cac3d9015404aa3d1ddfd438ac57bd915fa0a7
extra : source : 60594740401732695f12f5f5232fa0f8e6681111
See comment 3 for the detail. We can't assert !mClosed since NotifyDataReceived()
could be called after the stream is closed.
MozReview-Commit-ID: 4pTfjABdl9B
--HG--
extra : rebase_source : d7d8b38268f3f54242dd728fe5fd0ada17d6ee48
extra : source : 713510f4087b38f0d447529dbf601f19b3a89eae
The load ID works as follows:
1. A load ID is passed to MediaCacheStream::NotifyDataStarted()
when loading a new channel.
2. Each MediaCacheStream::NotifyDataReceived() call is also associated
with a load ID from which the data is received.
3. If |mLoadID != aLoadID| tests to be true in NotifyDataReceived(), it means
the data is from an old channel and should be discarded.
4. MediaCache::Update() reset mLoadID for the stream before calling
CacheClientSeek() to prevent data from the old channel from being
stored to the wrong position.
MozReview-Commit-ID: 9kBoublLlln
--HG--
extra : rebase_source : 58e6d3fe40ec7a549cabc70b30db8006b49c0563
This fixes the data race when Seek() read mClosed off the main thread.
MozReview-Commit-ID: GO7Kk5VgVpg
--HG--
extra : rebase_source : e29353aea1e077e30fd284a80a56472d6772e9e1
extra : intermediate-source : 20a5860220a6eb54616cbe059afdaebc81e07e1f
extra : source : 0722d581e2d03eb140ea722527975534471c31b5
We have MediaCacheStream::mChannelOffset to keep the download positon.
We don't need 2 variables for the same purpose.
MozReview-Commit-ID: IpnEJWuA9A9
--HG--
extra : rebase_source : 8e720d878c12555d0a5528167c183ddb881b249e
extra : source : 623cf4cc3ab5ad0d9d263bac05a58699b3577277
We also make it return void since it now always succeeds.
MozReview-Commit-ID: H1oQWoguEzF
--HG--
extra : rebase_source : b5c6714832bed6fceb80c4afcdf4a590cc7dc567
extra : intermediate-source : 01aa9da848391bbf0b39f8dca874c0234f3202fb
extra : source : af04510d8603ffe407069ef342fdb4d3bca33509
When MediaCacheStream::NotifyDataReceived() runs off the main thread,
there is no guarantee that the principal will be updated before the new
data is observable to the consumer because the principal can only be
updated on the main thread while the consumer can access the data off
the main thread.
To make the code simpler, we always dispatch a task to run UpdatePrincipal()
even when CacheClientUpdatePrincipal() already runs in the main thread.
This also avoid the deadlock because ChannelMediaResource::UpdatePrincipal()
will never run with the cache monitor held.
MozReview-Commit-ID: 9CdrJnaV0hl
--HG--
extra : rebase_source : 128d54f4583199e7bfa8c72895583ab7fb668706
extra : intermediate-source : c2310f99bdc7529f1e1c67edbb8274b20b679cb2
extra : source : b6cc234d83e7b18ab69502af78d27ce5eda3b350
1. mCacheStream.Close() should happen after CloseChannel() to avoid data race
in reading mClosed in MediaCacheStream::NotifyDataReceived().
2. MediaCache::Update() and CloseStreamsForPrivateBrowsing() should call
ChannelMediaResource::Close() to ensure mCacheStream.Close() happens
after CloseChannel().
MozReview-Commit-ID: 1o3yPbm3Gy6
--HG--
extra : rebase_source : 0a39af9ae228bdf4098ad16793bb6eccd15c3ec7
extra : source : f4b6deb231be5915dc42318ec22d850d20562b0e
This is a workaround to fix bug 687972. It should be OK now to remove
it since we have fix bug 1108960 where MediaCache failed to run the
update loop to update the suspend status of the stream.
MozReview-Commit-ID: MbInehhScs
--HG--
extra : rebase_source : 94345a00af31834db8b9858cdf5a9e889044156a
extra : source : 70f626894c3a15c8077f70425a97004478caa9e1
Retrieve pref from MediaPrefs, which is more efficient than
Preferences::GetInt().
Also refactored computations to avoid unnecessary type conversions.
MozReview-Commit-ID: Ii27lthRRNI
--HG--
extra : rebase_source : d1ea46060cd2c35b7fd07a191184c0318187c080
Bug 1374173 seems to be the cause of all MediaCache intermittent assertion
failures. It's not an important bug, so let's back-out and verify that
intermittents disappear.
MozReview-Commit-ID: 2X8iW1hWu99
--HG--
extra : rebase_source : e22443a5b9d5ba9a6ce4b126953a880e8e469cd2
So its code can be simplified by removing the code doing other operations.
MozReview-Commit-ID: 32g7lp2XLNE
--HG--
extra : rebase_source : 23846b37c73a9dd2b5770e86254f4a07f21def53
Now that MediaCache doesn't use the content length to decide which block cache
to use, and can know it's the file-backed MediaCache (to reset the pointer,
and for telemetry purposes), we don't need to store mContentLength anymore.
MozReview-Commit-ID: KjxarKFe9WK
--HG--
extra : rebase_source : e2cb397c6d5e37a8390479f4245255ef52265483
MemoryBlockCache won't allow initializing, or growing an existing buffer,
above the limit (min of 'media.memory_caches_combined_limit_kb' or
sysmem*'media.memory_caches_combined_limit_pc_sysmem'/100).
MozReview-Commit-ID: 6MkwFp2eeth
--HG--
extra : rebase_source : 17345f6fe9f00fddfbef87090665afccaabb2cf5
This allows a fallback to the file-backed MediaCache, if a MemoryBlockCache
could not be created and initialized (which may happen in the next patch,
where MemoryBlockCache will take care of not using more than
MediaMemoryCachesCombinedLimit).
MediaCache::Init() is not needed anymore, as its only work was to initialize
its block cache.
MozReview-Commit-ID: ItAdOPuxEvt
--HG--
extra : rebase_source : 08461d61b8d738edb8c2088bca4e33213b8ae4e1
This saves from destruction&re-construction efforts, makes the flushing less
prone to first-initialization failures.
And it will allow moving the choice of block cache outside of MediaCache::Init.
MozReview-Commit-ID: 8vSunM3rRkL
--HG--
extra : rebase_source : d244c9ff0cb34f9b2171e5f5848501cc1d71d2bc
MediaCacheStreams have owning shared pointers to their MediaCache, and
a MediaCache owns itself while an update is in flight.
A non-owning pointer `gMediaCache` is only used by GetMediaCache and
~MediaCache to manage the one file-backed MediaCache.
MozReview-Commit-ID: AQHuXWGrKt6
--HG--
extra : rebase_source : f256e20080b8701f87418209aa42c5a0fe3f5239
The only external use of Close was always followed by an implicit destruction
(by resetting the RefPtr), so we don't need to expose it, and it can be done
from the destructor.
FileBlockCache keeps its Close() function for internal use.
Also, FileBlockCache::mIsOpen is redundant, as it's true iff mThread is not
null.
MozReview-Commit-ID: LV7YVrwJvGG
--HG--
extra : rebase_source : 23decadf249b9e63190b3e19d81edc4a090afcef
Don't go over the lowest of 'media.memory_caches_combined_limit_kb'
(kilobytes) or 'media.memory_caches_combined_limit_pc_sysmem' (percents of
system memory).
Added more logging around creation/destruction of MediaCaches.
MozReview-Commit-ID: Cdz4ycyn1RR
--HG--
extra : rebase_source : 63168234f186c3ef9c0289a189a647d67d8526a4
Because blocks may not necessarily be held in files anymore.
MozReview-Commit-ID: 2GNc7B5w2Jt
--HG--
extra : rebase_source : 4ceda80ca6736b159d8b726cdcfb8d7f74cf8529
The initial telemetry collection was done in NotifyDataLength() because that
was the first point where the length was introduced; but some extra code was
needed to ensure that were collecting the first length.
Now that this initial length is passed directly to Init(), we can report that
number instead.
In the "worst" case, it will actually be a bit more correct about what we
initially wanted to report, i.e., the initial length given by the HTTP
response header; and it's what we really want to know, now that we are using
this number to make a decision about which MediaCache to use.
MozReview-Commit-ID: 11Th8pensZt
--HG--
extra : rebase_source : 97a6d2dcbfad6c9b37819bfe6471baff2ec7e335
This will give enough information (for now) for GetMediaCache to decide whether
to use the (one global shared) file-backed MediaCache, or a discrete memory-
backed MediaCache.
(Note that GetMediaCache doesn't use this length yet in this patch.)
MozReview-Commit-ID: 5B2E3sIsc4k
--HG--
extra : rebase_source : 940e782665bf2c3640bbe7389fca02ea7c1482cd
JW Wang
Sebastian Hengst
Narcis Beleuzu
Chris Pearce
Phil Ringnalda
Andrea Marchesini
Bevis Tseng
Gerald Squelart
Jean-Yves Avenard