Bug 82978 – inspector/styles/svg-style.xhtml is flaky (XMLDocumentParser fails to pause entirely)

• Status: RESOLVED
• Resolution: INVALID
• Component: DOM

[Enrica Casucci]

inspector/styles/svg-style.xhtml

[Joseph Pecoraro]

Interesting, this test fails the first time with:

    rect  { (svg-style.xhtml:38)

But when run repeatedly all subsequent runs correctly get:

    rect  { (svg-style.xhtml:30)

Very weird.

* STEPS TO REPRODUCE
shell> run-webkit-tests --debug --repeat-each=10 inspector/styles/svg-style.xhtml

[Joseph Pecoraro]

Looks like the backend gives us the wrong source line info for the inline <style> in this SVG. Maybe just the first time?

The frontend asks for the styles:

    frontend: {"method":"CSS.getMatchedStylesForNode","params":{"nodeId":9,"includePseudo":true,"includeInherited":true},"id":59}

And gets back a list of rules, the rule for the SVG has a bad sourceLine, notice it has 37 below, I'd expect something like 29.

    "sourceURL": "file:///Volumes/Data/Code/webkit-open-source/LayoutTests/inspector/styles/svg-style.xhtml",
    "sourceLine": 37,
    "origin": "regular",
    "style": {
        "cssProperties": [{
            "name": "fill",
            "value": "red",
            "text": "fill: red",
            "implicit": false,
            "range": {
                "startLine": 1,
                "startColumn": 15,
                "endLine": 1,
                "endColumn": 24
            },
            "status": "active"
        }],
        ...
    },
    "ruleId": {
        "styleSheetId": "4",
        "ordinal": 0
    }

[Joseph Pecoraro]

Wrong line number comes in from the CSSParser:

(lldb) bt
* thread #1: tid = 0x963fa, 0x00000001073fc826 WebCore`WebCore::StyleRuleBase::StyleRuleBase(this=0x00007fdbd542deb0, type=Style, sourceLine=37) + 22 at StyleRule.h:104, queue = 'com.apple.main-thread, stop reason = breakpoint 2.2
    frame #0: 0x00000001073fc826 WebCore`WebCore::StyleRuleBase::StyleRuleBase(this=0x00007fdbd542deb0, type=Style, sourceLine=37) + 22 at StyleRule.h:104
    frame #1: 0x00000001073fab98 WebCore`WebCore::StyleRule::StyleRule(this=0x00007fdbd542deb0, sourceLine=37, properties=0x00007fff5c8be610) + 56 at StyleRule.cpp:238
    frame #2: 0x00000001073fab4b WebCore`WebCore::StyleRule::StyleRule(this=0x00007fdbd542deb0, sourceLine=37, properties=<unavailable>) + 27 at StyleRule.cpp:239
    frame #3: 0x0000000106059536 WebCore`WebCore::StyleRule::create(sourceLine=37, properties=0x00007fff5c8be660) + 86 at StyleRule.h:123
    frame #4: 0x00000001060478fc WebCore`WebCore::CSSParser::createStyleRule(this=0x00007fff5c8c0968, selectors=0x00007fff5c8c1ba8) + 140 at CSSParser.cpp:11746
    frame #5: 0x0000000106006d2f WebCore`cssyyparse(parser=0x00007fff5c8c0968) + 6623 at CSSGrammar.y:1039
    frame #6: 0x000000010601e585 WebCore`WebCore::CSSParser::parseSheet(this=0x00007fff5c8c0968, sheet=0x00007fdbd542d9e0, string=0x00007fff5c8c1da8, startLineNumber=36, ruleSourceDataResult=0x0000000000000000, logErrors=true) + 357 at CSSParser.cpp:469
    frame #7: 0x0000000107401eaf WebCore`WebCore::StyleSheetContents::parseStringAtLine(this=0x00007fdbd542d9e0, sheetText=0x00007fff5c8c1da8, startLineNumber=36, createdByParser=true) + 111 at StyleSheetContents.cpp:325
    frame #8: 0x00000001073b251a WebCore`WebCore::InlineStyleSheetOwner::createSheet(this=0x00007fdbd542d880, element=0x00007fdbd542d7d0, text=0x00007fff5c8c1da8) + 1274 at InlineStyleSheetOwner.cpp:158
    frame #9: 0x00000001073b1cc4 WebCore`WebCore::InlineStyleSheetOwner::createSheetFromTextContents(this=0x00007fdbd542d880, element=0x00007fdbd542d7d0) + 68 at InlineStyleSheetOwner.cpp:107
    frame #10: 0x00000001073b2010 WebCore`WebCore::InlineStyleSheetOwner::finishParsingChildren(this=0x00007fdbd542d880, element=0x00007fdbd542d7d0) + 112 at InlineStyleSheetOwner.cpp:101
    frame #11: 0x000000010752968b WebCore`WebCore::SVGStyleElement::finishParsingChildren(this=0x00007fdbd542d7d0) + 43 at SVGStyleElement.cpp:139
    frame #12: 0x00000001076d4a98 WebCore`WebCore::XMLDocumentParser::endElementNs(this=0x00007fdbd22499a0) + 184 at XMLDocumentParserLibxml2.cpp:891
    frame #13: 0x00000001076dc3f9 WebCore`WebCore::PendingCallbacks::PendingEndElementNSCallback::call(this=0x00007fdbd224eaa0, parser=0x00007fdbd22499a0) + 25 at XMLDocumentParserLibxml2.cpp:257
    frame #14: 0x00000001076d8cd2 WebCore`WebCore::PendingCallbacks::callAndRemoveFirstCallback(this=0x00007fdbd2248f10, parser=0x00007fdbd22499a0) + 98 at XMLDocumentParserLibxml2.cpp:209
    frame #15: 0x00000001076d6e2c WebCore`WebCore::XMLDocumentParser::resumeParsing(this=0x00007fdbd22499a0) + 236 at XMLDocumentParserLibxml2.cpp:1470
    frame #16: 0x00000001076d1750 WebCore`WebCore::XMLDocumentParser::notifyFinished(this=0x00007fdbd22499a0, unusedResource=0x00007fdbd542ea30) + 688 at XMLDocumentParser.cpp:271
    frame #17: 0x00000001076d17af WebCore`non-virtual thunk to WebCore::XMLDocumentParser::notifyFinished(WebCore::CachedResource*) + 47
    frame #18: 0x0000000105ea63ed WebCore`WebCore::CachedResource::checkNotify(this=0x00007fdbd542ea30) + 109 at CachedResource.cpp:369
    frame #19: 0x0000000105ea64f4 WebCore`WebCore::CachedResource::finishLoading(this=0x00007fdbd542ea30) + 52 at CachedResource.cpp:385
    frame #20: 0x0000000105ebe594 WebCore`WebCore::CachedScript::finishLoading(this=0x00007fdbd542ea30, data=0x00007fdbd542ede0) + 164 at CachedScript.cpp:89
    frame #21: 0x0000000107411b5c WebCore`WebCore::SubresourceLoader::didFinishLoading(this=0x00007fdbd306bc00, finishTime=0) + 444 at SubresourceLoader.cpp:282
    frame #22: 0x00000001037c763c WebKit2`WebKit::WebResourceLoader::didFinishResourceLoad(this=0x00007fdbd5418f90, finishTime=0) + 156 at WebResourceLoader.cpp:129
    frame #23: 0x00000001037c992b WebKit2`void CoreIPC::callMemberFunction<WebKit::WebResourceLoader, void (args=0x00007fff5c8c2318, object=0x00007fdbd5418f90, function=test at 0x00007fff5c8c22c0)(double), double>(CoreIPC::Arguments1<double> const&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(double)) + 139 at HandleMessage.h:21
    frame #24: 0x00000001037c9202 WebKit2`void CoreIPC::handleMessage<Messages::WebResourceLoader::DidFinishResourceLoad, WebKit::WebResourceLoader, void (decoder=0x00007fdbd441cda0, object=0x00007fdbd5418f90, function=test at 0x00007fff5c8c2320)(double)>(CoreIPC::MessageDecoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(double)) + 114 at HandleMessage.h:376
    frame #25: 0x00000001037c8b38 WebKit2`WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(this=0x00007fdbd5418f90, decoder=0x00007fdbd441cda0) + 952 at WebResourceLoaderMessageReceiver.cpp:66
    frame #26: 0x000000010349f68c WebKit2`WebKit::NetworkProcessConnection::didReceiveMessage(this=0x00007fdbd141ac00, connection=0x00007fdbd1421450, decoder=0x00007fdbd441cda0) + 172 at NetworkProcessConnection.cpp:60

(lldb) 
frame #4: 0x00000001060478fc WebCore`WebCore::CSSParser::createStyleRule(this=0x00007fff5c8c0968, selectors=0x00007fff5c8c1ba8) + 140 at CSSParser.cpp:11746
   11743	        m_allowNamespaceDeclarations = false;
   11744	        if (m_hasFontFaceOnlyValues)
   11745	            deleteFontFaceOnlyValues();
-> 11746	        RefPtr<StyleRule> rule = StyleRule::create(m_lastSelectorLineNumber, createStylePropertySet());
   11747	        rule->parserAdoptSelectorVector(*selectors);
   11748	        result = rule.get();
   11749	        m_parsedRules.append(rule.release());

(lldb) p m_lastSelectorLineNumber
(int) $17 = 37

[Joseph Pecoraro]

Apparently the line is wrong when the InlineStyleSheetOwner::InlineStyleSheetOwner is created, which comes from the XMLDocumentParser.

    if (createdByParser && document && document->scriptableDocumentParser() && !document->isInDocumentWrite())
        m_startLineNumber = document->scriptableDocumentParser()->lineNumber();

    (lldb) bt
    * thread #1: tid = 0x963fa, 0x00000001073b1b33 WebCore`WebCore::InlineStyleSheetOwner::InlineStyleSheetOwner(this=0x00007fdbdbd14d40, document=0x00007fdbd3097600, createdByParser=true) + 211 at InlineStyleSheetOwner.cpp:46, queue = 'com.apple.main-thread, stop reason = step in
        frame #0: 0x00000001073b1b33 WebCore`WebCore::InlineStyleSheetOwner::InlineStyleSheetOwner(this=0x00007fdbdbd14d40, document=0x00007fdbd3097600, createdByParser=true) + 211 at InlineStyleSheetOwner.cpp:46
        frame #1: 0x00000001073b1a4c WebCore`WebCore::InlineStyleSheetOwner::InlineStyleSheetOwner(this=0x00007fdbdbd14d40, document=0x00007fdbd3097600, createdByParser=true) + 44 at InlineStyleSheetOwner.cpp:46
        frame #2: 0x0000000107529a95 WebCore`WebCore::SVGStyleElement::SVGStyleElement(this=0x00007fdbdbd14c90, tagName=0x00007fff5c8c1e80, document=0x00007fdbd3097600, createdByParser=true) + 101 at SVGStyleElement.cpp:41
        frame #3: 0x00000001075298d4 WebCore`WebCore::SVGStyleElement::SVGStyleElement(this=0x00007fdbdbd14c90, tagName=0x00007fff5c8c1e80, document=0x00007fdbd3097600, createdByParser=true) + 52 at SVGStyleElement.cpp:43
        frame #4: 0x0000000107529076 WebCore`WebCore::SVGStyleElement::create(tagName=0x00007fff5c8c1e80, document=0x00007fdbd3097600, createdByParser=true) + 86 at SVGStyleElement.cpp:52
        frame #5: 0x000000010747802e WebCore`styleConstructor(tagName=0x00007fff5c8c1e80, document=0x00007fdbd3097600, createdByParser=true) + 62 at SVGElementFactory.cpp:484
        frame #6: 0x0000000107475f1c WebCore`WebCore::SVGElementFactory::createSVGElement(qName=0x00007fff5c8c1e80, document=0x00007fdbd3097600, createdByParser=true) + 220 at SVGElementFactory.cpp:655
        frame #7: 0x00000001061ada4f WebCore`WebCore::Document::createElement(this=0x00007fdbd3097600, qName=0x00007fff5c8c1e80, createdByParser=true) + 223 at Document.cpp:1102
        frame #8: 0x00000001076d40b8 WebCore`WebCore::XMLDocumentParser::startElementNs(this=0x00007fdbd54411f0, xmlLocalName=0x00007fdbd5449250, xmlPrefix=0x00007fdbd5449260, xmlURI=0x00007fdbd5449270, nb_namespaces=0, libxmlNamespaces=0x00007fdbd5449290, nb_attributes=1, nb_defaulted=0, libxmlAttributes=0x00007fdbd54492a0) + 712 at XMLDocumentParserLibxml2.cpp:824

    (lldb) p m_startLineNumber
    (WTF::OrdinalNumber) $35 = {
      m_zeroBasedValue = 36
    }

[Joseph Pecoraro]

Actually the scriptableDocumentParser might not be the XMLDocumentParser. In which case this would kind of make sense. Line 36 (0-based) would be AFTER the <svg> is parsed. But the <svg:style> in this case is somewhere inside the <svg>. Seems like this could be the problem.

[Joseph Pecoraro]

(In reply to comment #5)
> Actually the scriptableDocumentParser might not be the XMLDocumentParser. In which case this would kind of make sense. Line 36 (0-based) would be AFTER the <svg> is parsed. But the <svg:style> in this case is somewhere inside the <svg>. Seems like this could be the problem.

Hmm, nope that was overreaching.

Here the ScriptableDocumentParser is the XMLDocument, and we're in XMLDocumentParser::startElementNs (NOTE: XMLDocumentParserLibxml2.cpp). So I would expect XMLDocumentParser::lineNumber() to be correct at this point.

[Joseph Pecoraro]

Breakthough. Apparently the XMLDocumentParser, when paused, can keep consuming input. So it adds pending callbacks for "startElement":

    void XMLDocumentParser::startElementNs(const xmlChar* xmlLocalName, const xmlChar* xmlPrefix, const xmlChar* xmlURI, int nb_namespaces, const xmlChar** libxmlNamespaces, int nb_attributes, int nb_defaulted, const xmlChar** libxmlAttributes)
    {
        ...
        if (m_parserPaused) {
            m_pendingCallbacks->appendStartElementNSCallback(xmlLocalName, xmlPrefix, xmlURI, nb_namespaces, libxmlNamespaces,
                                                             nb_attributes, nb_defaulted, libxmlAttributes);
            return;
        }

So by the time we replay the pendingCallback, the real libXML parser's context()->input will be far past this particular element. We can save the context information at each startElement and return that when asked for lineNumber/columnNumber.

The line number seems correct (and is only what we care about right now), but the column number is a bit off. At the point in which we get XMLDocumentParser::startElementNs for <svg:style> the libxml parser's context says line (29) and column (22). The column is not quite what we would have wanted: (the | inserted by me)

      <svg:style type=|"text/css">

If we want truly accurate starting line/column information of the contents within the <style> we may need to do some extra work. Especially if we wanted to correctly handle input like:

    <style
        type="text/css">…</style>

For now I don't see any urgency in fixing this so I'm going to drop the issue.

[Brian Burg]

Test deleted.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.