Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(642)

Issue 23455024: Move ownership of the TextResourceDecoder to DecodedDataDocumentParser (Closed)

Created:
7 years, 3 months ago by jamesr
Modified:
6 years, 9 months ago
CC:
blink-reviews, eae+blinkwatch, leviw+renderwatch, dglazkov+blink, adamk+blink_chromium.org, jchaffraix+rendering, Nate Chapin, gavinp+loader_chromium.org
Visibility:
Public.

Description

Move ownership of the TextResourceDecoder to DecodedDataDocumentParser This moves the Document's reference to the decoder to the parser. The parser informs the document of changes in the encoding as well as two bits indicating if the encoding was determined heuristically (to set up the decoder properly for child frames) and if the decoder has encountered any decoding errors (to generate errors for XML documents). BUG=277886

Patch Set 1 #

Total comments: 4

Patch Set 2 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+113 lines, -41 lines) Patch
M Source/core/dom/DecodedDataDocumentParser.h View 3 chunks +8 lines, -0 lines 0 comments Download
M Source/core/dom/DecodedDataDocumentParser.cpp View 1 3 chunks +23 lines, -6 lines 0 comments Download
M Source/core/dom/Document.h View 1 3 chunks +13 lines, -7 lines 0 comments Download
M Source/core/dom/Document.cpp View 1 6 chunks +21 lines, -15 lines 0 comments Download
M Source/core/dom/DocumentParser.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/fetch/TextResourceDecoder.h View 1 2 chunks +7 lines, -7 lines 0 comments Download
M Source/core/loader/DocumentLoader.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/loader/DocumentLoader.cpp View 1 1 chunk +6 lines, -0 lines 0 comments Download
M Source/core/loader/DocumentWriter.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/loader/DocumentWriter.cpp View 1 3 chunks +11 lines, -2 lines 0 comments Download
M Source/core/loader/TextResourceDecoderBuilder.cpp View 1 2 chunks +3 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderText.cpp View 1 2 chunks +11 lines, -0 lines 0 comments Download
M Source/core/xml/XSLTProcessor.cpp View 1 1 chunk +3 lines, -1 line 0 comments Download
M Source/core/xml/parser/XMLDocumentParser.cpp View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 2 (0 generated)
jamesr
Document::DocumentEncodingData is really ugly in this patch, but I think the rest is in pretty ...
7 years, 3 months ago (2013-08-30 22:44:48 UTC) #1
abarth-chromium
7 years, 3 months ago (2013-08-30 22:50:38 UTC) #2
https://codereview.chromium.org/23455024/diff/1/Source/core/dom/DecodedDataDo...
File Source/core/dom/DecodedDataDocumentParser.cpp (right):

https://codereview.chromium.org/23455024/diff/1/Source/core/dom/DecodedDataDo...
Source/core/dom/DecodedDataDocumentParser.cpp:109:
document()->setEncoding(encodingData);
This happens for every call to appendBytes?  We can probably do better than
that.

https://codereview.chromium.org/23455024/diff/1/Source/core/dom/DecodedDataDo...
Source/core/dom/DecodedDataDocumentParser.cpp:133:
document()->setEncoding(encodingData);
Copy/paste code makes me a sad panda.

https://codereview.chromium.org/23455024/diff/1/Source/core/dom/Document.cpp
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/23455024/diff/1/Source/core/dom/Document.cpp#...
Source/core/dom/Document.cpp:1135: setEncoding(data);
Won't this get spammed over the next time we get a chunk of data from the
network?

https://codereview.chromium.org/23455024/diff/1/Source/core/loader/DocumentWr...
File Source/core/loader/DocumentWriter.cpp (right):

https://codereview.chromium.org/23455024/diff/1/Source/core/loader/DocumentWr...
Source/core/loader/DocumentWriter.cpp:102:
m_parser->asDecodedDataDocumentParser()->setDecoder(m_decoder);
Maybe setDecoder should be defined on the same interface as needsDecoder?  Then
you wouldn't need this dynamic cast.

Powered by Google App Engine
This is Rietveld 408576698