Split out CSSParser public API

Split out CSSParser public API

The goal was to identify and separate out the "public" API
for the CSS parsing subsystem from the larger set
of public methods on CSSParser (now BisonCSSParser)
which are marked public just for CSSGrammer.y to consume.

BisonCSSParser (previously CSSParser) exposes over *300*
public methods and members!  All but the 15 copied to
CSSParser were just for CSSGrammar.y integration.

This is all in preparation for attempting to swap out
the CSSParser wholesale with a from-scratch implementation
of the CSS3 Syntax specification.  CSSParser.cpp now provides
a place for me to do that run-time switching between
the Bison-based parser and my new hand-rolled parser (not yet landed).

Much of what is now BisonCSSParser will need to be split out
into a separate "CSSBuilder" object or something akin to the
HTMLConstructionSite and re-used by my proposed re-written CSS parser
as the CSS3 syntax spec doesn't specify all the details encapsulated
in the old parser grammar file.

Follow-up patches will work to further reduce the exposed CSSParser
API and remove more of this redundant indirection.

BUG=330389

[eseidel, 2013-12-27 10:18:10]

Nowhere near ready for landing, but comments on the direction/approach are most
welcome.

[darktears, 2013-12-27 12:32:31]

On 2013/12/27 10:18:10, eseidel wrote:
> Nowhere near ready for landing, but comments on the direction/approach are
most
> welcome.

Globally I think that's the way to move forward. 

I would start with hooking up parseValue(MutableStylePropertySet*,
CSSPropertyID, const String&, bool important, CSSParserMode,
StyleSheetContents*); which is a significant simpler parsing (you just parse the
inside of a declaration) so you would need a simpler tokenizer, and a rather
simpler parsing algorithm but that would be a good starting point and probably
already worth it. Note that we have few "shortcuts" because bison happens to be
slower than doing what these shortcuts are doing so if we can replace already
all the shortcuts and bison for that function it will be already a step forward
in term of cleanup and probably with performance. 

I think renaming the old CSSParser in BisonCSSParser is not a bad thing so we
can slowly fix the code paths all over the place to use the new CSSParser
object. In the other hand I'm wondering if you want to move today all the public
API into the new CSSParser because for now what the code would do would be to
call the old BisonCSSParser. I'm wondering what would be the benefits? Maybe we
should start to build that new public API incrementally : add parseValue (then
move the call sites to use the new class), then parseSelector and so on. It may
be that on the way we find some smarter API or better way to achieve the things.
Adding just an indirection is not necessarily an improvement, I'd rather have
the call site to use BisonCSSParser directly up until the new CSSParser
implements the functionality. What do you think?

[eseidel, 2013-12-27 18:57:37]

Spoke with darktears over #blink.

My original motivation with this work was to make it possible to make it runtime
switchable between the current bison-based parser and my in-progress hand-rolled
CSS Syntax Module-compilant parser.

It turns out that we have exactly 15 public methods on CSSParser. :)  That's
probably about 10 too many.

I don't plan to land this patch as-is.  I'm going to first split off about 5
starter patches, including renaming CSSParser to BisonCSSParser and land those
first.

[esprehn, 2013-12-27 19:00:21]

On 2013/12/27 18:57:37, eseidel wrote:
> Spoke with darktears over #blink.
> 
> My original motivation with this work was to make it possible to make it
runtime
> switchable between the current bison-based parser and my in-progress
hand-rolled
> CSS Syntax Module-compilant parser.
> 
> It turns out that we have exactly 15 public methods on CSSParser. :)  That's
> probably about 10 too many.
> 
> I don't plan to land this patch as-is.  I'm going to first split off about 5
> starter patches, including renaming CSSParser to BisonCSSParser and land those
> first.

sgtm

[eseidel, 2013-12-28 03:41:18]

I think this is ready for landing.  The question is mostly if we're OK with this
redundant plumbing.  I expect several of these indirections will last a long
time, but at least now it's very obvious what is public API to the CSS parsing
subsystem, and what is not.  And this makes obvious some of the warts of this
"API".

[eseidel, 2013-12-28 03:50:30]

Hmm.  Lots of test failures.

I'm still interested in hearing from folks if you think that CSSParser adds
enough value in API clarity (and ability to indirect to a new parser backend) to
justify its redundant indirection.

[eseidel, 2013-12-30 20:01:52]

Landing the renaming in https://codereview.chromium.org/100453011/, then it
should be easier to find my error.  Likely just a merge error which caused
failures.

[eseidel, 2013-12-31 01:02:23]

Moving this to https://codereview.chromium.org/112933010/ since #rietfail is
only giving 500s.