Bug 140166 - Modernize and streamline HTMLTokenizer
Summary: Modernize and streamline HTMLTokenizer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on: 140292
Blocks: 163978
  Show dependency treegraph
 
Reported: 2015-01-06 19:28 PST by Darin Adler
Modified: 2016-10-25 13:50 PDT (History)
13 users (show)

See Also:


Attachments
Patch (215.41 KB, patch)
2015-01-06 21:09 PST, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (215.12 KB, patch)
2015-01-06 23:41 PST, Darin Adler
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-mountainlion (213.18 KB, application/zip)
2015-01-07 00:43 PST, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-mountainlion-wk2 (213.77 KB, application/zip)
2015-01-07 00:46 PST, Build Bot
no flags Details
Patch (218.46 KB, patch)
2015-01-07 20:17 PST, Darin Adler
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-mountainlion (444.33 KB, application/zip)
2015-01-07 21:23 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-mountainlion-wk2 (670.86 KB, application/zip)
2015-01-07 21:32 PST, Build Bot
no flags Details
Patch (218.60 KB, patch)
2015-01-08 09:22 PST, Darin Adler
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2015-01-06 19:28:59 PST
Modernize and streamline HTMLTokenizer
Comment 1 Darin Adler 2015-01-06 21:09:55 PST
Created attachment 244138 [details]
Patch
Comment 2 Darin Adler 2015-01-06 23:41:07 PST
Created attachment 244142 [details]
Patch
Comment 3 WebKit Commit Bot 2015-01-06 23:44:01 PST
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 4 Build Bot 2015-01-07 00:43:30 PST
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.
Comment 5 Build Bot 2015-01-07 00:43:34 PST
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 6 Build Bot 2015-01-07 00:46:23 PST
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.
Comment 7 Build Bot 2015-01-07 00:46:27 PST
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
Comment 8 Darin Adler 2015-01-07 20:17:10 PST
Created attachment 244237 [details]
Patch
Comment 9 WebKit Commit Bot 2015-01-07 20:18:58 PST
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 10 Build Bot 2015-01-07 21:23:33 PST
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.
Comment 11 Build Bot 2015-01-07 21:23:37 PST
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 12 Build Bot 2015-01-07 21:32:09 PST
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.
Comment 13 Build Bot 2015-01-07 21:32:12 PST
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 14 Darin Adler 2015-01-08 08:25:22 PST
Comment on attachment 244237 [details]
Patch

Let me fix the XSS auditor now.
Comment 15 Darin Adler 2015-01-08 09:22:24 PST
Created attachment 244265 [details]
Patch
Comment 16 WebKit Commit Bot 2015-01-08 09:25:24 PST
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.
Comment 17 Darin Adler 2015-01-08 21:07:21 PST
Committed r178154: <http://trac.webkit.org/changeset/178154>
Comment 18 Csaba Osztrogonác 2015-01-09 00:03:57 PST
(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.
Comment 19 Csaba Osztrogonác 2015-01-09 00:14:45 PST
(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?
Comment 20 Csaba Osztrogonác 2015-01-09 00:15:29 PST
(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
Comment 21 Alexey Proskuryakov 2015-01-09 09:37:41 PST
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
Comment 22 WebKit Commit Bot 2015-01-09 09:40:53 PST
Re-opened since this is blocked by bug 140292
Comment 23 Alexey Proskuryakov 2015-01-09 09:46:20 PST
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?
Comment 24 Darin Adler 2015-01-10 18:57:04 PST
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.
Comment 25 Darin Adler 2015-01-12 08:22:48 PST
Committed r178265: <http://trac.webkit.org/changeset/178265>
Comment 26 Daniel Bates 2016-10-25 13:50:16 PDT
(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.