Avoid calculating style for ::before/::after without content.

Avoid calculating style for ::before/::after without content.

A universal ::before or ::after rule without content triggers calculation
of pseudo style for most elements on theverge.com just to find that most of
them won't be rendered due to the missing content.

Check if matching ::before or ::after rules actually contain a content
value before marking the render style with setHasPseudoStyle.

This change reduces the style recalc time by ~50% when expanding the
comments section on theverge.com articles.

R=esprehn@chromium.org,eseidel@chromium.org
BUG=412558
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182308

[rune, 2014-09-14 09:55:30]

The regression most likely due to
https://code.google.com/p/chromium/issues/detail?id=414088

[esprehn, 2014-09-14 19:36:13]

I think you're making us hit this branch:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

and you've made us not cache the style anymore by not applying the rules so we
return the element's own style.

[rune, 2014-09-15 09:23:54]

On 2014/09/14 at 19:36:13, esprehn wrote:
> I think you're making us hit this branch:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> and you've made us not cache the style anymore by not applying the rules so we
return the element's own style.

You're right. I can try to support pseudo style on non-rendered pseudo elements
in another CL.

The regression of not being able to get proper computed style off of
non-rendered pseudo elements (has ::before/::after rule, but no content
property) does not sound like it's going to cause problems in the wild. Do you
think we can land this CL by changing the expected result?

[esprehn, 2014-09-15 14:33:37]

I don't think we can have this regression, this was a bug that was filed
against webkit long ago and broke websites. I also suspect you broke the
web inspector.

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[rune, 2014-09-15 15:09:40]

On 2014/09/15 at 14:33:37, esprehn wrote:
> I don't think we can have this regression, this was a bug that was filed
> against webkit long ago and broke websites. I also suspect you broke the
> web inspector.

OK. I have a patch that fixes the cases in
https://code.google.com/p/chromium/issues/detail?id=414088

It doesn't address the !inDocument() cases, but that's an issue for normal
elements as well. I'm in the middle of something else, I'll see if I get to
upload it later this evening.

[rune, 2014-09-15 21:12:35]

On 2014/09/15 at 15:09:40, rune wrote:
> On 2014/09/15 at 14:33:37, esprehn wrote:
> > I don't think we can have this regression, this was a bug that was filed
> > against webkit long ago and broke websites. I also suspect you broke the
> > web inspector.
> 
> OK. I have a patch that fixes the cases in
https://code.google.com/p/chromium/issues/detail?id=414088
> 
> It doesn't address the !inDocument() cases, but that's an issue for normal
elements as well. I'm in the middle of something else, I'll see if I get to
upload it later this evening.

https://codereview.chromium.org/572043002

[rune, 2014-09-18 07:46:08]

On 2014/09/15 at 21:12:35, rune wrote:
> On 2014/09/15 at 15:09:40, rune wrote:
> > On 2014/09/15 at 14:33:37, esprehn wrote:
> > > I don't think we can have this regression, this was a bug that was filed
> > > against webkit long ago and broke websites. I also suspect you broke the
> > > web inspector.
> > 
> > OK. I have a patch that fixes the cases in
https://code.google.com/p/chromium/issues/detail?id=414088
> > 
> > It doesn't address the !inDocument() cases, but that's an issue for normal
elements as well. I'm in the middle of something else, I'll see if I get to
upload it later this evening.
> 
> https://codereview.chromium.org/572043002

Now this one's passing try-bots. \o/

[esprehn, 2014-09-19 04:00:36]

lgtm, you rock:

http://i.imgur.com/fg20Dvo.gif

[rune, 2014-09-19 07:43:09]

The CQ bit was checked by rune@opera.com

[commit-bot: I haz the power, 2014-09-19 07:43:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/570713002/20001

[rune, 2014-09-19 07:43:56]

On 2014/09/19 04:00:36, esprehn wrote:
> lgtm, you rock:
> 
> http://i.imgur.com/fg20Dvo.gif

I think I need to get a web-cam too :-)

Or do reviews from my tablet.

[commit-bot: I haz the power, 2014-09-19 08:47:54]

Committed patchset #2 (id:20001) as 182308