Change StyleResolver::document() to be private

Change StyleResolver::document() to be private

Moved StyleResolver::document() into the private section and changed all callers to get the document from other sources.

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

[rtakacs, 2014-09-21 08:15:28]

rtakacs.u-szeged@partner.samsung.com changed reviewers:
+ eseidel@chromium.org

[rune, 2014-09-24 08:16:20]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2014-09-24 08:16:21]

https://codereview.chromium.org/579253002/diff/1/Source/core/animation/css/CS...
File Source/core/animation/css/CSSAnimations.cpp (right):

https://codereview.chromium.org/579253002/diff/1/Source/core/animation/css/CS...
Source/core/animation/css/CSSAnimations.cpp:203: // FIXME: This is all
implementation detail of style resolver, CSSAnimations shouldn't be reaching
into any of it.
Fixing this FIXME would make more sense.

https://codereview.chromium.org/579253002/diff/1/Source/core/css/resolver/Sco...
File Source/core/css/resolver/ScopedStyleResolver.cpp (right):

https://codereview.chromium.org/579253002/diff/1/Source/core/css/resolver/Sco...
Source/core/css/resolver/ScopedStyleResolver.cpp:77: AddRuleFlags addRuleFlags =
m_scope->document().securityOrigin()->canRequest(sheet->baseURL()) ?
RuleHasDocumentSecurityOrigin : RuleHasNoSpecialState;
Why are we having StyleResolver call into ScopedStyleResolver and then back into
StyleResolver?

There are two statements here that needs the context of the ScopedStyleResolver
afaict:

m_authorStyleSheets.append(cssSheet)
resolver->processScopedRules(...)

We have the treeScope necessary for processScoped rules retrieved in
StyleResolver::appendCSSStyleSheet. We could make this method just add the
stylesheet to m_authorStyleSheets and return the index?

https://codereview.chromium.org/579253002/diff/1/Source/core/css/resolver/Sty...
File Source/core/css/resolver/StyleResolver.cpp (right):

https://codereview.chromium.org/579253002/diff/1/Source/core/css/resolver/Sty...
Source/core/css/resolver/StyleResolver.cpp:190: TreeScope* treeScope =
ScopedStyleResolver::treeScopeFor(*m_document, cssSheet);
Why not keep document() and make it private instead? I think the current code
looks prettier.

[Timothy Loh, 2014-09-24 08:20:10]

https://codereview.chromium.org/579253002/diff/1/Source/core/animation/css/CS...
File Source/core/animation/css/CSSAnimations.cpp (right):

https://codereview.chromium.org/579253002/diff/1/Source/core/animation/css/CS...
Source/core/animation/css/CSSAnimations.cpp:203: // FIXME: This is all
implementation detail of style resolver, CSSAnimations shouldn't be reaching
into any of it.
On 2014/09/24 08:16:20, rune wrote:
> Fixing this FIXME would make more sense.

I think esprehn has a patch up for this somewhere.

[esprehn, 2014-09-24 08:39:33]

https://codereview.chromium.org/579253002/diff/1/Source/core/animation/css/CS...
File Source/core/animation/css/CSSAnimations.cpp (right):

https://codereview.chromium.org/579253002/diff/1/Source/core/animation/css/CS...
Source/core/animation/css/CSSAnimations.cpp:203: // FIXME: This is all
implementation detail of style resolver, CSSAnimations shouldn't be reaching
into any of it.
On 2014/09/24 at 08:20:10, Timothy Loh wrote:
> On 2014/09/24 08:16:20, rune wrote:
> > Fixing this FIXME would make more sense.
> 
> I think esprehn has a patch up for this somewhere.

Yeah we can do that in a separate patch.

[rtakacs, 2014-10-07 10:42:49]

On 2014/09/24 08:16:21, rune wrote:
>
https://codereview.chromium.org/579253002/diff/1/Source/core/animation/css/CS...
> File Source/core/animation/css/CSSAnimations.cpp (right):
> 
>
https://codereview.chromium.org/579253002/diff/1/Source/core/animation/css/CS...
> Source/core/animation/css/CSSAnimations.cpp:203: // FIXME: This is all
> implementation detail of style resolver, CSSAnimations shouldn't be reaching
> into any of it.
> Fixing this FIXME would make more sense.
> 
>
https://codereview.chromium.org/579253002/diff/1/Source/core/css/resolver/Sco...
> File Source/core/css/resolver/ScopedStyleResolver.cpp (right):
> 
>
https://codereview.chromium.org/579253002/diff/1/Source/core/css/resolver/Sco...
> Source/core/css/resolver/ScopedStyleResolver.cpp:77: AddRuleFlags addRuleFlags
=
> m_scope->document().securityOrigin()->canRequest(sheet->baseURL()) ?
> RuleHasDocumentSecurityOrigin : RuleHasNoSpecialState;
> Why are we having StyleResolver call into ScopedStyleResolver and then back
into
> StyleResolver?
> 
> There are two statements here that needs the context of the
ScopedStyleResolver
> afaict:
> 
> m_authorStyleSheets.append(cssSheet)
> resolver->processScopedRules(...)
> 
> We have the treeScope necessary for processScoped rules retrieved in
> StyleResolver::appendCSSStyleSheet. We could make this method just add the
> stylesheet to m_authorStyleSheets and return the index?
> 
>
https://codereview.chromium.org/579253002/diff/1/Source/core/css/resolver/Sty...
> File Source/core/css/resolver/StyleResolver.cpp (right):
> 
>
https://codereview.chromium.org/579253002/diff/1/Source/core/css/resolver/Sty...
> Source/core/css/resolver/StyleResolver.cpp:190: TreeScope* treeScope =
> ScopedStyleResolver::treeScopeFor(*m_document, cssSheet);
> Why not keep document() and make it private instead? I think the current code
> looks prettier.

ping

[rune, 2014-10-07 10:59:00]

Great. LGTM.

[rtakacs, 2014-10-07 13:04:01]

The CQ bit was checked by rtakacs.u-szeged@partner.samsung.com

[commit-bot: I haz the power, 2014-10-07 13:05:17]

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

[commit-bot: I haz the power, 2014-10-07 13:55:21]

Committed patchset #2 (id:20001) as 183333