Clean up the loadXXXStyle() idiom in StyleResolver.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Mike West]

See https://bugs.webkit.org/show_bug.cgi?id=90834#c11 for details.

[Alexey Proskuryakov]

> See https://bugs.webkit.org/show_bug.cgi?id=90834#c11 for details.

Can you post adequate information here? It's a simple copy/paste for you, but extra effort for multiple other people to do the research.

[Mike West]

(In reply to comment #1)
> > See https://bugs.webkit.org/show_bug.cgi?id=90834#c11 for details.
> 
> Can you post adequate information here? It's a simple copy/paste for you, but extra effort for multiple other people to do the research.

Very fair. This was meant as a note to myself, but you're absolutely right; the bug tracker should be a source of truth.

---

In https://bugs.webkit.org/show_bug.cgi?id=90834#c11, akling@ suggested:

> Source/WebCore/css/StyleResolver.cpp:1361
> +        if (!defaultSeamlessStyle)
> +            loadSeamlessStyle();
> +        matchUARules(result, defaultSeamlessStyle);

This pattern seems overly verbose, it would be nicer if we could just do:

matchUARules(result, defaultSeamlessStyle());

(I see that we already have code like this elsewhere in StyleResolver.cpp, but I think we can do better.)

> Source/WebCore/css/StyleResolver.cpp:5304
> +        if (!defaultSeamlessStyle)
> +            loadSeamlessStyle();
> +        m_features.add(defaultSeamlessStyle->features());

Again, would be nicer with just:

m_features.add(defaultSeamlessStyle()->features());

[Mike West]

Created attachment 181018 [details]
Patch

[Mike West]

akling@: Hopefully this is more or less what you were looking for. What do you think?

[Darin Adler]

Comment on attachment 181018 [details]
Patch

Looks OK.

Also not sure that adding “UA” to these function names improved anything. It just seems like mysterious additional acronym/jargon. Not sure that it even helps in the name matchUARules.

[Mike West]

(In reply to comment #5)
> (From update of attachment 181018 [details])
> Looks OK.

Thanks. I'm dropping the seamless bit from the patch, as that got rolled back and it looks like we'll be doing things entirely differently. The view source bit is still relevant, however, so I appreciate the review.

> Also not sure that adding “UA” to these function names improved anything. It just seems like mysterious additional acronym/jargon. Not sure that it even helps in the name matchUARules.

In context, I think you're right. I'll drop UA from this method name. I don't think I want to touch matchUARules, however. I'll leave that decision for someone who knows what they're doing. :)

[Mike West]

Created attachment 181050 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 181050 [details]
Patch for landing

Clearing flags on attachment: 181050

Committed r138639: <http://trac.webkit.org/changeset/138639>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.