| Summary: | Modernize and streamline HTMLTokenizer | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||||||||||
| Component: | Page Loading | Assignee: | Darin Adler <darin> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | ap, benjamin, buildbot, calvaris, cmarcelo, commit-queue, dbates, eric.carlson, esprehn+autocc, gyuyoung.kim, ossy, philipj, rniwa | ||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||
| Bug Depends on: | 140292 | ||||||||||||||||||||
| Bug Blocks: | 163978 | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Darin Adler
2015-01-06 19:28:59 PST
Created attachment 244138 [details]
Patch
Created attachment 244142 [details]
Patch
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.
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. 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
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. 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
Created attachment 244237 [details]
Patch
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.
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. 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
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. 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
Comment on attachment 244237 [details]
Patch
Let me fix the XSS auditor now.
Created attachment 244265 [details]
Patch
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.
Committed r178154: <http://trac.webkit.org/changeset/178154> (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. (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? (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 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 Re-opened since this is blocked by bug 140292 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? 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. Committed r178265: <http://trac.webkit.org/changeset/178265> (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. |