Script streaming: fix small script recognition heuristic.

Source/bindings/core/v8/ScriptStreamer.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "bindings/core/v8/ScriptStreamer.h"
 
 #include "bindings/core/v8/ScriptStreamerThread.h"
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "core/dom/Document.h"
@@ skipping 255 matching lines @@
     ASSERT(!m_loadingFinished);
     m_streamingSuppressed = true;
 }
 
 void ScriptStreamer::notifyAppendData(ScriptResource* resource)
 {
     ASSERT(isMainThread());
     ASSERT(m_resource == resource);
     if (m_streamingSuppressed)
         return;
-    if (!m_firstDataChunkReceived) {
-        m_firstDataChunkReceived = true;
-        const char* histogramName = startedStreamingHistogramName(m_scriptType);
-        // Check the size of the first data chunk. The expectation is that if
-        // the first chunk is small, there won't be a second one. In those
-        // cases, it doesn't make sense to stream at all.
+    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.
         if (resource->resourceBuffer()->size() < kSmallScriptThreshold) {

[haraken, 2014/10/22 14:20:39]

What happens if the kSmallScriptThreshold is 1 KB and you receive a 10 MB script
in a sequence of 500 byte chunks? Then m_haveEnoughDataForStreaming won't become
true forever.

I guess you need to count the accumulated size of received data and compare the
accumulated size with kSmallScriptThreshold.

[haraken, 2014/10/22 14:25:27]

Discussed offline. resource->resourceBuffer()->size() is the accumulated size,
and thus the CL looks great.

-            suppressStreaming();
-            blink::Platform::current()->histogramEnumeration(histogramName, 0, 2);
             return;
         }
+        m_haveEnoughDataForStreaming = true;
+        const char* histogramName = startedStreamingHistogramName(m_scriptType);
         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,
             // because the running task can block and wait for data from the
-            // network. At the moment we are only streaming parser blocking
-            // scripts, but this code can still be hit when multiple frames are
-            // loading simultaneously.
+            // network.
             suppressStreaming();
             blink::Platform::current()->histogramEnumeration(histogramName, 0, 2);
             return;
         }
         ASSERT(m_task);
         // ScriptStreamer needs to stay alive as long as the background task is
         // running. This is taken care of with a manual ref() & deref() pair;
         // the corresponding deref() is in streamingComplete.
         ref();
         ScriptStreamingTask* task = new ScriptStreamingTask(m_task.release(), this);
         ScriptStreamerThread::shared()->postTask(task);
         blink::Platform::current()->histogramEnumeration(histogramName, 1, 2);
     }
     m_stream->didReceiveData();
 }
 
 void ScriptStreamer::notifyFinished(Resource* resource)
 {
     ASSERT(isMainThread());
     ASSERT(m_resource == resource);
-    // A special case: empty scripts. We didn't receive any data before this
-    // notification. In that case, there won't be a "parsing complete"
-    // notification either, and we should not wait for it.
-    if (!m_firstDataChunkReceived)
+    // 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
+    // it.
+    if (!m_haveEnoughDataForStreaming) {
+        const char* histogramName = startedStreamingHistogramName(m_scriptType);
+        blink::Platform::current()->histogramEnumeration(histogramName, 0, 2);
         suppressStreaming();
+    }
     m_stream->didFinishLoading();
     m_loadingFinished = true;
     notifyFinishedToClient();
 }
 
 ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::StreamedSource::Encoding encoding, PendingScript::Type scriptType)
     : m_resource(resource)
     , m_detached(false)
     , m_stream(new SourceStream(this))
     , m_source(m_stream, encoding) // m_source takes ownership of m_stream.
     , m_client(0)
     , m_loadingFinished(false)
     , m_parsingFinished(false)
-    , m_firstDataChunkReceived(false)
+    , m_haveEnoughDataForStreaming(false)
     , m_streamingSuppressed(false)
     , m_scriptType(scriptType)
 {
 }
 
 void ScriptStreamer::notifyFinishedToClient()
 {
     ASSERT(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
@@ skipping 84 matching lines @@
     if (scriptStreamingTask) {
         streamer->m_task = adoptPtr(scriptStreamingTask);
         script.setStreamer(streamer.release());
         return true;
     }
     // Otherwise, V8 cannot stream the script.
     return false;
 }
 
 } // namespace blink