Bug 140166 – Modernize and streamline HTMLTokenizer

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Darin Adler]

Modernize and streamline HTMLTokenizer

[Darin Adler]

Created attachment 244138 [details]
Patch

[Darin Adler]

Created attachment 244142 [details]
Patch

[WebKit Commit Bot]

Attachment 244142 [details] did not pass style-queue:


ERROR: Source/WebCore/html/parser/HTMLTokenizer.cpp:193:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/xml/parser/MarkupTokenizerInlines.h:71:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/xml/parser/MarkupTokenizerInlines.h:82:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/xml/parser/MarkupTokenizerInlines.h:92:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 4 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 244142 [details]
Patch

Attachment 244142 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6733458424463360

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 244146 [details]
Archive of layout-test-results from ews101 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 244142 [details]
Patch

Attachment 244142 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6405899522408448

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 244147 [details]
Archive of layout-test-results from ews104 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Darin Adler]

Created attachment 244237 [details]
Patch

[WebKit Commit Bot]

Attachment 244237 [details] did not pass style-queue:


ERROR: Source/WebCore/html/parser/HTMLTokenizer.cpp:213:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/xml/parser/MarkupTokenizerInlines.h:71:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/xml/parser/MarkupTokenizerInlines.h:82:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 3 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 244237 [details]
Patch

Attachment 244237 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4618170388185088

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 244239 [details]
Archive of layout-test-results from ews101 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 244237 [details]
Patch

Attachment 244237 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6476922175356928

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 244241 [details]
Archive of layout-test-results from ews106 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Darin Adler]

Comment on attachment 244237 [details]
Patch

Let me fix the XSS auditor now.

[Darin Adler]

Created attachment 244265 [details]
Patch

[WebKit Commit Bot]

Attachment 244265 [details] did not pass style-queue:


ERROR: Source/WebCore/html/parser/HTMLTokenizer.cpp:213:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/xml/parser/MarkupTokenizerInlines.h:71:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/xml/parser/MarkupTokenizerInlines.h:82:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 3 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Committed r178154: <http://trac.webkit.org/changeset/178154>

[Csaba Osztrogonác]

(In reply to comment #17)
> Committed r178154: <http://trac.webkit.org/changeset/178154>

It broke the EFL build, but unfortunately EWS was dead that time.
build log:
In file included from ../../Source/WebCore/html/parser/HTMLEntityParser.cpp:31:0:
../../Source/WebCore/xml/parser/CharacterReferenceParserInlines.h: In instantiation of 'bool WebCore::consumeCharacterReference(WebCore::SegmentedString&, WTF::StringBuilder&, bool&, UChar) [with ParserFunctions = WebCore::HTMLEntityParser; UChar = short unsigned int]':
../../Source/WebCore/html/parser/HTMLEntityParser.cpp:120:126:   required from here
../../Source/WebCore/xml/parser/CharacterReferenceParserInlines.h:62:10: error: variable 'overflow' set but not used [-Werror=unused-but-set-variable]
cc1plus: all warnings being treated as errors

I removed the unused overflow variable to fix the build:
https://trac.webkit.org/changeset/178163

Please file a new bug report to add it back with real use 
of its value if it is really necessary as the FIXME says.

[Csaba Osztrogonác]

(In reply to comment #17)
> Committed r178154: <http://trac.webkit.org/changeset/178154>

Additionally it made 10 tests crash on Apple debug bots and 7 tests fail 
on Apple release bots. Could you possibly check this regression?

[Csaba Osztrogonác]

(In reply to comment #18)
> I removed the unused overflow variable to fix the build:
> https://trac.webkit.org/changeset/178163

One more fix after the fix - https://trac.webkit.org/changeset/178164

[Alexey Proskuryakov]

This made multiple entity related tests assert, I'm going to roll out.

https://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r178171%20(9710)/results.html

[WebKit Commit Bot]

Re-opened since this is blocked by bug 140292

[Alexey Proskuryakov]

I:n release builds, tests fail like this: 

https://build.webkit.org/results/Apple%20Yosemite%20Release%20WK2%20(Tests)/r178171%20(1837)/fast/parser/numeric-entities-pretty-diff.html

I'm very surprised that EWS didn't catch this. Were there additional changes landed that it didn't check?

[Darin Adler]

All the entity problems and release build failures were from a bad merge when I rebased this because of the overlap with bug 140179.

That’s why EWS didn’t catch the problems. EWS checked my original correct patch, not the rebased one I landed.

[Darin Adler]

Committed r178265: <http://trac.webkit.org/changeset/178265>

[Daniel Bates]

(In reply to comment #25)
> Committed r178265: <http://trac.webkit.org/changeset/178265>

This regressed the XSS Auditor's ability to block an injected attribute that spans document.write() boundaries. See bug #163978 for more details.