Bug 1415397 - use Span<> to replace low level pointer arithmetic in ReadFromCache().
MozReview-Commit-ID: HH6KXtMfSIJ
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -9,16 +9,17 @@
#include "ChannelMediaResource.h"
#include "FileBlockCache.h"
#include "MediaBlockCacheBase.h"
#include "MediaPrefs.h"
#include "MediaResource.h"
#include "MemoryBlockCache.h"
#include "mozilla/Attributes.h"
#include "mozilla/ClearOnShutdown.h"
+#include "mozilla/ErrorNames.h"
#include "mozilla/Logging.h"
#include "mozilla/Preferences.h"
#include "mozilla/ReentrantMonitor.h"
#include "mozilla/Services.h"
#include "mozilla/StaticPtr.h"
#include "mozilla/SystemGroup.h"
#include "mozilla/Telemetry.h"
#include "nsContentUtils.h"
@@ -30,19 +31,21 @@
#include "nsThreadUtils.h"
#include "prio.h"
#include <algorithm>
namespace mozilla {
#undef LOG
#undef LOGI
+#undef LOGE
LazyLogModule gMediaCacheLog("MediaCache");
#define LOG(...) MOZ_LOG(gMediaCacheLog, LogLevel::Debug, (__VA_ARGS__))
#define LOGI(...) MOZ_LOG(gMediaCacheLog, LogLevel::Info, (__VA_ARGS__))
+#define LOGE(...) NS_DebugBreak(NS_DEBUG_WARNING, nsPrintfCString(__VA_ARGS__).get(), nullptr, __FILE__, __LINE__)
// For HTTP seeking, if number of bytes needing to be
// seeked forward is less than this value then a read is
// done rather than a byte range request.
//
// If we assume a 100Mbit connection, and assume reissuing an HTTP seek causes
// a delay of 200ms, then in that 200ms we could have simply read ahead 2MB. So
// setting SEEK_VS_READ_THRESHOLD to 1MB sounds reasonable.
@@ -2457,16 +2460,82 @@ MediaCacheStream::ThrottleReadahead(bool
ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
if (mThrottleReadahead != bThrottle) {
LOGI("Stream %p ThrottleReadahead %d", this, bThrottle);
mThrottleReadahead = bThrottle;
mMediaCache->QueueUpdate();
}
}
+uint32_t
+MediaCacheStream::ReadPartialBlock(int64_t aOffset, Span<char> aBuffer)
+{
+ mMediaCache->GetReentrantMonitor().AssertCurrentThreadIn();
+ MOZ_ASSERT(IsOffsetAllowed(aOffset));
+
+ if (OffsetToBlockIndexUnchecked(mChannelOffset) !=
+ OffsetToBlockIndexUnchecked(aOffset) ||
+ aOffset >= mChannelOffset) {
+ // Not in the partial block or no data to read.
+ return 0;
+ }
+
+ auto source = MakeSpan<const uint8_t>(
+ mPartialBlockBuffer.get() + OffsetInBlock(aOffset),
+ OffsetInBlock(mChannelOffset) - OffsetInBlock(aOffset));
+ // We have |source.Length() <= BLOCK_SIZE < INT32_MAX| to guarantee
+ // that |bytesToRead| can fit into a uint32_t.
+ uint32_t bytesToRead = std::min(aBuffer.Length(), source.Length());
+ memcpy(aBuffer.Elements(), source.Elements(), bytesToRead);
+ return bytesToRead;
+}
+
+Result<uint32_t, nsresult>
+MediaCacheStream::ReadBlockFromCache(int64_t aOffset, Span<char> aBuffer)
+{
+ mMediaCache->GetReentrantMonitor().AssertCurrentThreadIn();
+ MOZ_ASSERT(IsOffsetAllowed(aOffset));
+
+ // OffsetToBlockIndexUnchecked() is always non-negative.
+ uint32_t index = OffsetToBlockIndexUnchecked(aOffset);
+ int32_t cacheBlock = index < mBlocks.Length() ? mBlocks[index] : -1;
+ if (cacheBlock < 0) {
+ // Not in the cache.
+ return 0;
+ }
+
+ if (aBuffer.Length() > size_t(BLOCK_SIZE)) {
+ // Clamp the buffer to avoid overflow below since we will read at most
+ // BLOCK_SIZE bytes.
+ aBuffer = aBuffer.First(BLOCK_SIZE);
+ }
+ // |BLOCK_SIZE - OffsetInBlock(aOffset)| <= BLOCK_SIZE
+ int32_t bytesToRead =
+ std::min<int32_t>(BLOCK_SIZE - OffsetInBlock(aOffset), aBuffer.Length());
+ int32_t bytesRead = 0;
+ nsresult rv =
+ mMediaCache->ReadCacheFile(cacheBlock * BLOCK_SIZE + OffsetInBlock(aOffset),
+ aBuffer.Elements(),
+ bytesToRead,
+ &bytesRead);
+
+ // Ensure |cacheBlock * BLOCK_SIZE + OffsetInBlock(aOffset)| won't overflow.
+ static_assert(INT64_MAX >= BLOCK_SIZE * (uint32_t(INT32_MAX) + 1),
+ "BLOCK_SIZE too large!");
+
+ if (NS_FAILED(rv)) {
+ nsCString name;
+ GetErrorName(rv, name);
+ LOGE("Stream %p ReadCacheFile failed, rv=%s", this, name.Data());
+ return mozilla::Err(rv);
+ }
+
+ return bytesRead;
+}
+
int64_t
MediaCacheStream::Tell()
{
ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
return mStreamOffset;
}
nsresult
@@ -2601,77 +2670,59 @@ MediaCacheStream::ReadAt(int64_t aOffset
ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
nsresult rv = Seek(nsISeekableStream::NS_SEEK_SET, aOffset);
if (NS_FAILED(rv)) return rv;
return Read(aBuffer, aCount, aBytes);
}
nsresult
-MediaCacheStream::ReadFromCache(char* aBuffer, int64_t aOffset, int64_t aCount)
+MediaCacheStream::ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount)
{
ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
+ // The buffer we are about to fill.
+ auto buffer = MakeSpan<char>(aBuffer, aCount);
+
// Read one block (or part of a block) at a time
- uint32_t count = 0;
int64_t streamOffset = aOffset;
- while (count < aCount) {
+ while (!buffer.IsEmpty()) {
if (mClosed) {
// We need to check |mClosed| in each iteration which might be changed
// after calling |mMediaCache->ReadCacheFile|.
return NS_ERROR_FAILURE;
}
- int32_t streamBlock = OffsetToBlockIndex(streamOffset);
- if (streamBlock < 0) {
- break;
- }
- uint32_t offsetInStreamBlock =
- uint32_t(streamOffset - streamBlock*BLOCK_SIZE);
- int64_t size = std::min<int64_t>(aCount - count, BLOCK_SIZE - offsetInStreamBlock);
- if (mStreamLength >= 0) {
- // Don't try to read beyond the end of the stream
- int64_t bytesRemaining = mStreamLength - streamOffset;
- if (bytesRemaining <= 0) {
- return NS_ERROR_FAILURE;
- }
- size = std::min(size, bytesRemaining);
- // Clamp size until 64-bit file size issues are fixed.
- size = std::min(size, int64_t(INT32_MAX));
+ if (!IsOffsetAllowed(streamOffset)) {
+ LOGE("Stream %p invalid offset=%" PRId64, this, streamOffset);
+ return NS_ERROR_ILLEGAL_VALUE;
+ }
+
+ Result<uint32_t, nsresult> rv = ReadBlockFromCache(streamOffset, buffer);
+ if (rv.isErr()) {
+ return rv.unwrapErr();
}
- int32_t bytes;
- int32_t channelBlock = OffsetToBlockIndexUnchecked(mChannelOffset);
- int32_t cacheBlock =
- size_t(streamBlock) < mBlocks.Length() ? mBlocks[streamBlock] : -1;
- if (channelBlock == streamBlock && streamOffset < mChannelOffset) {
- // We can just use the data in mPartialBlockBuffer. In fact we should
- // use it rather than waiting for the block to fill and land in
- // the cache.
- // Clamp bytes until 64-bit file size issues are fixed.
- int64_t toCopy = std::min<int64_t>(size, mChannelOffset - streamOffset);
- bytes = std::min(toCopy, int64_t(INT32_MAX));
- MOZ_ASSERT(bytes >= 0 && bytes <= toCopy, "Bytes out of range.");
- memcpy(aBuffer + count,
- mPartialBlockBuffer.get() + offsetInStreamBlock, bytes);
- } else {
- if (cacheBlock < 0) {
- // We expect all blocks to be cached! Fail!
- return NS_ERROR_FAILURE;
- }
- int64_t offset = cacheBlock*BLOCK_SIZE + offsetInStreamBlock;
- MOZ_ASSERT(size >= 0 && size <= INT32_MAX, "Size out of range.");
- nsresult rv = mMediaCache->ReadCacheFile(
- offset, aBuffer + count, int32_t(size), &bytes);
- if (NS_FAILED(rv)) {
- return rv;
- }
+ uint32_t bytes = rv.unwrap();
+ if (bytes > 0) {
+ // Read data from the cache successfully. Let's try next block.
+ streamOffset += bytes;
+ buffer = buffer.From(bytes);
+ continue;
}
- streamOffset += bytes;
- count += bytes;
+
+ // The partial block is our last chance to get data.
+ bytes = ReadPartialBlock(streamOffset, buffer);
+ if (bytes < buffer.Length()) {
+ // Not enough data to read.
+ return NS_ERROR_FAILURE;
+ }
+
+ // Return for we've got all the requested bytes.
+ return NS_OK;
}
return NS_OK;
}
nsresult
MediaCacheStream::Init(int64_t aContentLength)
{
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -3,16 +3,17 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef MediaCache_h_
#define MediaCache_h_
#include "Intervals.h"
+#include "mozilla/Result.h"
#include "mozilla/UniquePtr.h"
#include "nsCOMPtr.h"
#include "nsHashKeys.h"
#include "nsTArray.h"
#include "nsTHashtable.h"
class nsIEventTarget;
class nsIPrincipal;
@@ -309,19 +310,17 @@ public:
// growing. The stream should be pinned while this runs and while its results
// are used, to ensure no data is evicted.
nsresult GetCachedRanges(MediaByteRangeSet& aRanges);
// Reads from buffered data only. Will fail if not all data to be read is
// in the cache. Will not mark blocks as read. Can be called from the main
// thread. It's the caller's responsibility to wrap the call in a pin/unpin,
// and also to check that the range they want is cached before calling this.
- nsresult ReadFromCache(char* aBuffer,
- int64_t aOffset,
- int64_t aCount);
+ nsresult ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount);
// IsDataCachedToEndOfStream returns true if all the data from
// aOffset to the end of the stream (the server-reported end, if the
// real end is not known) is in cache. If we know nothing about the
// end of the stream, this returns false.
bool IsDataCachedToEndOfStream(int64_t aOffset);
// The mode is initially MODE_PLAYBACK.
void SetReadMode(ReadMode aMode);
@@ -421,16 +420,26 @@ private:
nsTHashtable<Entry> mEntries;
// The index of the first block in the list, or -1 if the list is empty.
int32_t mFirstBlock;
// The number of blocks in the list.
int32_t mCount;
};
+ // Read data from the partial block and return the number of bytes read
+ // successfully. 0 if aOffset is not an offset in the partial block or there
+ // is nothing to read.
+ uint32_t ReadPartialBlock(int64_t aOffset, Span<char> aBuffer);
+
+ // Read data from the cache block specified by aOffset. Return the number of
+ // bytes read successfully or an error code if any failure.
+ Result<uint32_t, nsresult> ReadBlockFromCache(int64_t aOffset,
+ Span<char> aBuffer);
+
// Returns the end of the bytes starting at the given offset
// which are in cache.
// This method assumes that the cache monitor is held and can be called on
// any thread.
int64_t GetCachedDataEndInternal(int64_t aOffset);
// Returns the offset of the first byte of cached data at or after aOffset,
// or -1 if there is no such cached data.
// This method assumes that the cache monitor is held and can be called on