From c6f3d36fe2fbc47e708672f9be4d9525effa445f Mon Sep 17 00:00:00 2001
From: Elliott Sprehn <esprehn@chromium.org>
Date: Tue, 9 Dec 2014 19:01:46 -0800
Subject: [PATCH] Simplify HTMLStyleElement.

The parser will never insert more than one child into a <style>, so we don't
need explicit logic to batch up parsing. Once I removed that it exposed that
all the line number and parser created logic is from error reporting in the
parser that's been dead in Blink for over a year.

By doing this simplification I was able to remove the finishParsingChildren()
callback entirely.

R=eseidel@chromium.org

Review URL: https://codereview.chromium.org/788113002
---
 engine/core/css/CSSTestHelper.cpp             |  2 +-
 engine/core/css/StyleSheetContents.cpp        |  8 +-----
 engine/core/css/parser/BisonCSSParser-in.cpp  | 28 +------------------
 engine/core/css/parser/BisonCSSParser.h       |  9 ++----
 engine/core/css/parser/CSSGrammar.y           |  3 --
 engine/core/dom/Element.h                     |  8 ------
 engine/core/dom/StyleEngine.cpp               | 18 ++++--------
 engine/core/dom/StyleEngine.h                 |  4 +--
 engine/core/html/HTMLStyleElement.cpp         | 27 ++++--------------
 engine/core/html/HTMLStyleElement.h           | 11 ++------
 engine/core/html/HTMLTagNames.in              |  2 +-
 .../core/html/parser/HTMLConstructionSite.cpp |  7 -----
 engine/core/html/parser/HTMLElementStack.cpp  | 23 +--------------
 engine/core/html/parser/HTMLElementStack.h    |  1 -
 14 files changed, 20 insertions(+), 131 deletions(-)

diff --git a/engine/core/css/CSSTestHelper.cpp b/engine/core/css/CSSTestHelper.cpp
index 07f2dd2dc81..614ddacaaab 100644
--- a/engine/core/css/CSSTestHelper.cpp
+++ b/engine/core/css/CSSTestHelper.cpp
@@ -62,7 +62,7 @@ void CSSTestHelper::addCSSRules(const char* cssText)
 {
     TextPosition position;
     unsigned sheetLength = m_styleSheet->length();
-    ASSERT_TRUE(m_styleSheet->contents()->parseStringAtPosition(cssText, position, true));
+    ASSERT_TRUE(m_styleSheet->contents()->parseString(cssText));
     ASSERT_TRUE(m_styleSheet->length() > sheetLength);
 }
 
diff --git a/engine/core/css/StyleSheetContents.cpp b/engine/core/css/StyleSheetContents.cpp
index 0391de8b0b5..ef8923fbc8b 100644
--- a/engine/core/css/StyleSheetContents.cpp
+++ b/engine/core/css/StyleSheetContents.cpp
@@ -110,16 +110,10 @@ void StyleSheetContents::clearRules()
 }
 
 bool StyleSheetContents::parseString(const String& sheetText)
-{
-    return parseStringAtPosition(sheetText, TextPosition::minimumPosition(), false);
-}
-
-bool StyleSheetContents::parseStringAtPosition(const String& sheetText, const TextPosition& startPosition, bool createdByParser)
 {
     CSSParserContext context(parserContext(), UseCounter::getFrom(this));
     BisonCSSParser p(context);
-    p.parseSheet(this, sheetText, startPosition, 0, createdByParser);
-
+    p.parseSheet(this, sheetText);
     return true;
 }
 
diff --git a/engine/core/css/parser/BisonCSSParser-in.cpp b/engine/core/css/parser/BisonCSSParser-in.cpp
index 5dbf821b2d9..0fa96eeab38 100644
--- a/engine/core/css/parser/BisonCSSParser-in.cpp
+++ b/engine/core/css/parser/BisonCSSParser-in.cpp
@@ -107,7 +107,6 @@ BisonCSSParser::BisonCSSParser(const CSSParserContext& context)
     , m_selectorListForParseSelector(0)
     , m_numParsedPropertiesBeforeMarginBox(INVALID_NUM_PARSED_PROPERTIES)
     , m_hadSyntacticallyValidCSSRule(false)
-    , m_logErrors(false)
     , m_ignoreErrors(false)
     , m_defaultNamespace(starAtom)
     , m_observer(0)
@@ -139,15 +138,12 @@ void BisonCSSParser::setupParser(const char* prefix, unsigned prefixLength, cons
     m_ruleHasHeader = true;
 }
 
-void BisonCSSParser::parseSheet(StyleSheetContents* sheet, const String& string, const TextPosition& startPosition, CSSParserObserver* observer, bool logErrors)
+void BisonCSSParser::parseSheet(StyleSheetContents* sheet, const String& string)
 {
     setStyleSheet(sheet);
     m_defaultNamespace = starAtom; // Reset the default namespace.
-    TemporaryChange<CSSParserObserver*> scopedObsever(m_observer, observer);
-    m_logErrors = logErrors && sheet->singleOwnerDocument() && !sheet->baseURL().isEmpty() && sheet->singleOwnerDocument()->frameHost();
     m_ignoreErrors = false;
     m_tokenizer.m_lineNumber = 0;
-    m_startPosition = startPosition;
     m_source = &string;
     m_tokenizer.m_internal = false;
     setupParser("", string, "");
@@ -157,7 +153,6 @@ void BisonCSSParser::parseSheet(StyleSheetContents* sheet, const String& string,
     m_rule = nullptr;
     m_lineEndings.clear();
     m_ignoreErrors = false;
-    m_logErrors = false;
     m_tokenizer.m_internal = true;
 }
 
@@ -1584,25 +1579,6 @@ void BisonCSSParser::endInvalidRuleHeader()
     endRuleHeader();
 }
 
-void BisonCSSParser::reportError(const CSSParserLocation&, CSSParserError)
-{
-    // FIXME: error reporting temporatily disabled.
-}
-
-bool BisonCSSParser::isLoggingErrors()
-{
-    return m_logErrors && !m_ignoreErrors;
-}
-
-void BisonCSSParser::logError(const String& message, const CSSParserLocation& location)
-{
-    unsigned lineNumberInStyleSheet;
-    unsigned columnNumber = 0;
-    lineNumberInStyleSheet = location.lineNumber;
-    FrameConsole& console = m_styleSheet->singleOwnerDocument()->frame()->console();
-    console.addMessage(ConsoleMessage::create(CSSMessageSource, WarningMessageLevel, message, m_styleSheet->baseURL().string(), lineNumberInStyleSheet + m_startPosition.m_line.zeroBasedInt() + 1, columnNumber + 1));
-}
-
 StyleRuleKeyframes* BisonCSSParser::createKeyframesRule(const String& name, PassOwnPtr<Vector<RefPtr<StyleKeyframe> > > popKeyframes, bool isPrefixed)
 {
     OwnPtr<Vector<RefPtr<StyleKeyframe> > > keyframes = popKeyframes;
@@ -1763,7 +1739,6 @@ void BisonCSSParser::endRule(bool valid)
 
 void BisonCSSParser::startRuleHeader(CSSRuleSourceData::Type ruleType)
 {
-    resumeErrorLogging();
     m_ruleHeaderType = ruleType;
     m_ruleHeaderStartOffset = m_tokenizer.safeUserStringTokenOffset();
     m_ruleHeaderStartLineNumber = m_tokenizer.m_tokenStartLineNumber;
@@ -1804,7 +1779,6 @@ void BisonCSSParser::startRuleBody()
 
 void BisonCSSParser::startProperty()
 {
-    resumeErrorLogging();
     if (m_observer)
         m_observer->startProperty(m_tokenizer.safeUserStringTokenOffset());
 }
diff --git a/engine/core/css/parser/BisonCSSParser.h b/engine/core/css/parser/BisonCSSParser.h
index e8bb0c1499c..93613a9a1fd 100644
--- a/engine/core/css/parser/BisonCSSParser.h
+++ b/engine/core/css/parser/BisonCSSParser.h
@@ -86,7 +86,7 @@ public:
     void rollbackLastProperties(int num);
     void setCurrentProperty(CSSPropertyID);
 
-    void parseSheet(StyleSheetContents*, const String&, const TextPosition& startPosition = TextPosition::minimumPosition(), CSSParserObserver* = 0, bool = false);
+    void parseSheet(StyleSheetContents*, const String&);
     PassRefPtr<StyleRuleBase> parseRule(StyleSheetContents*, const String&);
     PassRefPtr<StyleKeyframe> parseKeyframeRule(StyleSheetContents*, const String&);
     bool parseSupportsCondition(const String&);
@@ -184,7 +184,6 @@ public:
     unsigned m_numParsedPropertiesBeforeMarginBox;
 
     bool m_hadSyntacticallyValidCSSRule;
-    bool m_logErrors;
     bool m_ignoreErrors;
 
     AtomicString m_defaultNamespace;
@@ -205,8 +204,7 @@ public:
     void startEndUnknownRule();
 
     void endInvalidRuleHeader();
-    void reportError(const CSSParserLocation&, CSSParserError = GeneralCSSError);
-    void resumeErrorLogging() { m_ignoreErrors = false; }
+    void reportError(const CSSParserLocation&, CSSParserError = GeneralCSSError) { }
     void setLocationLabel(const CSSParserLocation& location) { m_locationLabel = location; }
     const CSSParserLocation& lastLocationLabel() const { return m_locationLabel; }
 
@@ -270,9 +268,6 @@ private:
 
     OwnPtr<RuleSourceDataList> m_supportsRuleDataStack;
 
-    bool isLoggingErrors();
-    void logError(const String& message, const CSSParserLocation&);
-
     CSSTokenizer m_tokenizer;
 
     friend class TransformOperationInfo;
diff --git a/engine/core/css/parser/CSSGrammar.y b/engine/core/css/parser/CSSGrammar.y
index 86f8c6f085e..acc1b19b038 100644
--- a/engine/core/css/parser/CSSGrammar.y
+++ b/engine/core/css/parser/CSSGrammar.y
@@ -766,7 +766,6 @@ keyframes_rule:
 keyframe_rule_list:
     /* empty */ {
         $$ = parser->createFloatingKeyframeVector();
-        parser->resumeErrorLogging();
     }
     |  keyframe_rule_list keyframe_rule maybe_space location_label {
         $$ = $1;
@@ -774,7 +773,6 @@ keyframe_rule_list:
     }
     | keyframe_rule_list keyframes_error_recovery invalid_block maybe_space location_label {
         parser->clearProperties();
-        parser->resumeErrorLogging();
     }
     ;
 
@@ -1427,7 +1425,6 @@ regular_invalid_at_rule_header:
         parser->popSupportsRuleData();
     }
   | error_location invalid_at at_rule_header_recovery {
-        parser->resumeErrorLogging();
         parser->reportError($1, InvalidRuleCSSError);
     }
   ;
diff --git a/engine/core/dom/Element.h b/engine/core/dom/Element.h
index 0a54917cab9..0cb5b0d4d9c 100644
--- a/engine/core/dom/Element.h
+++ b/engine/core/dom/Element.h
@@ -247,14 +247,6 @@ public:
 
     String title() const;
 
-    // Called by the parser when this element's close tag is reached,
-    // signaling that all child tags have been parsed and added.
-    // This is needed for <applet> and <object> elements, which can't lay themselves out
-    // until they know all of their nested <param>s. [Radar 3603191, 4040848].
-    // Also used for script elements and some SVG elements for similar purposes,
-    // but making parsing a special case in this respect should be avoided if possible.
-    virtual void finishParsingChildren() { }
-
     bool matches(const String& selectors, ExceptionState&);
 
     DOMTokenList& classList();
diff --git a/engine/core/dom/StyleEngine.cpp b/engine/core/dom/StyleEngine.cpp
index 2cc2d8021b8..ee38518b796 100644
--- a/engine/core/dom/StyleEngine.cpp
+++ b/engine/core/dom/StyleEngine.cpp
@@ -383,15 +383,15 @@ static bool isCacheableForStyleElement(const StyleSheetContents& contents)
     return true;
 }
 
-PassRefPtr<CSSStyleSheet> StyleEngine::createSheet(Element* e, const String& text, TextPosition startPosition, bool createdByParser)
+PassRefPtr<CSSStyleSheet> StyleEngine::createSheet(Element* e, const String& text)
 {
-    RefPtr<CSSStyleSheet> styleSheet = nullptr;
-
+    RefPtr<CSSStyleSheet> styleSheet;
     AtomicString textContent(text);
 
     HashMap<AtomicString, RawPtr<StyleSheetContents> >::AddResult result = m_textToSheetCache.add(textContent, nullptr);
     if (result.isNewEntry || !result.storedValue->value) {
-        styleSheet = StyleEngine::parseSheet(e, text, startPosition, createdByParser);
+        styleSheet = CSSStyleSheet::createInline(e, KURL());
+        styleSheet->contents()->parseString(text);
         if (result.isNewEntry && isCacheableForStyleElement(*styleSheet->contents())) {
             result.storedValue->value = styleSheet->contents();
             m_sheetToTextCache.add(styleSheet->contents(), textContent);
@@ -401,21 +401,13 @@ PassRefPtr<CSSStyleSheet> StyleEngine::createSheet(Element* e, const String& tex
         ASSERT(contents);
         ASSERT(isCacheableForStyleElement(*contents));
         ASSERT(contents->singleOwnerDocument() == e->document());
-        styleSheet = CSSStyleSheet::createInline(contents, e, startPosition);
+        styleSheet = CSSStyleSheet::createInline(contents, e);
     }
 
     ASSERT(styleSheet);
     return styleSheet;
 }
 
-PassRefPtr<CSSStyleSheet> StyleEngine::parseSheet(Element* e, const String& text, TextPosition startPosition, bool createdByParser)
-{
-    RefPtr<CSSStyleSheet> styleSheet = nullptr;
-    styleSheet = CSSStyleSheet::createInline(e, KURL(), startPosition);
-    styleSheet->contents()->parseStringAtPosition(text, startPosition, createdByParser);
-    return styleSheet;
-}
-
 void StyleEngine::removeSheet(StyleSheetContents* contents)
 {
     HashMap<RawPtr<StyleSheetContents>, AtomicString>::iterator it = m_sheetToTextCache.find(contents);
diff --git a/engine/core/dom/StyleEngine.h b/engine/core/dom/StyleEngine.h
index 207429b8878..cf898f2fc22 100644
--- a/engine/core/dom/StyleEngine.h
+++ b/engine/core/dom/StyleEngine.h
@@ -126,7 +126,7 @@ public:
 
     void markDocumentDirty();
 
-    PassRefPtr<CSSStyleSheet> createSheet(Element*, const String& text, TextPosition startPosition, bool createdByParser);
+    PassRefPtr<CSSStyleSheet> createSheet(Element*, const String& text);
     void removeSheet(StyleSheetContents*);
 
     void addScopedStyleResolver(const ScopedStyleResolver* resolver) { m_scopedStyleResolvers.add(resolver); }
@@ -156,8 +156,6 @@ private:
 
     void createResolver();
 
-    static PassRefPtr<CSSStyleSheet> parseSheet(Element*, const String& text, TextPosition startPosition, bool createdByParser);
-
     const DocumentStyleSheetCollection* documentStyleSheetCollection() const
     {
         return m_documentStyleSheetCollection.get();
diff --git a/engine/core/html/HTMLStyleElement.cpp b/engine/core/html/HTMLStyleElement.cpp
index e8ab1d70469..6942b9ee596 100644
--- a/engine/core/html/HTMLStyleElement.cpp
+++ b/engine/core/html/HTMLStyleElement.cpp
@@ -36,15 +36,11 @@
 
 namespace blink {
 
-inline HTMLStyleElement::HTMLStyleElement(Document& document, bool createdByParser)
+inline HTMLStyleElement::HTMLStyleElement(Document& document)
     : HTMLElement(HTMLNames::styleTag, document)
-    , m_createdByParser(createdByParser)
     , m_loading(false)
     , m_registeredAsCandidate(false)
-    , m_startPosition(TextPosition::belowRangePosition())
 {
-    if (createdByParser)
-        m_startPosition = document.parserPosition();
 }
 
 HTMLStyleElement::~HTMLStyleElement()
@@ -54,9 +50,9 @@ HTMLStyleElement::~HTMLStyleElement()
         clearSheet();
 }
 
-PassRefPtr<HTMLStyleElement> HTMLStyleElement::create(Document& document, bool createdByParser)
+PassRefPtr<HTMLStyleElement> HTMLStyleElement::create(Document& document)
 {
-    return adoptRef(new HTMLStyleElement(document, createdByParser));
+    return adoptRef(new HTMLStyleElement(document));
 }
 
 void HTMLStyleElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
@@ -69,12 +65,6 @@ void HTMLStyleElement::parseAttribute(const QualifiedName& name, const AtomicStr
     }
 }
 
-void HTMLStyleElement::finishParsingChildren()
-{
-    process();
-    m_createdByParser = false;
-}
-
 void HTMLStyleElement::insertedInto(ContainerNode* insertionPoint)
 {
     HTMLElement::insertedInto(insertionPoint);
@@ -121,9 +111,6 @@ void HTMLStyleElement::removedFrom(ContainerNode* insertionPoint)
 void HTMLStyleElement::childrenChanged(const ChildrenChange& change)
 {
     HTMLElement::childrenChanged(change);
-
-    if (m_createdByParser)
-        return;
     process();
 }
 
@@ -177,8 +164,7 @@ void HTMLStyleElement::createSheet()
         if (screenEval.eval(mediaQueries.get()) || printEval.eval(mediaQueries.get())) {
             m_loading = true;
             const String& text = textFromChildren();
-            TextPosition startPosition = m_startPosition == TextPosition::belowRangePosition() ? TextPosition::minimumPosition() : m_startPosition;
-            m_sheet = document().styleEngine()->createSheet(this, text, startPosition, m_createdByParser);
+            m_sheet = document().styleEngine()->createSheet(this, text);
             m_sheet->setMediaQueries(mediaQueries.release());
             m_loading = false;
         }
@@ -206,10 +192,7 @@ void HTMLStyleElement::processStyleSheet()
     ASSERT(inDocument());
 
     m_registeredAsCandidate = true;
-    document().styleEngine()->addStyleSheetCandidateNode(this, m_createdByParser);
-    if (m_createdByParser)
-        return;
-
+    document().styleEngine()->addStyleSheetCandidateNode(this, false);
     process();
 }
 
diff --git a/engine/core/html/HTMLStyleElement.h b/engine/core/html/HTMLStyleElement.h
index fb7fb551b72..5c3ddd0dca1 100644
--- a/engine/core/html/HTMLStyleElement.h
+++ b/engine/core/html/HTMLStyleElement.h
@@ -31,14 +31,11 @@ namespace blink {
 
 class ContainerNode;
 class Document;
-class Element;
-class HTMLStyleElement;
-class TreeScope;
 
 class HTMLStyleElement final : public HTMLElement {
     DEFINE_WRAPPERTYPEINFO();
 public:
-    static PassRefPtr<HTMLStyleElement> create(Document&, bool createdByParser);
+    static PassRefPtr<HTMLStyleElement> create(Document&);
     virtual ~HTMLStyleElement();
 
     ContainerNode* scopingNode();
@@ -49,7 +46,7 @@ public:
     const AtomicString& type() const;
 
 private:
-    HTMLStyleElement(Document&, bool createdByParser);
+    HTMLStyleElement(Document&);
 
     void createSheet();
     void process();
@@ -64,14 +61,10 @@ private:
     virtual void removedFrom(ContainerNode*) override;
     virtual void childrenChanged(const ChildrenChange&) override;
 
-    virtual void finishParsingChildren() override;
-
     RefPtr<CSSStyleSheet> m_sheet;
 
-    bool m_createdByParser : 1;
     bool m_loading : 1;
     bool m_registeredAsCandidate : 1;
-    TextPosition m_startPosition;
 };
 
 } // namespace blink
diff --git a/engine/core/html/HTMLTagNames.in b/engine/core/html/HTMLTagNames.in
index a96e2ef3973..bd0194596eb 100644
--- a/engine/core/html/HTMLTagNames.in
+++ b/engine/core/html/HTMLTagNames.in
@@ -8,6 +8,6 @@ iframe interfaceName=HTMLIFrameElement
 img interfaceName=HTMLImageElement, constructorNeedsCreatedByParser
 import
 script
-style constructorNeedsCreatedByParser
+style
 template
 title
diff --git a/engine/core/html/parser/HTMLConstructionSite.cpp b/engine/core/html/parser/HTMLConstructionSite.cpp
index 42a5e82f4a0..a0c22a47bb3 100644
--- a/engine/core/html/parser/HTMLConstructionSite.cpp
+++ b/engine/core/html/parser/HTMLConstructionSite.cpp
@@ -77,14 +77,7 @@ static inline void insert(HTMLConstructionSiteTask& task)
 static inline void executeInsertTask(HTMLConstructionSiteTask& task)
 {
     ASSERT(task.operation == HTMLConstructionSiteTask::Insert);
-
     insert(task);
-
-    if (task.child->isElementNode()) {
-        Element& child = toElement(*task.child);
-        if (task.selfClosing)
-            child.finishParsingChildren();
-    }
 }
 
 static inline void executeInsertTextTask(HTMLConstructionSiteTask& task)
diff --git a/engine/core/html/parser/HTMLElementStack.cpp b/engine/core/html/parser/HTMLElementStack.cpp
index a946b9b1f81..db88829f6ad 100644
--- a/engine/core/html/parser/HTMLElementStack.cpp
+++ b/engine/core/html/parser/HTMLElementStack.cpp
@@ -57,12 +57,8 @@ void HTMLElementStack::popAll()
 {
     m_rootNode = nullptr;
     m_stackDepth = 0;
-    while (m_top) {
-        Node& node = *topNode();
-        if (node.isElementNode())
-            toElement(node).finishParsingChildren();
+    while (m_top)
         m_top = m_top->releaseNext();
-    }
 }
 
 void HTMLElementStack::pop()
@@ -112,27 +108,10 @@ void HTMLElementStack::pushCommon(PassRefPtr<ContainerNode> node)
 
 void HTMLElementStack::popCommon()
 {
-    top()->finishParsingChildren();
     m_top = m_top->releaseNext();
     m_stackDepth--;
 }
 
-void HTMLElementStack::removeNonTopCommon(Element* element)
-{
-    ASSERT(top() != element);
-    for (ElementRecord* pos = m_top.get(); pos; pos = pos->next()) {
-        if (pos->next()->element() == element) {
-            // FIXME: Is it OK to call finishParsingChildren()
-            // when the children aren't actually finished?
-            element->finishParsingChildren();
-            pos->setNext(pos->next()->releaseNext());
-            m_stackDepth--;
-            return;
-        }
-    }
-    ASSERT_NOT_REACHED();
-}
-
 #ifndef NDEBUG
 
 void HTMLElementStack::show()
diff --git a/engine/core/html/parser/HTMLElementStack.h b/engine/core/html/parser/HTMLElementStack.h
index 96bcf8ed510..769c7f256d3 100644
--- a/engine/core/html/parser/HTMLElementStack.h
+++ b/engine/core/html/parser/HTMLElementStack.h
@@ -102,7 +102,6 @@ private:
     void popUntil(Element*);
     void pushCommon(PassRefPtr<ContainerNode>);
     void popCommon();
-    void removeNonTopCommon(Element*);
 
     OwnPtr<ElementRecord> m_top;