Properly suspend HTMLDocumentParser

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 98 matching lines @@
     , m_tokenizer(m_options.useThreading ? nullptr : HTMLTokenizer::create(m_options))
     , m_scriptRunner(HTMLScriptRunner::create(&document, this))
     , m_treeBuilder(HTMLTreeBuilder::create(this, &document, parserContentPolicy(), reportErrors, m_options))
     , m_parserScheduler(HTMLParserScheduler::create(this))
     , m_xssAuditorDelegate(&document)
     , m_weakFactory(this)
     , m_preloader(HTMLResourcePreloader::create(document))
     , m_isPinnedToMainThread(false)
     , m_endWasDelayed(false)
     , m_haveBackgroundParser(false)
+    , m_tasksWereSuspended(false)
     , m_pumpSessionNestingLevel(0)
 {
     ASSERT(shouldUseThreading() || (m_token && m_tokenizer));
 }
 
 // FIXME: Member variables should be grouped into self-initializing structs to
 // minimize code duplication between these constructors.
 HTMLDocumentParser::HTMLDocumentParser(DocumentFragment* fragment, Element* contextElement, ParserContentPolicy parserContentPolicy)
     : ScriptableDocumentParser(fragment->document(), parserContentPolicy)
     , m_options(&fragment->document())
     , m_token(adoptPtr(new HTMLToken))
     , m_tokenizer(HTMLTokenizer::create(m_options))
     , m_treeBuilder(HTMLTreeBuilder::create(this, fragment, contextElement, this->parserContentPolicy(), m_options))
     , m_xssAuditorDelegate(&fragment->document())
     , m_weakFactory(this)
     , m_isPinnedToMainThread(true)
     , m_endWasDelayed(false)
     , m_haveBackgroundParser(false)
+    , m_tasksWereSuspended(false)
     , m_pumpSessionNestingLevel(0)
 {
     ASSERT(!shouldUseThreading());
     bool reportErrors = false; // For now document fragment parsing never reports errors.
     m_tokenizer->setState(tokenizerStateForContextElement(contextElement, reportErrors, m_options));
     m_xssAuditor.initForFragment();
 }
 
 HTMLDocumentParser::~HTMLDocumentParser()
 {
@@ skipping 127 matching lines @@
 }
 
 // Used by HTMLParserScheduler
 void HTMLDocumentParser::resumeParsingAfterYield()
 {
     ASSERT(!m_isPinnedToMainThread);
     // pumpTokenizer can cause this parser to be detached from the Document,
     // but we need to ensure it isn't deleted yet.
     RefPtrWillBeRawPtr<HTMLDocumentParser> protect(this);
 
-    if (m_haveBackgroundParser) {
+    if (m_haveBackgroundParser || !m_speculations.isEmpty()) {

[João Eiras, 2014/10/06 12:14:30]

I'm a bit unsure about how these two variables relate, hence I modified this
check because m_speculations is being added the chunks when the parser is
suspended. If you tell me that m_speculations depend on m_haveBackgroundParser
being set to true, then I can revert this line.

[eseidel, 2014/10/06 18:07:08]

Speculations are parsed chunks sent to us from the background parser.  Unclear
how you could ever have speculations and not have a background parser...

[João Eiras, 2014/10/07 13:25:07]

OK, line reverted.

         pumpPendingSpeculations();
         return;
     }
 
     // We should never be here unless we can pump immediately.  Call pumpTokenizer()
     // directly so that ASSERTS will fire if we're wrong.
-    pumpTokenizer(AllowYield);
+    if (m_tokenizer)

[João Eiras, 2014/10/06 12:14:31]

As consequence of forcing a call to this method when resumeScheduledTasks is
called, m_tokenizer will be empty for document that have done all their parsing.
Instead of adding even more state to know if
didReceiveParsedChunkFromBackgroundParser was called or not, this is the
simplest way I found to avoid the assert inside pumpTokenizer().

[eseidel, 2014/10/06 18:07:08]

This seems wrong.  We should just be more careful as to when we call this
method.

[João Eiras, 2014/10/07 13:25:07]

Ok, added a couple extra checks in HTMLDocumentParser::resumeScheduledTasks()
then

+        pumpTokenizer(AllowYield);
     endIfDelayed();
 }
 
 void HTMLDocumentParser::runScriptsForPausedTreeBuilder()
 {
     ASSERT(scriptingContentIsAllowed(parserContentPolicy()));
 
     TextPosition scriptStartPosition = TextPosition::belowRangePosition();
     RefPtrWillBeRawPtr<Element> scriptElement = m_treeBuilder->takeScriptToProcess(scriptStartPosition);
     // We will not have a scriptRunner when parsing a DocumentFragment.
@@ skipping 40 matching lines @@
     return true;
 }
 
 void HTMLDocumentParser::didReceiveParsedChunkFromBackgroundParser(PassOwnPtr<ParsedChunk> chunk)
 {
     TRACE_EVENT0("blink", "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) {
+    if (isWaitingForScripts() || !m_speculations.isEmpty() || document()->activeParserCount() > 0 || m_tasksWereSuspended) {
         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.
     RefPtrWillBeRawPtr<HTMLDocumentParser> protect(this);
 
     ASSERT(m_speculations.isEmpty());
@@ skipping 659 matching lines @@
 {
     RefPtrWillBeRawPtr<HTMLDocumentParser> parser = HTMLDocumentParser::create(fragment, contextElement, parserContentPolicy);
     parser->insert(source); // Use insert() so that the parser will not yield.
     parser->finish();
     ASSERT(!parser->processingData()); // Make sure we're done. <rdar://problem/3963151>
     parser->detach(); // Allows ~DocumentParser to assert it was detached before destruction.
 }
 
 void HTMLDocumentParser::suspendScheduledTasks()
 {
+    ASSERT(!m_tasksWereSuspended);
+    m_tasksWereSuspended = true;
     if (m_parserScheduler)
         m_parserScheduler->suspend();
 }
 
 void HTMLDocumentParser::resumeScheduledTasks()
 {
+    ASSERT(m_tasksWereSuspended);
+    m_tasksWereSuspended = false;
     if (m_parserScheduler)
-        m_parserScheduler->resume();
+        m_parserScheduler->scheduleForResume();
 }
 
 void HTMLDocumentParser::appendBytes(const char* data, size_t length)
 {
     if (!length || isStopped())
         return;
 
     if (shouldUseThreading()) {
         if (!m_haveBackgroundParser)
             startBackgroundParser();
@@ skipping 24 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()));
 }
 
 }