Custom properties must preserve their input exactly for serialization |
|||||||||||||||
Issue descriptionThe CSSWG recently resolved that "custom properties must preserve their input exactly for serialization". Our implementation currently serializes the parser tokens, although doesn't yet handle many of the edge cases like comment insertion. Serializing tokens leads to some potentially unexpected behaviour if custom properties are just used to store text, rather than as CSS. Firefox already preserves the user input (adding closing brackets as required), while Safari currently serializes a parsed representation. Example test case: https://jsfiddle.net/6ogth0dj/ https://logs.csswg.org/irc.w3.org/css/2016-09-19/#e722375 https://drafts.csswg.org/css-syntax/#serialization [forked from bug 617694 ]
,
Nov 7 2016
,
Nov 9 2016
,
Nov 11 2016
,
Jan 9 2017
,
Jan 24 2017
I'm not working on this any more, assigning to the style team TL, meade@, to triage. I also updated the doc since I posted the link above with some more details a couple weeks ago.
,
Jan 25 2017
So it doesn't get lost, I was previously working on the first part of this (streaming parser): https://codereview.chromium.org/2503683003/
,
Feb 13 2017
,
Mar 6 2017
,
Mar 8 2017
,
Apr 18 2017
,
Aug 1 2017
Taking this over from timloh@.
,
Aug 1 2017
The plan going forward is that timloh's WIP patch will be split into smaller pieces that are easier to test and review. Ideally we won't hit any perf regression along the way.
,
Aug 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7aaef89230f0d9bce34530df37b11ad9b34227b7 commit 7aaef89230f0d9bce34530df37b11ad9b34227b7 Author: Darren Shen <shend@chromium.org> Date: Fri Aug 04 06:12:58 2017 [CSSParser] Do not tokenize in CSSTokenizer constructor. Currently, when a CSSTokenizer is created from a String, the string is completely tokenized in the constructor. In order to make the css parser tokenize on demand, we do not tokenize everything in the constructor. Instead, we introduce a new class called CSSParserTokenStream that acts a streaming interface to the tokenizer. Eventually, this will have methods such as "Next" and "Peek" that tokenize on demand. For now, it just has a method to fully tokenize the input and returns the range. Hence, this patch simply moves full string tokenization from CSSTokenizer constructor to CSSParserTokenStream::MakeRangeToEOF(). The plan is to eventually replace (most) CSSParserTokenRanges with CSSParserTokenStreams. This patch replaces some of the top level parsing functions to use a CSSParserTokenStream. To call lower level parsing functions that use ranges, we convert these streams to ranges, which triggers a full tokenization. This is a temporary fallback to the current range system. Future patches will change those level parsing functions to use streams as well, layer by layer, so we won't need to trigger full tokenizations. Note that only one of the two constructors are affected by this patch. The other constructor uses observers and is more difficult to change. This patch does not change observable behaviour, and should not have any performance impacts (since we're just delaying when the string gets fully tokenized). Bug: 661854 Change-Id: I6f20269835d806106e95740f8dcee6744d0de944 Reviewed-on: https://chromium-review.googlesource.com/595025 Reviewed-by: nainar <nainar@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#491950} [modify] https://crrev.com/7aaef89230f0d9bce34530df37b11ad9b34227b7/third_party/WebKit/Source/core/css/BUILD.gn [modify] https://crrev.com/7aaef89230f0d9bce34530df37b11ad9b34227b7/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/7aaef89230f0d9bce34530df37b11ad9b34227b7/third_party/WebKit/Source/core/css/parser/CSSParserToken.h [add] https://crrev.com/7aaef89230f0d9bce34530df37b11ad9b34227b7/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/7aaef89230f0d9bce34530df37b11ad9b34227b7/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/7aaef89230f0d9bce34530df37b11ad9b34227b7/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h
,
Aug 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/726787e05f41c9103250fad8b85881544365f77f commit 726787e05f41c9103250fad8b85881544365f77f Author: Darren Shen <shend@chromium.org> Date: Fri Aug 11 01:47:27 2017 [CSSParser] Add Next/Peek for stream parsing of ConsumeDeclarationList. This patch adds Next/Peek methods on CSSParserTokenStream, which tokenizes on demand. It also offers fast path UncheckedNext/Peek methods that can be used when we've already called Peek(). Using these new features, we can begin to convert CSSParserImpl:: ConsumeDeclarationList to use streams (we add an overload because there are still other callers that use ranges). Unfortunately, using CSSParserObservers with streaming requires some refactoring, so for now, we only call ConsumeDeclarationList when we don't have an observer. Since ConsumeDeclarationList calls lower level parsing functions that require ranges, we convert streams to ranges using MakeRangeToEOF(). When those lower level parsing functions modify the ranges that they take, we need to sync those changes with the stream using UpdatePositionFromRange. Future patches will continue to streamify those lower level parsing functions, layer by layer. This patch does not change observable parsing behaviour, and should have minimal performance impact: ranges are inherently faster than streams, but the unchecked consume/peeks should close the gap. For reviewers: - CSSParserTokenStream::ConsumeComponentValue is copied from CSSParserTokenRange::ConsumeComponentValue. - The streaming CSSParserTokenStream::ConsumeDeclarationList is adapted from the ranges version. Bug: 661854 Change-Id: Id2732f7030d012daf213f963ed0e0506d6eed145 Reviewed-on: https://chromium-review.googlesource.com/597607 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Reviewed-by: Bugs Nash <bugsnash@chromium.org> Cr-Commit-Position: refs/heads/master@{#493640} [modify] https://crrev.com/726787e05f41c9103250fad8b85881544365f77f/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/726787e05f41c9103250fad8b85881544365f77f/third_party/WebKit/Source/core/css/BUILD.gn [modify] https://crrev.com/726787e05f41c9103250fad8b85881544365f77f/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/726787e05f41c9103250fad8b85881544365f77f/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [add] https://crrev.com/726787e05f41c9103250fad8b85881544365f77f/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.cpp [modify] https://crrev.com/726787e05f41c9103250fad8b85881544365f77f/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [add] https://crrev.com/726787e05f41c9103250fad8b85881544365f77f/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp
,
Aug 11 2017
,
Aug 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d39de8fbe45814b9b4c75b41f85e472c103f68e8 commit d39de8fbe45814b9b4c75b41f85e472c103f68e8 Author: Kenneth Russell <kbr@chromium.org> Date: Fri Aug 11 18:22:58 2017 Revert "[CSSParser] Add Next/Peek for stream parsing of ConsumeDeclarationList." This reverts commit 726787e05f41c9103250fad8b85881544365f77f. Reason for revert: caused assertion failures in CSSParserImpl.cpp. See Issue 754739 . Original change's description: > [CSSParser] Add Next/Peek for stream parsing of ConsumeDeclarationList. > > This patch adds Next/Peek methods on CSSParserTokenStream, which > tokenizes on demand. It also offers fast path UncheckedNext/Peek methods > that can be used when we've already called Peek(). > > Using these new features, we can begin to convert CSSParserImpl:: > ConsumeDeclarationList to use streams (we add an overload because there > are still other callers that use ranges). Unfortunately, using > CSSParserObservers with streaming requires some refactoring, so for now, > we only call ConsumeDeclarationList when we don't have an observer. > > Since ConsumeDeclarationList calls lower level parsing functions that > require ranges, we convert streams to ranges using MakeRangeToEOF(). > When those lower level parsing functions modify the ranges that they > take, we need to sync those changes with the stream using > UpdatePositionFromRange. > > Future patches will continue to streamify those lower level parsing > functions, layer by layer. > > This patch does not change observable parsing behaviour, and should > have minimal performance impact: ranges are inherently faster than > streams, but the unchecked consume/peeks should close the gap. > > For reviewers: > - CSSParserTokenStream::ConsumeComponentValue is copied from > CSSParserTokenRange::ConsumeComponentValue. > - The streaming CSSParserTokenStream::ConsumeDeclarationList is adapted > from the ranges version. > > Bug: 661854 > Change-Id: Id2732f7030d012daf213f963ed0e0506d6eed145 > Reviewed-on: https://chromium-review.googlesource.com/597607 > Commit-Queue: Darren Shen <shend@chromium.org> > Reviewed-by: meade_UTC10 <meade@chromium.org> > Reviewed-by: Bugs Nash <bugsnash@chromium.org> > Cr-Commit-Position: refs/heads/master@{#493640} TBR=meade@chromium.org,shend@chromium.org,bugsnash@chromium.org Change-Id: If0244556e6746f0cba43148575f0512a190998f5 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 661854 Reviewed-on: https://chromium-review.googlesource.com/612371 Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#493819} [modify] https://crrev.com/d39de8fbe45814b9b4c75b41f85e472c103f68e8/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/d39de8fbe45814b9b4c75b41f85e472c103f68e8/third_party/WebKit/Source/core/css/BUILD.gn [modify] https://crrev.com/d39de8fbe45814b9b4c75b41f85e472c103f68e8/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/d39de8fbe45814b9b4c75b41f85e472c103f68e8/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [delete] https://crrev.com/0eb2231add3b155a475d9dcd77f939e89484c226/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.cpp [modify] https://crrev.com/d39de8fbe45814b9b4c75b41f85e472c103f68e8/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [delete] https://crrev.com/0eb2231add3b155a475d9dcd77f939e89484c226/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp
,
Aug 11 2017
Sorry, but 726787e05f41c9103250fad8b85881544365f77f caused assertion failures inside the CSS parser per Issue 754739 and I've reverted it in https://chromium-review.googlesource.com/612371 . The chromium.gpu.fyi bots are currently compiling with MSVC rather than Clang, and that may be the reason for the difference compared to the normal CQ bots. You can prevent this regression from happening again by adding the following line to your CL description when relanding: CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel +thakis because this is another MSVC/Clang difference in the CSS code.
,
Aug 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/655f24fcfa2938dcbf8f774be7c8db88f031e935 commit 655f24fcfa2938dcbf8f774be7c8db88f031e935 Author: Darren Shen <shend@chromium.org> Date: Wed Aug 16 01:00:23 2017 Revert "Revert "[CSSParser] Add Next/Peek for stream parsing of ConsumeDeclarationList."" This reverts commit d39de8fbe45814b9b4c75b41f85e472c103f68e8. Reason for revert: Relanding Change: Fixed undefined behaviour due to unspecified order of operations with side effects (compare Patch 5 with Patch 1). Original change's description: > Revert "[CSSParser] Add Next/Peek for stream parsing of ConsumeDeclarationList." > > This reverts commit 726787e05f41c9103250fad8b85881544365f77f. > > Reason for revert: caused assertion failures in CSSParserImpl.cpp. See Issue 754739 . > > Original change's description: > > [CSSParser] Add Next/Peek for stream parsing of ConsumeDeclarationList. > > > > This patch adds Next/Peek methods on CSSParserTokenStream, which > > tokenizes on demand. It also offers fast path UncheckedNext/Peek methods > > that can be used when we've already called Peek(). > > > > Using these new features, we can begin to convert CSSParserImpl:: > > ConsumeDeclarationList to use streams (we add an overload because there > > are still other callers that use ranges). Unfortunately, using > > CSSParserObservers with streaming requires some refactoring, so for now, > > we only call ConsumeDeclarationList when we don't have an observer. > > > > Since ConsumeDeclarationList calls lower level parsing functions that > > require ranges, we convert streams to ranges using MakeRangeToEOF(). > > When those lower level parsing functions modify the ranges that they > > take, we need to sync those changes with the stream using > > UpdatePositionFromRange. > > > > Future patches will continue to streamify those lower level parsing > > functions, layer by layer. > > > > This patch does not change observable parsing behaviour, and should > > have minimal performance impact: ranges are inherently faster than > > streams, but the unchecked consume/peeks should close the gap. > > > > For reviewers: > > - CSSParserTokenStream::ConsumeComponentValue is copied from > > CSSParserTokenRange::ConsumeComponentValue. > > - The streaming CSSParserTokenStream::ConsumeDeclarationList is adapted > > from the ranges version. > > > > Bug: 661854 > > Change-Id: Id2732f7030d012daf213f963ed0e0506d6eed145 > > Reviewed-on: https://chromium-review.googlesource.com/597607 > > Commit-Queue: Darren Shen <shend@chromium.org> > > Reviewed-by: meade_UTC10 <meade@chromium.org> > > Reviewed-by: Bugs Nash <bugsnash@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#493640} > > TBR=meade@chromium.org,shend@chromium.org,bugsnash@chromium.org > > Change-Id: If0244556e6746f0cba43148575f0512a190998f5 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 661854 > Reviewed-on: https://chromium-review.googlesource.com/612371 > Reviewed-by: Kenneth Russell <kbr@chromium.org> > Commit-Queue: Kenneth Russell <kbr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#493819} TBR=meade@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I497adee4374828b1e8040a2e0c11745b64a5c4d0 Bug: 661854 Reviewed-on: https://chromium-review.googlesource.com/611884 Reviewed-by: Darren Shen <shend@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#494646} [modify] https://crrev.com/655f24fcfa2938dcbf8f774be7c8db88f031e935/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/655f24fcfa2938dcbf8f774be7c8db88f031e935/third_party/WebKit/Source/core/css/BUILD.gn [modify] https://crrev.com/655f24fcfa2938dcbf8f774be7c8db88f031e935/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/655f24fcfa2938dcbf8f774be7c8db88f031e935/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [add] https://crrev.com/655f24fcfa2938dcbf8f774be7c8db88f031e935/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.cpp [modify] https://crrev.com/655f24fcfa2938dcbf8f774be7c8db88f031e935/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [add] https://crrev.com/655f24fcfa2938dcbf8f774be7c8db88f031e935/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp
,
Aug 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7220fb9e1b829a9275b878b6fa499e55c4999cb9 commit 7220fb9e1b829a9275b878b6fa499e55c4999cb9 Author: Darren Shen <shend@chromium.org> Date: Wed Aug 16 23:07:59 2017 [CSSParser] Allow parsers with observers to use streams. Previously, we have CSSParserObservers which receive information about the parsing process (e.g. where a particular rule starts or ends). The way it worked for stylesheet parsing was that we first tokenized the entirety of the input, storing token information in a vector. Then, as we parsed, we queried the tokens for their location in the input string (their offset) using the vector. To get the same behaviour with streams, it's more difficult. Before, offset information were available prior to parsing. Now, offset information is only available as we parse. This requires some refactoring to work. In the meantime, we would like parsers with observers to still use streams. Hence, this patch implements a workaround: when we have observers, we tokenize everything before parsing as we did before. We then create a stream over that tokenizer (technically no longer a stream since everything's already tokenized) and use the same parsing code path. Any time we call into the observer with a range, we just convert the stream back into a range. This means that until we implement proper streaming with observers, parsing with observers will tokenize up front, whereas parsing without observers will tokenize on demand (mostly). For this to work, we also have to relax the requirement that streams must use 'fresh' tokenizers with no consumed tokens, since streams with observers requires using tokenizers that have already consumed the entirety of the input. Turns out this doesn't require any changes other than removing a DCHECK. For reviewers: - We add back the observer code that was in ConsumeDeclarationList but was deleted because observers didn't accept streams before. Bug: 661854 Change-Id: Ifa53b2e7f2622edb371c74260fd24287d0fd4f89 Reviewed-on: https://chromium-review.googlesource.com/599467 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Reviewed-by: Bugs Nash <bugsnash@chromium.org> Cr-Commit-Position: refs/heads/master@{#494989} [modify] https://crrev.com/7220fb9e1b829a9275b878b6fa499e55c4999cb9/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/7220fb9e1b829a9275b878b6fa499e55c4999cb9/third_party/WebKit/Source/core/css/parser/CSSParserObserverWrapper.h [modify] https://crrev.com/7220fb9e1b829a9275b878b6fa499e55c4999cb9/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/7220fb9e1b829a9275b878b6fa499e55c4999cb9/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp [modify] https://crrev.com/7220fb9e1b829a9275b878b6fa499e55c4999cb9/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp
,
Aug 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08801673b73f9a7bb098fc0988e821f02b036587 commit 08801673b73f9a7bb098fc0988e821f02b036587 Author: Darren Shen <shend@chromium.org> Date: Thu Aug 17 08:02:37 2017 [CSSParser] Add iterators to CSSParserTokenStream. We often want to get a CSSParserTokenRange to a particular subsequence of tokens. For example, in ConsumeDeclarationList, we want to get a range to the "declaration start" section and pass that down to ConsumeDeclaration. The canonical way of doing this is to expose iterators: we record where the declaration starts in an iterator, consume some values, record where the declaration ends, and get a range between those two iterators. Currently, we use pointers to CSSParserToken as iterators. This was fine before because we prefilled the tokens vector, so all iterators are guaranteed to remain valid while parsing. However, when we use streams, we incrementally push tokens to the tokens vector, which may trigger a resize, thus invalidating all the iterators. Hence, we create our own stable iterator in CSSParserTokenStream that does not get invalidated because it's based on indices and not pointers. Bug: 661854 Change-Id: I8aaf002de398101f20feda1e5a685707b76e3a35 Reviewed-on: https://chromium-review.googlesource.com/599267 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Cr-Commit-Position: refs/heads/master@{#495102} [modify] https://crrev.com/08801673b73f9a7bb098fc0988e821f02b036587/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/08801673b73f9a7bb098fc0988e821f02b036587/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/08801673b73f9a7bb098fc0988e821f02b036587/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp [modify] https://crrev.com/08801673b73f9a7bb098fc0988e821f02b036587/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/08801673b73f9a7bb098fc0988e821f02b036587/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/006fe05062cd9696cd11f2cb607c92f577471b7d commit 006fe05062cd9696cd11f2cb607c92f577471b7d Author: Darren Shen <shend@chromium.org> Date: Mon Aug 21 00:04:28 2017 [CSSParser] Allow stream parsing for ConsumeAtRule. This patch adds an overload to CSSParserImpl::ConsumeAtRule which takes streams instead of ranges. The reason that we're implementing streams for ConsumeAtRule first is that it provides a use case for streaming block parsing. Once we implemented streaming block parsing for ConsumeAtRule, we will be in a position to convert the remaining parsing functions to streams in one go. For reviewers: - The new ConsumeAtRule is copied from the old one that takes ranges, with some changes to make it work with streams. - ConsumeWhitespace and ConsumeIncludingWhitespace are copied from CSSParserTokenRange. Bug: 661854 Change-Id: I87b3e2aec9854224eb8503a7e4fcebc58cd3feb5 Reviewed-on: https://chromium-review.googlesource.com/601131 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Cr-Commit-Position: refs/heads/master@{#495856} [modify] https://crrev.com/006fe05062cd9696cd11f2cb607c92f577471b7d/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/006fe05062cd9696cd11f2cb607c92f577471b7d/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [modify] https://crrev.com/006fe05062cd9696cd11f2cb607c92f577471b7d/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.cpp [modify] https://crrev.com/006fe05062cd9696cd11f2cb607c92f577471b7d/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/006fe05062cd9696cd11f2cb607c92f577471b7d/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5819e64ac263e30c62fe1e2b3ebb565665ad3035 commit 5819e64ac263e30c62fe1e2b3ebb565665ad3035 Author: Darren Shen <shend@chromium.org> Date: Mon Aug 21 22:32:11 2017 [CSSParser] Fix CSSParserObserverWrapper workaround. In [1], we introduced a temporary workaround that allowed CSSParserObserverWrapper to take streams as well as ranges. However, there was a bug in that patch. We used MakeRangeToEOF() to convert the stream to a range, but that means the converted range would end at EOF. While this was fine for CSSParserObserverWrapper::StartOffset, which only used the the starting point of the range, it was broken for CSSParserObserverWrapper::EndOffset, which used the ending point. It's quite tricky to get the "end point" of a stream, since it's not well defined. We looked at usages of CSSParserObserverWrapper::EndOffset and saw that they were only called when parsing a range is successful (i.e. the range was fully consumed). In that scenario, the end point of the range is equal to the start point of the range. Hence, the correct end point is really just the position of the stream when we call EndOffset. Since the observer wrapper only looks at either the starting or ending point of the range, we can just pass it an empty range with starting and ending points set to the correct value (the current position of the stream). [1] https://chromium-review.googlesource.com/c/599467 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel Bug: 661854 Change-Id: I191dbf6946dc7b3b1a6e81b11d2cf95ef3b72b33 Reviewed-on: https://chromium-review.googlesource.com/622331 Reviewed-by: nainar <nainar@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#496089} [modify] https://crrev.com/5819e64ac263e30c62fe1e2b3ebb565665ad3035/third_party/WebKit/Source/core/css/parser/CSSParserObserverWrapper.h [modify] https://crrev.com/5819e64ac263e30c62fe1e2b3ebb565665ad3035/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h
,
Aug 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/26d5c0d9218faabcdf678e29c867ee90a971efe2 commit 26d5c0d9218faabcdf678e29c867ee90a971efe2 Author: Darren Shen <shend@chromium.org> Date: Thu Aug 24 04:38:42 2017 [CSSParser] Do not invalidate ranges when consuming from streams. This patch makes ranges stable: they do not get invalidated when the backing vector resizes. This is done by keeping a reference to the backing vector. To make this work, we had to change CSSTokenizer to not use an inline buffer size. This may have a small performance cost for short strings. We add a unit test for this (which fails without these changes). CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel Bug: 661854 Change-Id: Idaa2478abe77f13b1fcd17ad11c8879f84795480 Reviewed-on: https://chromium-review.googlesource.com/625552 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Cr-Commit-Position: refs/heads/master@{#496951} [modify] https://crrev.com/26d5c0d9218faabcdf678e29c867ee90a971efe2/third_party/WebKit/Source/core/css/parser/CSSParserTokenRange.cpp [modify] https://crrev.com/26d5c0d9218faabcdf678e29c867ee90a971efe2/third_party/WebKit/Source/core/css/parser/CSSParserTokenRange.h [modify] https://crrev.com/26d5c0d9218faabcdf678e29c867ee90a971efe2/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp [modify] https://crrev.com/26d5c0d9218faabcdf678e29c867ee90a971efe2/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h
,
Sep 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16c95656c2e3d266d10fd2af768b87c9b6574ce2 commit 16c95656c2e3d266d10fd2af768b87c9b6574ce2 Author: Darren Shen <shend@chromium.org> Date: Mon Sep 04 00:42:39 2017 [CSSparser] Implement stream block parsing for ConsumeFontFaceRule. This patch implements the stream parsing of CSS blocks (tokens wrapped by '(' or '{' or '['). Stream block parsing solves the following issue: When we parse with ranges, we consume a block in its entirety and then pass that block to another function which will do a second pass over that range. This means that even if the block contained invalid syntax, the whole block will still be consumed. This allows us to continue parsing the rest of the stylesheet without issues (error recovery). If we simply substituted ranges with streams in this case, we would get a different result. When we encounter an error, the rest of the block would not be consumed yet. To fix this, we conceptually change these streams to be 'block streams', meaning that they can only parse the inner contents of a single block. This means it is a runtime error to consume a token representing an open block, and the stream ends at EOF or if the next token closes the block. We also need to add internal peek/consume methods that do not have these checks (i.e. that have the old behaviour). To parse nested blocks, you must create a BlockGuard. Upon creation, the BlockGuard would advance the stream past the open block token. When the BlockGuard goes out of scope, error recovery kicks in and we ensure that the rest of the block is consumed using the BlockGuard destructor. We test this on ConsumeFontFaceRule (because it is one of the simpler functions that use blocks). Unfortunately, we need to keep both the stream and range code paths temporarily to keep the CL small. We also had to overload ConsumeFontFaceRule for streams, but the range overload will be removed in the future. Bug: 661854 Change-Id: I4dcc7e20c8eb62c23b589277556960dcb85eca70 Reviewed-on: https://chromium-review.googlesource.com/601133 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Cr-Commit-Position: refs/heads/master@{#499422} [modify] https://crrev.com/16c95656c2e3d266d10fd2af768b87c9b6574ce2/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/16c95656c2e3d266d10fd2af768b87c9b6574ce2/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [modify] https://crrev.com/16c95656c2e3d266d10fd2af768b87c9b6574ce2/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.cpp [modify] https://crrev.com/16c95656c2e3d266d10fd2af768b87c9b6574ce2/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/16c95656c2e3d266d10fd2af768b87c9b6574ce2/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp
,
Sep 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1717b6b46acfdddc31580854f3be47171ef004c0 commit 1717b6b46acfdddc31580854f3be47171ef004c0 Author: Darren Shen <shend@chromium.org> Date: Mon Sep 04 04:17:55 2017 [CSSParser] Add offset information to CSSParserTokenStream. One of the benefits of streaming parser is that we can get token offset information (where a token starts in the input string) on the fly. This patch adds CSSParserTokenStream::Offset() which returns the index in the input string of the next token to be consumed. Naively, we would implement this by querying CSSTokenizerParserInputStream::Offset(). However, CSSTokenizerInputStream::Offset() might be ahead of CSSParserTokenStream::Offset() because peeking a token would advance CSSTokenizerInputStream but not CSSParserTokenStream, so some extra logic is required to handle this. Note that this method is not used (except for tests) until later, but we put it in this patch to avoid large patches. Bug: 661854 Change-Id: I67ceaeb4cb16345454b1ccf92c103401996fd298 Reviewed-on: https://chromium-review.googlesource.com/615857 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: Bugs Nash <bugsnash@chromium.org> Cr-Commit-Position: refs/heads/master@{#499434} [modify] https://crrev.com/1717b6b46acfdddc31580854f3be47171ef004c0/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/1717b6b46acfdddc31580854f3be47171ef004c0/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp
,
Sep 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/25f72cb089f2be7f89c2dfedf2e8693ab2e2b5be commit 25f72cb089f2be7f89c2dfedf2e8693ab2e2b5be Author: Darren Shen <shend@chromium.org> Date: Tue Sep 05 09:07:09 2017 [CSSParser] Remove tokenization tracing metrics for stylesheet parsing. When we eventually switch to a streaming parser, tokenization will be part of parsing, so it is difficult to actually track how much time tokenization takes. We could wrap CSSTokenizer::TokenizeSingle() with tracing, but it is unclear if it's bad practice to put tracing calls into a frequently called function. For the lack of a better solution, we are just going to remove the tokenization metrics from stylesheet parsing. We do this in its own patch for clarity. Bug: 661854 Change-Id: Iabef67f4a34ac7d8568a0e1b3d827644c0885f28 Reviewed-on: https://chromium-review.googlesource.com/619812 Reviewed-by: dstockwell <dstockwell@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#499588} [modify] https://crrev.com/25f72cb089f2be7f89c2dfedf2e8693ab2e2b5be/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp
,
Sep 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca7c1bf8eb877f562f5a38e745aea68874876d81 commit ca7c1bf8eb877f562f5a38e745aea68874876d81 Author: Darren Shen <shend@chromium.org> Date: Tue Sep 05 23:47:45 2017 [CSSParser] Switch some parsing functions to use streams. This patch switches the majority of functions in CSSParserImpl to use streams instead of ranges. The functions we picked correspond to the original patch in https://codereview.chromium.org/2503683003. To note: - We delete some duplicate parsing functions that use ranges since they are no longer used. - For lazy parsing to work with streams, we store the offset of where the block (to be lazily parsed) starts. When we decide to parse the block, we create a new stream starting at that offset. Note that we are still eagerly tokenizing the block. In the future, we will skip the block entirely without tokenizing. - We add ConsumeDeclarationListForAtApply, which is like a special case of ConsumeDeclarationList that works on ranges and only for @apply rules. The code is from https://codereview.chromium.org/2503683003 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel Bug: 661854 Change-Id: I994985459afbde7a992760094d9849f952477e72 Reviewed-on: https://chromium-review.googlesource.com/620290 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Reviewed-by: nainar <nainar@chromium.org> Cr-Commit-Position: refs/heads/master@{#499803} [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSLazyParsingTest.cpp [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.cpp [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.h [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/ca7c1bf8eb877f562f5a38e745aea68874876d81/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h
,
Sep 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f181c11ae2db3aac9abadb2fce12682677c23430 commit f181c11ae2db3aac9abadb2fce12682677c23430 Author: Darren Shen <shend@chromium.org> Date: Thu Sep 07 09:44:10 2017 Revert "[CSSParser] Switch some parsing functions to use streams." This reverts commit ca7c1bf8eb877f562f5a38e745aea68874876d81. Reason for revert: Performance regression crbug.com/762209 Original change's description: > [CSSParser] Switch some parsing functions to use streams. > > This patch switches the majority of functions in CSSParserImpl to use > streams instead of ranges. The functions we picked correspond to the > original patch in https://codereview.chromium.org/2503683003. > > To note: > > - We delete some duplicate parsing functions that use ranges since they > are no longer used. > > - For lazy parsing to work with streams, we store the offset of where > the block (to be lazily parsed) starts. When we decide to parse the > block, we create a new stream starting at that offset. Note that we > are still eagerly tokenizing the block. In the future, we will skip > the block entirely without tokenizing. > > - We add ConsumeDeclarationListForAtApply, which is like a special case of > ConsumeDeclarationList that works on ranges and only for @apply rules. > The code is from https://codereview.chromium.org/2503683003 > > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel > > Bug: 661854 > Change-Id: I994985459afbde7a992760094d9849f952477e72 > Reviewed-on: https://chromium-review.googlesource.com/620290 > Commit-Queue: Darren Shen <shend@chromium.org> > Reviewed-by: meade_UTC10 <meade@chromium.org> > Reviewed-by: nainar <nainar@chromium.org> > Cr-Commit-Position: refs/heads/master@{#499803} TBR=nainar@chromium.org,meade@chromium.org,shend@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 661854 Change-Id: I254115563a146c8d6e2f48f8e111e85baaa3f8ba Cq-Include-Trybots: master.tryserver.chromium.win:win_optional_gpu_tests_rel Reviewed-on: https://chromium-review.googlesource.com/654538 Reviewed-by: Darren Shen <shend@chromium.org> Reviewed-by: meade_UTC10 <meade@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#500259} [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSLazyParsingTest.cpp [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.cpp [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.h [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/f181c11ae2db3aac9abadb2fce12682677c23430/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h
,
Sep 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34de07a7da1921205306d2f6299187dcddd42d2d commit 34de07a7da1921205306d2f6299187dcddd42d2d Author: Darren Shen <shend@chromium.org> Date: Thu Sep 28 13:43:01 2017 [CSSParser] Use streaming parser for CSS parsing. This is a squashed commit of the following five CLs: https://chromium-review.googlesource.com/c/656637/ https://chromium-review.googlesource.com/c/642666/ https://chromium-review.googlesource.com/c/656657/ https://chromium-review.googlesource.com/c/663102/ https://chromium-review.googlesource.com/c/665778/ We did this to make it easier to revert if anything goes wrong. All five CLs have been LGTM'd. No significant performance regression is expected from the patch. Bug: 661854 Change-Id: I5f08b4bebcb687488fb43183a490b607512f3910 Reviewed-on: https://chromium-review.googlesource.com/676366 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: nainar <nainar@chromium.org> Reviewed-by: dstockwell <dstockwell@chromium.org> Reviewed-by: Eric Willigers <ericwilligers@chromium.org> Cr-Commit-Position: refs/heads/master@{#505005} [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/animation/CSSInterpolationType.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/BUILD.gn [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/PropertyRegistration.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSLazyParsingTest.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.h [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParser.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserObserverWrapper.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserObserverWrapper.h [add] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserScopedTokenBuffer.h [add] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserTokenBuffer.h [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserTokenRange.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserTokenRange.h [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/CSSTokenizerTest.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/MediaConditionTest.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/MediaQueryParser.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/SizesAttributeParser.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/css/parser/SizesCalcParserTest.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/inspector/InspectorStyleSheet.cpp [modify] https://crrev.com/34de07a7da1921205306d2f6299187dcddd42d2d/third_party/WebKit/Source/core/intersection_observer/IntersectionObserver.cpp
,
Sep 29 2017
,
Oct 3 2017
Still waiting for any perf and clusterfuzz reports to come in. Will consider the patch safely landed if no more reports at the end of this week (fingers crossed!)
,
Oct 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ab1291fb2b682683463ca51ed3e616af39c0926f commit ab1291fb2b682683463ca51ed3e616af39c0926f Author: Darren Shen <shend@chromium.org> Date: Tue Oct 17 06:19:00 2017 [CSSParser] Use streaming parser for selector parsing. This patch change selector parsing to use streams. The advantage is two fold: - We no longer have to allocate memory for long selector preludes, e.g. .a, .b, .c, .d, .e .... { }. This is useful for websites that minify their CSS, as the minified CSS tend to form very long selector lists. - This is the last use of CSSParserObserverWrapper, so we can remove it in a future patch. To use streaming, we have to be careful with error recovery. If an error occurs in the prelude, we still have to skip the rest of the block. For reviewers: - Most of this code is from the original patch in [1]. - The 'new' code in CSSSelectorParser is pretty much just the streaming version of the Range based version of ConsumeComplexSelectorList. [1] https://codereview.chromium.org/2503683003/ Bug: 661854 Change-Id: Ied369ffba695f15baf347591e833aaec40080662 Reviewed-on: https://chromium-review.googlesource.com/676888 Reviewed-by: meade_UTC10 <meade@chromium.org> Reviewed-by: nainar <nainar@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#509297} [modify] https://crrev.com/ab1291fb2b682683463ca51ed3e616af39c0926f/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/ab1291fb2b682683463ca51ed3e616af39c0926f/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [modify] https://crrev.com/ab1291fb2b682683463ca51ed3e616af39c0926f/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/ab1291fb2b682683463ca51ed3e616af39c0926f/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp [modify] https://crrev.com/ab1291fb2b682683463ca51ed3e616af39c0926f/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h [modify] https://crrev.com/ab1291fb2b682683463ca51ed3e616af39c0926f/third_party/WebKit/Source/core/inspector/InspectorStyleSheet.cpp
,
Oct 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9048063d3ef0e7635882d37a9df8916a91fbbac3 commit 9048063d3ef0e7635882d37a9df8916a91fbbac3 Author: Darren Shen <shend@chromium.org> Date: Mon Oct 30 01:29:13 2017 [CSSParser] Delete CSSParserObserverWrapper. CSSParserObserverWrapper was used as an interface between CSSParserObserver and CSSParserTokenRanges for inspector parsing. Since we no longer use CSSParserTokenRanges for inspector parsing, we can delete this class and use CSSParserObserver directly. Bug: 661854 Change-Id: I099e2db34437ce666274508396f1e6eb9915382c Reviewed-on: https://chromium-review.googlesource.com/688974 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: Renée Wright <rjwright@chromium.org> Cr-Commit-Position: refs/heads/master@{#512430} [modify] https://crrev.com/9048063d3ef0e7635882d37a9df8916a91fbbac3/third_party/WebKit/Source/core/css/BUILD.gn [modify] https://crrev.com/9048063d3ef0e7635882d37a9df8916a91fbbac3/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/9048063d3ef0e7635882d37a9df8916a91fbbac3/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [delete] https://crrev.com/9c310d7826c99786affa4c0704c6071a90b09b4c/third_party/WebKit/Source/core/css/parser/CSSParserObserverWrapper.cpp [delete] https://crrev.com/9c310d7826c99786affa4c0704c6071a90b09b4c/third_party/WebKit/Source/core/css/parser/CSSParserObserverWrapper.h [modify] https://crrev.com/9048063d3ef0e7635882d37a9df8916a91fbbac3/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.cpp [modify] https://crrev.com/9048063d3ef0e7635882d37a9df8916a91fbbac3/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/9048063d3ef0e7635882d37a9df8916a91fbbac3/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp [modify] https://crrev.com/9048063d3ef0e7635882d37a9df8916a91fbbac3/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h [modify] https://crrev.com/9048063d3ef0e7635882d37a9df8916a91fbbac3/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp
,
Nov 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/93b5990b8beca44d533e66b959d4baafdc0b7dad commit 93b5990b8beca44d533e66b959d4baafdc0b7dad Author: Darren Shen <shend@chromium.org> Date: Tue Nov 07 04:57:50 2017 [CSSParser] Simplify token lifetime management. Most of this patch is from the original patch: https://codereview.chromium.org/2503683003/ The way we currently manage token lifetimes in the parser is a bit complicated. This patch tries to simplify this by taking advantage of two assumptions which currently hold: - The tokens for only one prelude or declaration needs to be stored at any point. - We never need to 'append' tokens to a Range. With these assumptions, we can introduce a new method called ConsumeUntilPeekedIs, which consumes tokens until the peeked token type is one of the specified ones. These tokens are stored in a vector and kept alive until the next call to ConsumeUntilPeekedIs. This dramatically simplifies token lifetime management and allows us to delete CSSParserScopedTokenRange and move CSSParserTokenRange to be a nested class in CSSParserTokenStream. Bug: 661854 Change-Id: I33bb8956a1333c280a8cc35609be800d41645d61 Reviewed-on: https://chromium-review.googlesource.com/688258 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: nainar <nainar@chromium.org> Cr-Commit-Position: refs/heads/master@{#514397} [modify] https://crrev.com/93b5990b8beca44d533e66b959d4baafdc0b7dad/third_party/WebKit/Source/core/css/BUILD.gn [modify] https://crrev.com/93b5990b8beca44d533e66b959d4baafdc0b7dad/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/93b5990b8beca44d533e66b959d4baafdc0b7dad/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h [delete] https://crrev.com/a6e04d9a38b99858c7247da58edc43867b312447/third_party/WebKit/Source/core/css/parser/CSSParserScopedTokenBuffer.h [delete] https://crrev.com/a6e04d9a38b99858c7247da58edc43867b312447/third_party/WebKit/Source/core/css/parser/CSSParserTokenBuffer.h [modify] https://crrev.com/93b5990b8beca44d533e66b959d4baafdc0b7dad/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.cpp [modify] https://crrev.com/93b5990b8beca44d533e66b959d4baafdc0b7dad/third_party/WebKit/Source/core/css/parser/CSSParserTokenStream.h [modify] https://crrev.com/93b5990b8beca44d533e66b959d4baafdc0b7dad/third_party/WebKit/Source/core/css/parser/CSSParserTokenStreamTest.cpp [modify] https://crrev.com/93b5990b8beca44d533e66b959d4baafdc0b7dad/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp
,
Nov 20 2017
Unassigning myself for now, will probably continue this work in 2018Q1. In the meantime, anyone can feel free to take this over.
,
Nov 27 2017
,
Dec 6 2017
,
Apr 4 2018
|
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by phistuck@chromium.org
, Nov 3 2016