Don't use consume() in CSSTokenizer when ignoring the return value.

Don't use consume() in CSSTokenizer when ignoring the return value.

consume() (the no argument version) does a bunch of work handling out of bounds
access, NUL replacement characters, and returning the char. Many of the callers
just want to increment the offset. We could do that by replacing the call sites
with consume(1) which is the "fast" version that doesn't have a return value,
but it'd make the code more clear to just inline advance() in the right spots.

BUG=605792
Committed: https://crrev.com/c0d8da0516d76101a420989d2fc9ce9444c21c32
Cr-Commit-Position: refs/heads/master@{#402328}

[esprehn, 2016-06-27 19:35:25]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 19:36:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[esprehn, 2016-06-27 21:03:53]

Description was changed from

==========
Don't use consume() in CSSTokenizer when ignoring the return value.

BUG=
==========

to

==========
Don't use consume() in CSSTokenizer when ignoring the return value.

consume() does a bunch of work handling out of bounds access, NUL replacement
characters, and returning the char. Many of the callers just want to increment
the offset, lets do that directly instead.

BUG=605792
==========

[esprehn, 2016-06-27 21:04:08]

esprehn@chromium.org changed reviewers:
+ meade@chromium.org, timloh@chromium.org

[meade_UTC10, 2016-06-27 21:35:55]

lgtm

I don't really understand the comment about consume() doing extra work here,
this looks like a simple substitution?

[esprehn, 2016-06-27 21:46:29]

https://codereview.chromium.org/2099413002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp (left):

https://codereview.chromium.org/2099413002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp:117: void
CSSTokenizer::consume(unsigned offset)
This is confusing, but there was actually two methods called consume(). One took
an argument and and just did advance(), the other right above this takes no
arguments and does:

    UChar current = m_input.nextInputChar();
    m_input.advance();
    return current;

which is what the comment is about.

[esprehn, 2016-06-27 21:47:21]

Description was changed from

==========
Don't use consume() in CSSTokenizer when ignoring the return value.

consume() does a bunch of work handling out of bounds access, NUL replacement
characters, and returning the char. Many of the callers just want to increment
the offset, lets do that directly instead.

BUG=605792
==========

to

==========
Don't use consume() in CSSTokenizer when ignoring the return value.

consume() (the no argument version) does a bunch of work handling out of bounds
access, NUL replacement characters, and returning the char. Many of the callers
just want to increment the offset. We could do that by replacing the call sites
with consume(1) which is the "fast" version that doesn't have a return value,
but it'd make the code more clear to just inline advance() in the right spots.

BUG=605792
==========

[esprehn, 2016-06-27 21:47:50]

The CQ bit was unchecked by esprehn@chromium.org

[esprehn, 2016-06-27 21:47:51]

The CQ bit was checked by esprehn@chromium.org

[commit-bot: I haz the power, 2016-06-27 21:49:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-27 23:23:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-06-27 23:25:10]

Description was changed from

==========
Don't use consume() in CSSTokenizer when ignoring the return value.

consume() (the no argument version) does a bunch of work handling out of bounds
access, NUL replacement characters, and returning the char. Many of the callers
just want to increment the offset. We could do that by replacing the call sites
with consume(1) which is the "fast" version that doesn't have a return value,
but it'd make the code more clear to just inline advance() in the right spots.

BUG=605792
==========

to

==========
Don't use consume() in CSSTokenizer when ignoring the return value.

consume() (the no argument version) does a bunch of work handling out of bounds
access, NUL replacement characters, and returning the char. Many of the callers
just want to increment the offset. We could do that by replacing the call sites
with consume(1) which is the "fast" version that doesn't have a return value,
but it'd make the code more clear to just inline advance() in the right spots.

BUG=605792

Committed: https://crrev.com/c0d8da0516d76101a420989d2fc9ce9444c21c32
Cr-Commit-Position: refs/heads/master@{#402328}
==========

[commit-bot: I haz the power, 2016-06-27 23:25:11]

Patchset 1 (id:??) landed as
https://crrev.com/c0d8da0516d76101a420989d2fc9ce9444c21c32
Cr-Commit-Position: refs/heads/master@{#402328}