Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(112)

Unified Diff: third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp

Issue 2351943003: Adapt ScriptStreamer to recent changes in v8 streaming. (Closed)
Patch Set: Marja's feedback. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp
index 2c5e18eb4b5b6060f1ca10e33ae50f05bd9914af..71b98e685c62a16b767f0f9ca7f30ef5c73d291a 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp
@@ -46,7 +46,7 @@ void recordStartedStreamingHistogram(ScriptStreamer::Type scriptType, int reason
break;
}
default:
- ASSERT_NOT_REACHED();
+ NOTREACHED();
break;
}
}
@@ -85,7 +85,7 @@ void recordNotStreamingReasonHistogram(ScriptStreamer::Type scriptType, NotStrea
break;
}
default:
- ASSERT_NOT_REACHED();
+ NOTREACHED();
break;
}
}
@@ -116,7 +116,7 @@ public:
void produce(const uint8_t* data, size_t length)
{
MutexLocker locker(m_mutex);
- ASSERT(!m_finished);
+ DCHECK(!m_finished);
m_data.append(std::make_pair(data, length));
m_haveData.signal();
}
@@ -180,8 +180,6 @@ public:
, m_finished(false)
, m_queueLeadPosition(0)
, m_queueTailPosition(0)
- , m_bookmarkPosition(0)
- , m_lengthOfBOM(0)
, m_loadingTaskRunner(loadingTaskRunner->clone())
{
}
@@ -192,7 +190,7 @@ public:
// some data.
size_t GetMoreData(const uint8_t** src) override
{
- ASSERT(!isMainThread());
+ DCHECK(!isMainThread());
{
MutexLocker locker(m_mutex);
if (m_cancelled)
@@ -210,53 +208,22 @@ public:
return length;
}
- // Called by V8 on background thread.
- bool SetBookmark() override
- {
- ASSERT(!isMainThread());
- m_bookmarkPosition = m_queueLeadPosition;
- return true;
- }
-
- // Called by V8 on background thread.
- void ResetToBookmark() override
- {
- ASSERT(!isMainThread());
- {
- MutexLocker locker(m_mutex);
- m_queueLeadPosition = m_bookmarkPosition;
- // See comments at m_lengthOfBOM declaration below for why
- // we need this here.
- m_queueTailPosition = m_bookmarkPosition + m_lengthOfBOM;
- m_dataQueue.clear();
- }
-
- // Inform main thread to re-queue the data.
- m_loadingTaskRunner->postTask(
- BLINK_FROM_HERE, crossThreadBind(&SourceStream::fetchDataFromResourceBuffer, crossThreadUnretained(this), 0));
- }
-
void didFinishLoading()
{
- ASSERT(isMainThread());
-
- // ResetToBookmark may reset the data queue's 'finished' status,
- // so we may need to re-finish after a ResetToBookmark happened.
- // We do this by remembering m_finished, and always checking for it
- // at the end ot fetchDataFromResourceBuffer.
+ DCHECK(isMainThread());
m_finished = true;
- fetchDataFromResourceBuffer(0);
+ m_dataQueue.finish();
}
- void didReceiveData(ScriptStreamer* streamer, size_t lengthOfBOM)
+ void didReceiveData(ScriptStreamer* streamer)
{
- ASSERT(isMainThread());
- prepareDataOnMainThread(streamer, lengthOfBOM);
+ DCHECK(isMainThread());
+ prepareDataOnMainThread(streamer);
}
void cancel()
{
- ASSERT(isMainThread());
+ DCHECK(isMainThread());
// The script is no longer needed by the upper layers. Stop streaming
// it. The next time GetMoreData is called (or woken up), it will return
// 0, which will be interpreted as EOS by V8 and the parsing will
@@ -270,17 +237,14 @@ public:
}
private:
- void prepareDataOnMainThread(ScriptStreamer* streamer, size_t lengthOfBOM)
+ void prepareDataOnMainThread(ScriptStreamer* streamer)
{
- ASSERT(isMainThread());
+ DCHECK(isMainThread());
// The Resource must still be alive; otherwise we should've cancelled
// the streaming (if we have cancelled, the background thread is not
// waiting).
- ASSERT(streamer->resource());
-
- // BOM can only occur at the beginning of the data.
- ASSERT(lengthOfBOM == 0 || m_queueTailPosition == 0);
+ DCHECK(streamer->resource());
if (!streamer->resource()->response().cacheStorageCacheName().isNull()) {
streamer->suppressStreaming();
@@ -304,19 +268,15 @@ private:
m_resourceBuffer = streamer->resource()->resourceBuffer();
}
- fetchDataFromResourceBuffer(lengthOfBOM);
+ fetchDataFromResourceBuffer();
}
- void fetchDataFromResourceBuffer(size_t lengthOfBOM)
+ void fetchDataFromResourceBuffer()
{
- ASSERT(isMainThread());
- MutexLocker locker(m_mutex); // For m_cancelled + m_queueTailPosition.
-
- if (lengthOfBOM > 0) {
- ASSERT(!m_lengthOfBOM); // There should be only one BOM.
- m_lengthOfBOM = lengthOfBOM;
- }
+ DCHECK(isMainThread());
+ MutexLocker locker(m_mutex);
+ DCHECK(!m_finished);
if (m_cancelled) {
m_dataQueue.finish();
return;
@@ -324,43 +284,15 @@ private:
// Get as much data from the ResourceBuffer as we can.
const char* data = nullptr;
- Vector<const char*> chunks;
- Vector<size_t> chunkLengths;
- size_t bufferLength = 0;
while (size_t length = m_resourceBuffer->getSomeData(data, m_queueTailPosition)) {
- // FIXME: Here we can limit based on the total length, if it turns
- // out that we don't want to give all the data we have (memory
- // vs. speed).
- chunks.append(data);
- chunkLengths.append(length);
- bufferLength += length;
- m_queueTailPosition += length;
- }
+ // Copy the data chunks into a new buffer, since we're going to
+ // give the data to a background thread.
+ uint8_t* copiedData = new uint8_t[length];
+ memcpy(copiedData, data, length);
+ m_dataQueue.produce(copiedData, length);
- // Copy the data chunks into a new buffer, since we're going to give the
- // data to a background thread.
- if (bufferLength > lengthOfBOM) {
- size_t totalLength = bufferLength - lengthOfBOM;
- uint8_t* copiedData = new uint8_t[totalLength];
- size_t offset = 0;
- size_t offsetInChunk = lengthOfBOM;
- for (size_t i = 0; i < chunks.size(); ++i) {
- if (offsetInChunk >= chunkLengths[i]) {
- offsetInChunk -= chunkLengths[i];
- continue;
- }
-
- size_t dataLength = chunkLengths[i] - offsetInChunk;
- memcpy(copiedData + offset, chunks[i] + offsetInChunk, dataLength);
- offset += dataLength;
- // BOM is in the beginning of the buffer.
- offsetInChunk = 0;
- }
- m_dataQueue.produce(copiedData, totalLength);
+ m_queueTailPosition += length;
}
-
- if (m_finished)
- m_dataQueue.finish();
}
// For coordinating between the main thread and background thread tasks.
@@ -381,23 +313,6 @@ private:
SourceStreamDataQueue m_dataQueue; // Thread safe.
size_t m_queueLeadPosition; // Only used by v8 thread.
size_t m_queueTailPosition; // Used by both threads; guarded by m_mutex.
- size_t m_bookmarkPosition; // Only used by v8 thread.
-
- // BOM (Unicode Byte Order Mark) handling:
- // This class is responsible for stripping out the BOM, since Chrome
- // delivers the input stream potentially with BOM, but V8 doesn't want
- // to see the BOM. This is mostly easy to do, except for a funky edge
- // condition with bookmarking:
- // - m_queueLeadPosition counts the bytes that V8 has received
- // (i.e., without BOM)
- // - m_queueTailPosition counts the bytes that Chrome has sent
- // (i.e., with BOM)
- // So when resetting the bookmark, we have to adjust the lead position
- // to account for the BOM (which happens implicitly in the regular
- // streaming case).
- // We store this separately, to avoid having to guard all
- // m_queueLeadPosition references with a mutex.
- size_t m_lengthOfBOM; // Used by both threads; guarded by m_mutex.
std::unique_ptr<WebTaskRunner> m_loadingTaskRunner;
};
@@ -443,7 +358,7 @@ bool ScriptStreamer::isFinished() const
void ScriptStreamer::streamingCompleteOnBackgroundThread()
{
- ASSERT(!isMainThread());
+ DCHECK(!isMainThread());
{
MutexLocker locker(m_mutex);
m_parsingFinished = true;
@@ -462,7 +377,7 @@ void ScriptStreamer::streamingCompleteOnBackgroundThread()
void ScriptStreamer::cancel()
{
- ASSERT(isMainThread());
+ DCHECK(isMainThread());
// The upper layer doesn't need the script any more, but streaming might
// still be ongoing. Tell SourceStream to try to cancel it whenever it gets
// the control the next time. It can also be that V8 has already completed
@@ -476,7 +391,7 @@ void ScriptStreamer::cancel()
void ScriptStreamer::suppressStreaming()
{
MutexLocker locker(m_mutex);
- ASSERT(!m_loadingFinished);
+ DCHECK(!m_loadingFinished);
// It can be that the parsing task has already finished (e.g., if there was
// a parse error).
m_streamingSuppressed = true;
@@ -484,49 +399,49 @@ void ScriptStreamer::suppressStreaming()
void ScriptStreamer::notifyAppendData(ScriptResource* resource)
{
- ASSERT(isMainThread());
- ASSERT(m_resource == resource);
+ DCHECK(isMainThread());
+ DCHECK_EQ(m_resource, resource);
{
MutexLocker locker(m_mutex);
if (m_streamingSuppressed)
return;
}
- size_t lengthOfBOM = 0;
if (!m_haveEnoughDataForStreaming) {
// Even if the first data chunk is small, the script can still be big
// enough - wait until the next data chunk comes before deciding whether
// to start the streaming.
- ASSERT(resource->resourceBuffer());
+ DCHECK(resource->resourceBuffer());
if (resource->resourceBuffer()->size() < s_smallScriptThreshold)
return;
m_haveEnoughDataForStreaming = true;
- // Encoding should be detected only when we have some data. It's
- // possible that resource->encoding() returns a different encoding
- // before the loading has started and after we got some data. In
- // addition, check for byte order marks. Note that checking the byte
- // order mark might change the encoding. We cannot decode the full text
- // here, because it might contain incomplete UTF-8 characters. Also note
- // that have at least s_smallScriptThreshold worth of data, which is more
- // than enough for detecting a BOM.
- constexpr size_t maximumLengthOfBOM = 4;
- char maybeBOM[maximumLengthOfBOM] = {};
- if (!resource->resourceBuffer()->getPartAsBytes(maybeBOM, static_cast<size_t>(0), maximumLengthOfBOM)) {
- NOTREACHED();
- return;
- }
-
- std::unique_ptr<TextResourceDecoder> decoder(TextResourceDecoder::create("application/javascript", resource->encoding()));
- lengthOfBOM = decoder->checkForBOM(maybeBOM, maximumLengthOfBOM);
+ {
+ // Check for BOM (byte order marks), because that might change our
+ // understanding of the data encoding.
+ constexpr size_t maximumLengthOfBOM = 4;
+ char maybeBOM[maximumLengthOfBOM] = {};
+ if (!resource->resourceBuffer()->getPartAsBytes(maybeBOM, static_cast<size_t>(0), maximumLengthOfBOM)) {
+ NOTREACHED();
+ return;
+ }
- // Maybe the encoding changed because we saw the BOM; get the encoding
- // from the decoder.
- if (!convertEncoding(decoder->encoding().name(), &m_encoding)) {
- suppressStreaming();
- recordNotStreamingReasonHistogram(m_scriptType, EncodingNotSupported);
- recordStartedStreamingHistogram(m_scriptType, 0);
- return;
+ std::unique_ptr<TextResourceDecoder> decoder(TextResourceDecoder::create("application/javascript", resource->encoding()));
+ decoder->checkForBOM(maybeBOM, maximumLengthOfBOM);
+
+ // The encoding may change when we see the BOM. Check for BOM now
+ // and update the encoding from the decoder when necessary. Supress
+ // streaming if the encoding is unsupported.
+ //
+ // Also note that have at least s_smallScriptThreshold worth of
+ // data, which is more than enough for detecting a BOM.
+ if (!convertEncoding(decoder->encoding().name(), &m_encoding)) {
+ suppressStreaming();
+ recordNotStreamingReasonHistogram(m_scriptType, EncodingNotSupported);
+ recordStartedStreamingHistogram(m_scriptType, 0);
+ return;
+ }
}
+
if (ScriptStreamerThread::shared()->isRunningTask()) {
// At the moment we only have one thread for running the tasks. A
// new task shouldn't be queued before the running task completes,
@@ -545,8 +460,8 @@ void ScriptStreamer::notifyAppendData(ScriptResource* resource)
return;
}
- ASSERT(!m_stream);
- ASSERT(!m_source);
+ DCHECK(!m_stream);
+ DCHECK(!m_source);
m_stream = new SourceStream(m_loadingTaskRunner.get());
// m_source takes ownership of m_stream.
m_source = wrapUnique(new v8::ScriptCompiler::StreamedSource(m_stream, m_encoding));
@@ -567,13 +482,13 @@ void ScriptStreamer::notifyAppendData(ScriptResource* resource)
recordStartedStreamingHistogram(m_scriptType, 1);
}
if (m_stream)
- m_stream->didReceiveData(this, lengthOfBOM);
+ m_stream->didReceiveData(this);
}
void ScriptStreamer::notifyFinished(Resource* resource)
{
- ASSERT(isMainThread());
- ASSERT(m_resource == resource);
+ DCHECK(isMainThread());
+ DCHECK_EQ(m_resource, resource);
// A special case: empty and small scripts. We didn't receive enough data to
// start the streaming before this notification. In that case, there won't
// be a "parsing complete" notification either, and we should not wait for
@@ -623,7 +538,7 @@ void ScriptStreamer::streamingComplete()
{
// The background task is completed; do the necessary ramp-down in the main
// thread.
- ASSERT(isMainThread());
+ DCHECK(isMainThread());
// It's possible that the corresponding Resource was deleted before V8
// finished streaming. In that case, the data or the notification is not
@@ -641,7 +556,7 @@ void ScriptStreamer::streamingComplete()
void ScriptStreamer::notifyFinishedToClient()
{
- ASSERT(isMainThread());
+ DCHECK(isMainThread());
// Usually, the loading will be finished first, and V8 will still need some
// time to catch up. But the other way is possible too: if V8 detects a
// parse error, the V8 side can complete before loading has finished. Send
@@ -658,8 +573,8 @@ void ScriptStreamer::notifyFinishedToClient()
bool ScriptStreamer::startStreamingInternal(PendingScript* script, Type scriptType, Settings* settings, ScriptState* scriptState, WebTaskRunner* loadingTaskRunner)
{
- ASSERT(isMainThread());
- ASSERT(scriptState->contextIsValid());
+ DCHECK(isMainThread());
+ DCHECK(scriptState->contextIsValid());
ScriptResource* resource = script->resource();
if (resource->isLoaded()) {
recordNotStreamingReasonHistogram(scriptType, AlreadyLoaded);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698