Process decoded bytes when in stopped (but not detached) state

Process decoded bytes when in stopped (but not detached) state

When a DecodedDataDocumentParser receives bytes it can choose whether to
pass those through to the underlying document's decoder or not. If the
parser is the detached state, there is no underlying document so it doesn't
make sense to process the bytes at all. r158300 patched
DecodedDataDocumentParser to not process bytes when in the stopped or detached
state but it turns out in some scenarios involving XSLT we receive bytes
in the stopped state. This relaxes the check from isStopped() to isDetached().

BUG=304965
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159848

[jamesr, 2013-10-15 19:47:29]

I'm still working on understanding the sequence of events that leads to this
state better so I can make a good test.

[eseidel, 2013-10-15 19:55:19]

http://src.chromium.org/viewvc/blink?revision=158300&view=revision

[abarth-chromium, 2013-10-15 19:56:01]

Hum...  Maybe we should dig deeper and understand what's going on.  We shouldn't
be here when the parser is stopped.  If we go this route, we should add some
ASSERTs about this only being for the XSLT case.

[jamesr, 2013-10-15 19:56:22]

On 2013/10/15 19:55:19, eseidel wrote:
> http://src.chromium.org/viewvc/blink?revision=158300&view=revision

Right, this is a partial revert of that patch as I said in the description.

[eseidel, 2013-10-15 19:57:23]

The goal here is to allow the XMLDocumentParser to still recieve bytes in the
stopped state (and make its own decision about whether it should respect stopped
or not)?

[eseidel, 2013-10-15 19:58:01]

On 2013/10/15 19:56:22, jamesr wrote:
> On 2013/10/15 19:55:19, eseidel wrote:
> > http://src.chromium.org/viewvc/blink?revision=158300&view=revision
> 
> Right, this is a partial revert of that patch as I said in the description.

Yeah, rietveld just didnt' give me a clickable link, so I found one, and figured
others might want it.

[jamesr, 2013-10-15 20:31:54]

We call stopParsing() on this stack:

2   Content Shell Framework             0x02e2f35b
WebCore::DocumentParser::stopParsing() + 107
3   Content Shell Framework             0x04253060
WebCore::XMLDocumentParser::stopParsing() + 48
4   Content Shell Framework             0x04251858
WebCore::XMLDocumentParser::processingInstruction(WTF::String const&,
WTF::String const&) + 744
5   Content Shell Framework             0x042521df
WebCore::processingInstructionHandler(void*, unsigned char const*, unsigned char
const*) + 127
6   Content Shell Framework             0x031610d0 xmlParsePI + 2560
7   Content Shell Framework             0x03176c02 xmlParseTryOrFinish + 7746
8   Content Shell Framework             0x03174b27 xmlParseChunk + 1255
9   Content Shell Framework             0x0424fdb0
WebCore::parseChunk(_xmlParserCtxt*, WTF::String const&) + 240
10  Content Shell Framework             0x0424cfbd
WebCore::XMLDocumentParser::doWrite(WTF::String const&) + 429
11  Content Shell Framework             0x0424cd0c
WebCore::XMLDocumentParser::append(WTF::PassRefPtr<WTF::StringImpl>) + 364
12  Content Shell Framework             0x02dbb1eb
WebCore::DecodedDataDocumentParser::appendBytes(char const*, unsigned long) +
539
13  Content Shell Framework             0x040412c1
WebCore::DocumentWriter::addData(char const*, unsigned long) + 401
14  Content Shell Framework             0x040302e5
WebCore::DocumentLoader::commitData(char const*, unsigned long) + 229
15  Content Shell Framework             0x04032add
WebCore::DocumentLoader::dataReceived(WebCore::Resource*, char const*, int) +
1037
16  Content Shell Framework             0x03df06b3
WebCore::RawResource::appendData(char const*, int) + 179
17  Content Shell Framework             0x03e22560
WebCore::ResourceLoader::didReceiveData(WebKit::WebURLLoader*, char const*, int,
int) + 544

because of this code:

    m_sawXSLTransform = !m_sawFirstElement && pi->isXSL();
    if (m_sawXSLTransform && !document()->transformSourceDocument())
        stopParsing();

and then feed more bytes in via:

0   Content Shell Framework             0x004aaf6f
base::debug::StackTrace::StackTrace() + 63
1   Content Shell Framework             0x004aaf0b
base::debug::StackTrace::StackTrace() + 43
2   Content Shell Framework             0x02dbb0c0
WebCore::DecodedDataDocumentParser::appendBytes(char const*, unsigned long) +
240
3   Content Shell Framework             0x040412c1
WebCore::DocumentWriter::addData(char const*, unsigned long) + 401
4   Content Shell Framework             0x040302e5
WebCore::DocumentLoader::commitData(char const*, unsigned long) + 229
5   Content Shell Framework             0x04032add
WebCore::DocumentLoader::dataReceived(WebCore::Resource*, char const*, int) +
1037
6   Content Shell Framework             0x03df06b3
WebCore::RawResource::appendData(char const*, int) + 179
7   Content Shell Framework             0x03e22560
WebCore::ResourceLoader::didReceiveData(WebKit::WebURLLoader*, char const*, int,
int) + 544

[jamesr, 2013-10-16 19:05:24]

I've added a comment to XMLDocumentParser highlighting the craziness and a test.
The test is a bit tricky since it requires that we receive additional chunks of
data after processing the XSL processing instruction, so it's an http test.

PTAL

[eseidel, 2013-10-16 19:35:08]

The test doesn't seem "slow" despite labeling itself as such.  How are you
enforcing packet boundaries?

otherwise lgtm.

https://codereview.chromium.org/27394002/diff/9001/LayoutTests/http/tests/inc...
File LayoutTests/http/tests/incremental/slow-xsl-transformed-xml.pl (right):

https://codereview.chromium.org/27394002/diff/9001/LayoutTests/http/tests/inc...
LayoutTests/http/tests/incremental/slow-xsl-transformed-xml.pl:16: for
($count=1; $count<4000; $count++) {
Do you want to sleep in here?

https://codereview.chromium.org/27394002/diff/9001/Source/core/dom/DecodedDat...
File Source/core/dom/DecodedDataDocumentParser.cpp (right):

https://codereview.chromium.org/27394002/diff/9001/Source/core/dom/DecodedDat...
Source/core/dom/DecodedDataDocumentParser.cpp:61: if (isDetached())
I might add a comment explaining why this is only isDetached.

[jamesr, 2013-10-16 19:39:58]

I'm not doing anything specific to enforce packet boundaries other than what the
other tests in this directory do.  It seems to reliably fail for me without the
.cpp change.  I'm not sure what else to call it.

[abarth-chromium, 2013-10-17 01:08:07]

slow-xsl-transformed-xml.pl -> chunked-xsl-transformed-xml.pl ?

[commit-bot: I haz the power, 2013-10-17 01:49:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/27394002/18001

[jamesr, 2013-10-17 01:50:28]

Renamed the test, added a comment and stuck this in the CQ.  Happy to do more
tweaks if folks want it but I'd like to get the fix in soon so we can decide
whether to merge or revert the original crash fix in 31.

[commit-bot: I haz the power, 2013-10-17 09:10:37]

Change committed as 159848