Revert of "Revert of Moved text decoding to the parser thread"

Revert of "Revert of Moved text decoding to the parser thread"

Reverts https://codereview.chromium.org/118573002/ and adds a crash fix.

Original commit message:
>>Moved text decoding to the parser thread.

>>When the HTTPDocumentParser defers the document parsing to the background parser thread, it'll now also pass ownership of its TextResourceDecoder so the parser thread can do the decoding as well.

R=abarth@chromium.org,eseidel@chromium.org
BUG=277886
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164797

[oystein (OOO til 10th of July), 2014-01-08 19:45:50]

abarth: My original CL caused a crash when BackgroundHTMLParser::appendBytes()
got called without a decoder being present (i.e. the member was NULL). 

I haven't been able to reproduce this, but the only way I can see this happening
is if DocumentParser::setDecoder is explicitly called with a NULL decoder
mid-stream. Under the old behavior this would cause the decoder to be re-created
next time appendBytes() gets called, and I've restored this behavior now (the
only difference between this CL and the previous is in
DecodedDataDocumentParser::setDecoder).

[eseidel, 2014-01-09 00:40:21]

The revert of the revert is confusing.  If you want to do that, I'd land the
test separately first, and then just do a clean revert of a revert.

Normally I don't review revert patches particularly closely, but presumably I
should review this one since there are other changes?

[oystein (OOO til 10th of July), 2014-01-09 00:55:11]

On 2014/01/09 00:40:21, eseidel wrote:
> The revert of the revert is confusing.  If you want to do that, I'd land the
> test separately first, and then just do a clean revert of a revert.
> 
> Normally I don't review revert patches particularly closely, but presumably I
> should review this one since there are other changes?

I wasn't quite sure how to handle it to be honest, since it's a revert +
crashfix. I could do clean revert and then a separate CL with the crashfix I
suppose, just feels strange.

The only actual difference vs. the original CL is in
DecodedDataDocumentParser::setDecoder,

which has
m_needsDecoder = false;
changed to
m_needsDecoder = !decoder. (and a comment added).

[abarth-chromium, 2014-01-09 06:07:45]

LGTM

I didn't re-review the entire CL, but I did look at the one line that you
changed.  Thanks for sticking with this change.

[commit-bot: I haz the power, 2014-01-09 17:30:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oysteine@chromium.org/129123002/80001

[commit-bot: I haz the power, 2014-01-09 17:31:03]

Failed to apply patch for Source/core/dom/DecodedDataDocumentParser.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/dom/DecodedDataDocumentParser.h
  Hunk #1 FAILED at 42.
  1 out of 2 hunks FAILED -- saving rejects to file
Source/core/dom/DecodedDataDocumentParser.h.rej

Patch:       Source/core/dom/DecodedDataDocumentParser.h
Index: Source/core/dom/DecodedDataDocumentParser.h
diff --git a/Source/core/dom/DecodedDataDocumentParser.h
b/Source/core/dom/DecodedDataDocumentParser.h
index
453147d0ecdbddad2e90e7e9cda345afa016b534..979310d4874ea3d04e570ac5333e845432820d38
100644
--- a/Source/core/dom/DecodedDataDocumentParser.h
+++ b/Source/core/dom/DecodedDataDocumentParser.h
@@ -42,10 +42,11 @@ public:
     // The below functions are used by DocumentWriter (the loader).
     virtual void appendBytes(const char* bytes, size_t length) OVERRIDE;
     virtual void flush() OVERRIDE;
-    virtual bool needsDecoder() const OVERRIDE { return !m_decoder; }
+    virtual bool needsDecoder() const OVERRIDE { return m_needsDecoder; }
     virtual void setDecoder(PassOwnPtr<TextResourceDecoder>) OVERRIDE;
     virtual TextResourceDecoder* decoder() OVERRIDE;
-    virtual void setHasAppendedData() OVERRIDE;
+
+    PassOwnPtr<TextResourceDecoder> takeDecoder();
 
 protected:
     explicit DecodedDataDocumentParser(Document*);
@@ -57,7 +58,7 @@ private:
 
     void updateDocument(String& decodedData);
 
-    bool m_hasAppendedData;
+    bool m_needsDecoder;
     OwnPtr<TextResourceDecoder> m_decoder;
 };

[commit-bot: I haz the power, 2014-01-09 18:31:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oysteine@chromium.org/129123002/150001

[commit-bot: I haz the power, 2014-01-09 19:49:27]

Change committed as 164797