Make the HTMLDocumentParser yield more aggressively

Source/core/html/parser/HTMLDocumentParser.cpp

 /*
  * Copyright (C) 2010 Google, Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 307 matching lines @@
     if (mode == AllowYield)
         m_parserScheduler->checkForYieldBeforeToken(session);
 
     return true;
 }
 
 void HTMLDocumentParser::didReceiveParsedChunkFromBackgroundParser(PassOwnPtr<ParsedChunk> chunk)
 {
     TRACE_EVENT0("webkit", "HTMLDocumentParser::didReceiveParsedChunkFromBackgroundParser");
 
-    // alert(), runModalDialog, and the JavaScript Debugger all run nested event loops
-    // which can cause this method to be re-entered. We detect re-entry using
-    // hasActiveParser(), save the chunk as a speculation, and return.
-    if (isWaitingForScripts() || !m_speculations.isEmpty() || document()->activeParserCount() > 0) {
-        m_preloader->takeAndPreload(chunk->preloads);
-        m_speculations.append(chunk);
-        return;
-    }
-
-    // processParsedChunkFromBackgroundParser can cause this parser to be detached from the Document,
-    // but we need to ensure it isn't deleted yet.
-    RefPtr<HTMLDocumentParser> protect(this);
-
-    ASSERT(m_speculations.isEmpty());
-    chunk->preloads.clear(); // We don't need to preload because we're going to parse immediately.
+    m_preloader->takeAndPreload(chunk->preloads);
     m_speculations.append(chunk);

[abarth-chromium, 2014/04/29 18:29:00]

Should we rename m_speculations?  I'm not sure that's exactly the right name.

IMHO, we'll eventually want to integrate this logic with a global scheduler that
understands whether we're trying to rAF every 16ms or we're in a bulk processing
phase.

-    pumpPendingSpeculations();
+    // If we're already paused waiting for scripts, no need to schedule a resume.
+    if (!isWaitingForScripts())
+        m_parserScheduler->scheduleForResume();
 }
 
 void HTMLDocumentParser::didReceiveEncodingDataFromBackgroundParser(const DocumentEncodingData& data)
 {
     document()->setEncodingData(data);
 }
 
 void HTMLDocumentParser::validateSpeculations(PassOwnPtr<ParsedChunk> chunk)
 {
     ASSERT(chunk);
@@ skipping 74 matching lines @@
 
     HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::startedChunkWithCheckpoint, m_backgroundParser, chunk->inputCheckpoint));
 
     for (XSSInfoStream::const_iterator it = chunk->xssInfos.begin(); it != chunk->xssInfos.end(); ++it) {
         m_textPosition = (*it)->m_textPosition;
         m_xssAuditorDelegate.didBlockScript(**it);
         if (isStopped())
             break;
     }
 
+    // FIXME: Currently this always consumes an entire chunk at once thus the time
+    // between deadline checks is controlled by BackgroundHTMLParser.cpp's pendingTokenLimit.
+    // We could make this code able to yield within a chunk, which would separate the
+    // question of how much the tokenizer should batch into a chunk from how long
+    // the main thread should spend treebuilding before yielding.
     for (Vector<CompactHTMLToken>::const_iterator it = tokens->begin(); it != tokens->end(); ++it) {
         ASSERT(!isWaitingForScripts());
 
         if (document()->frame() && document()->frame()->navigationScheduler().locationChangePending()) {
 
             // To match main-thread parser behavior (which never checks locationChangePending on the EOF path)
             // we peek to see if this chunk has an EOF and process it anyway.
             if (tokens->last().type() == HTMLToken::EndOfFile) {
                 ASSERT(m_speculations.isEmpty()); // There should never be any chunks after the EOF.
                 prepareToStopParsing();
@@ skipping 27 matching lines @@
     }
 
     // Make sure any pending text nodes are emitted before returning.
     if (!isStopped())
         m_treeBuilder->flush();
 }
 
 void HTMLDocumentParser::pumpPendingSpeculations()
 {
     // FIXME: Share this constant with the parser scheduler.
-    const double parserTimeLimit = 0.500;
+    // FIXME: This could be much larger during intial load where it may be OK to hog the main thread.
+    const double parserTimeLimit = 0.008;

[abarth-chromium, 2014/04/29 18:29:00]

Again, this seems like a value the scheduler might want to tune dynamically.

 
     // ASSERT that this object is both attached to the Document and protected.
     ASSERT(refCount() >= 2);
     // If this assert fails, you need to call validateSpeculations to make sure
     // m_tokenizer and m_token don't have state that invalidates m_speculations.
     ASSERT(!m_tokenizer);
     ASSERT(!m_token);
     ASSERT(!m_lastChunkBeforeScript);
     ASSERT(!isWaitingForScripts());
     ASSERT(!isStopped());
@@ skipping 538 matching lines @@
 void HTMLDocumentParser::setDecoder(PassOwnPtr<TextResourceDecoder> decoder)
 {
     ASSERT(decoder);
     DecodedDataDocumentParser::setDecoder(decoder);
 
     if (m_haveBackgroundParser)
         HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::setDecoder, m_backgroundParser, takeDecoder()));
 }
 
 }