Remove friend declarations from CSSPropertyParser

Remove friend declarations from CSSPropertyParser

Add some more private CSSPropertyParser methods
so all friend declarations can be removed.

BUG=404023
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189984

[rwlbuis, 2015-02-09 22:07:43]

rob.buis@samsung.com changed reviewers:
+ timloh@chromium.org

[rwlbuis, 2015-02-09 22:07:43]

PTAL

[Timothy Loh, 2015-02-10 04:45:55]

I don't think implementation details for the CSSPropertyParser should be exposed
like this. Honestly having classes for most of these is just complexity we don't
need and they should be easier to be implemented with just plain functions.

[rwlbuis, 2015-02-10 21:27:07]

On 2015/02/10 04:45:55, Timothy Loh wrote:
> I don't think implementation details for the CSSPropertyParser should be
exposed
> like this. Honestly having classes for most of these is just complexity we
don't
> need and they should be easier to be implemented with just plain functions.

I think getting rid of the class FooParseContext stuff is quite some work.
I went in another direction, let me know what you think.

[Timothy Loh, 2015-02-11 02:59:48]

lgtm

I'll probably try to remove the context classes in a couple of months when I
have time :)

https://codereview.chromium.org/907063002/diff/40001/Source/core/css/parser/C...
File Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/907063002/diff/40001/Source/core/css/parser/C...
Source/core/css/parser/CSSPropertyParser.cpp:5558:
RefPtrWillBeRawPtr<CSSPrimitiveValue> length = createPrimitiveNumericValue(val);
Maybe just inline this in the function call

[rwlbuis, 2015-02-11 03:44:31]

On 2015/02/11 02:59:48, Timothy Loh wrote:
> lgtm

Thanks!

> I'll probably try to remove the context classes in a couple of months when I
> have time :)

CSS3 parser rewrite keeping you busy huh? :)
I like this bug, it is a good excuse to fix some annoying thing in
CSSPropertyParser, like the friends stuff. I'd
really like the cpp to be smaller but there is probably no way around that.

>
https://codereview.chromium.org/907063002/diff/40001/Source/core/css/parser/C...
> File Source/core/css/parser/CSSPropertyParser.cpp (right):
> 
>
https://codereview.chromium.org/907063002/diff/40001/Source/core/css/parser/C...
> Source/core/css/parser/CSSPropertyParser.cpp:5558:
> RefPtrWillBeRawPtr<CSSPrimitiveValue> length =
createPrimitiveNumericValue(val);
> Maybe just inline this in the function call

Will do (tomorrow).

[rwlbuis, 2015-02-11 16:44:32]

The CQ bit was checked by rob.buis@samsung.com

[rwlbuis, 2015-02-11 16:46:17]

On 2015/02/11 02:59:48, Timothy Loh wrote:
>
https://codereview.chromium.org/907063002/diff/40001/Source/core/css/parser/C...
> Source/core/css/parser/CSSPropertyParser.cpp:5558:
> RefPtrWillBeRawPtr<CSSPrimitiveValue> length =
createPrimitiveNumericValue(val);
> Maybe just inline this in the function call

I checked but there is no release() on what createPrimitiveNumericValue returns.

[commit-bot: I haz the power, 2015-02-11 16:46:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/907063002/40001

[commit-bot: I haz the power, 2015-02-11 21:45:14]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189984