HTMLDocumentParser: Dedupe member inits by moving them into in-class init

third_party/WebKit/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 71 matching lines @@
         || contextTag.matches(noframesTag))
         return reportErrors ? HTMLTokenizer::RAWTEXTState : HTMLTokenizer::PLAINTEXTState;
     if (contextTag.matches(scriptTag))
         return reportErrors ? HTMLTokenizer::ScriptDataState : HTMLTokenizer::PLAINTEXTState;
     if (contextTag.matches(plaintextTag))
         return HTMLTokenizer::PLAINTEXTState;
     return HTMLTokenizer::DataState;
 }
 
 HTMLDocumentParser::HTMLDocumentParser(HTMLDocument& document, ParserSynchronizationPolicy syncPolicy)
-    : ScriptableDocumentParser(document)
+    : HTMLDocumentParser(document, AllowScriptingContent, syncPolicy)
+{
+    m_treeBuilder = HTMLTreeBuilder::create(this, &document, AllowScriptingContent, m_options);

[Yoav Weiss, 2016/07/05 08:20:14]

Doesn't this mean that we're creating m_treeBuilder twice?

[kouhei (in TOK), 2016/07/06 00:49:54]

Yes. This does mean that m_treeBuilder Member smartptr is created twice, but
this should be cheap. (HTMLTreeBuilder itself is only created once.)
We plan to improve the code further in separate refactor CLs.

+}
+
+HTMLDocumentParser::HTMLDocumentParser(DocumentFragment* fragment, Element* contextElement, ParserContentPolicy parserContentPolicy)

[Yoav Weiss, 2016/07/05 08:20:14]

Not critical, but It would've been easier to review if you'd have kept this
constructor in its old location.

[kouhei (in TOK), 2016/07/06 00:49:54]

I think we should have delegated ctor def either at the top or the bottom. I put
this bottom in this case, as this was declared last.

+    : HTMLDocumentParser(fragment->document(), parserContentPolicy, ForceSynchronousParsing)
+{
+    m_treeBuilder = HTMLTreeBuilder::create(this, fragment, contextElement, parserContentPolicy, m_options);
+
+    bool reportErrors = false; // For now document fragment parsing never reports errors.
+    m_tokenizer->setState(tokenizerStateForContextElement(contextElement, reportErrors, m_options));
+    m_xssAuditor.initForFragment();
+}
+
+HTMLDocumentParser::HTMLDocumentParser(Document& document, ParserContentPolicy contentPolicy, ParserSynchronizationPolicy syncPolicy)
+    : ScriptableDocumentParser(document, contentPolicy)
     , m_options(&document)
     , m_token(syncPolicy == ForceSynchronousParsing ? wrapUnique(new HTMLToken) : nullptr)
     , m_tokenizer(syncPolicy == ForceSynchronousParsing ? HTMLTokenizer::create(m_options) : nullptr)
-    , m_scriptRunner(HTMLScriptRunner::create(&document, this))
-    , m_treeBuilder(HTMLTreeBuilder::create(this, &document, getParserContentPolicy(), m_options))
+    , m_scriptRunner(scriptingContentIsAllowed(contentPolicy) ? HTMLScriptRunner::create(&document, this) : nullptr)

[tzik, 2016/07/05 07:59:09]

# As we chatted offline.
On old code, m_scriptRunner was not created on DocumentFragment* mode and it is
created on new code. Is it intended?

     , m_loadingTaskRunner(wrapUnique(document.loadingTaskRunner()->clone()))
-    , m_parserScheduler(HTMLParserScheduler::create(this, m_loadingTaskRunner.get()))
+    , m_parserScheduler(syncPolicy == AllowAsynchronousParsing ? HTMLParserScheduler::create(this, m_loadingTaskRunner.get()) : nullptr)
     , m_xssAuditorDelegate(&document)
     , m_weakFactory(this)
     , m_preloader(HTMLResourcePreloader::create(document))
     , m_parsedChunkQueue(ParsedChunkQueue::create())
     , m_evaluator(DocumentWriteEvaluator::create(document))
     , m_shouldUseThreading(syncPolicy == AllowAsynchronousParsing)
     , m_endWasDelayed(false)
     , m_haveBackgroundParser(false)
     , m_tasksWereSuspended(false)
     , m_pumpSessionNestingLevel(0)
     , m_pumpSpeculationsSessionNestingLevel(0)
     , m_isParsingAtLineNumber(false)
     , m_triedLoadingLinkHeaders(false)
 {
     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(wrapUnique(new HTMLToken))
-    , m_tokenizer(HTMLTokenizer::create(m_options))
-    , m_treeBuilder(HTMLTreeBuilder::create(this, fragment, contextElement, this->getParserContentPolicy(), m_options))
-    , m_loadingTaskRunner(wrapUnique(fragment->document().loadingTaskRunner()->clone()))
-    , m_xssAuditorDelegate(&fragment->document())
-    , m_weakFactory(this)
-    , m_shouldUseThreading(false)
-    , m_endWasDelayed(false)
-    , m_haveBackgroundParser(false)
-    , m_tasksWereSuspended(false)
-    , m_pumpSessionNestingLevel(0)
-    , m_pumpSpeculationsSessionNestingLevel(0)
-{
-    bool reportErrors = false; // For now document fragment parsing never reports errors.
-    m_tokenizer->setState(tokenizerStateForContextElement(contextElement, reportErrors, m_options));
-    m_xssAuditor.initForFragment();
-}
-
 HTMLDocumentParser::~HTMLDocumentParser()
 {
     // In Oilpan, HTMLDocumentParser can die together with Document, and
     // detach() is not called in this case.
     if (m_haveBackgroundParser)
         stopBackgroundParser();
 }
 
 DEFINE_TRACE(HTMLDocumentParser)
 {
@@ skipping 996 matching lines @@
         (*closure)();
         return;
     case Asynchronous:
         m_loadingTaskRunner->postTask(BLINK_FROM_HERE, std::move(closure));
         return;
     }
     NOTREACHED();
 }
 
 } // namespace blink