Refactor SelectorChecker::BehaviorAtBoundary

Refactor SelectorChecker::BehaviorAtBoundary

As of Blink r176304, SelectorCheckingContext::scope
for styles in shadow tree changed from its shadow host to
shadow root.

Thus the subtlety of scoping difference between TreeScope
and SelectorCheckingContext::scope is gone, and code for
handling the difference can be cleaned up.

This made possible by dropping support for <style scoped>,
which introduced the complexity at r175993.

With this change no functional difference is expected,
thus all layout tests should pass.

- Convert SelectorChecker::parentElement(context) to static helper function.
- Convert SelectorChecker::scopeContainsLastMatchedElement() to static
  helper function.
- s/{b,B}ehaviorAtBounary/{c,C}ontextFlags/g
- Removed the 2nd parameter in rulesApplicableInCurrentTreeScope()
- Removed the 2nd argument for isHostInItsShadowTree()
- Removed StaysWithinTreeScope
- Removed DoesNotCrossBoundary
- Removed ScopeIsShadowRoot.
- Removed BoundaryBehaviorMask
- Removed ScopeIsShadowHostInPseudoHostParameter
- Introduced DefaultBehavior (to be removed, though)

BUG=none
TEST=pass all layout tests.

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

[kochi, 2014-06-26 09:12:46]

Hi Takashi,

There still remain 2 flags in BehaviorAtBoundary, but
the code should be much cleaner.

Can I have any advice on what/where can be improved?

Please take a look.

[tasak, 2014-06-27 03:35:50]

lgtm

[kochi, 2014-06-27 03:46:21]

On 2014/06/27 03:35:50, tasak wrote:
> lgtm

Thanks, as we discussed offline, I intend to remove the ContextFlags completely.

Elliott, could you take a look as OWNERS?

[esprehn, 2014-06-27 08:05:51]

This lgtm, but can you explain why it was correct to remove all of those checks?
The issue description says what you did, but not why it was correct. Can you add
that detail?

https://codereview.chromium.org/334263020/diff/1/Source/core/css/SelectorChec...
File Source/core/css/SelectorChecker.cpp (left):

https://codereview.chromium.org/334263020/diff/1/Source/core/css/SelectorChec...
Source/core/css/SelectorChecker.cpp:91: if ((context.behaviorAtBoundary &
SelectorChecker::BoundaryBehaviorMask) != SelectorChecker::StaysWithinTreeScope)
Why is this not needed now?

https://codereview.chromium.org/334263020/diff/1/Source/core/css/SelectorChec...
File Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/334263020/diff/1/Source/core/css/SelectorChec...
Source/core/css/SelectorChecker.cpp:84: if (context.scope && context.scope ==
context.element->containingShadowRoot())
Why don't we need the ScopeIsShadowRoot stuff anymore?

[kochi, 2014-06-30 02:44:06]

Thanks Elliott for the review.

Updated the description and replied to your comments inline.

https://codereview.chromium.org/334263020/diff/1/Source/core/css/SelectorChec...
File Source/core/css/SelectorChecker.cpp (left):

https://codereview.chromium.org/334263020/diff/1/Source/core/css/SelectorChec...
Source/core/css/SelectorChecker.cpp:91: if ((context.behaviorAtBoundary &
SelectorChecker::BoundaryBehaviorMask) != SelectorChecker::StaysWithinTreeScope)
On 2014/06/27 08:05:51, esprehn wrote:
> Why is this not needed now?

Now the condition will never be met.

This condition used to mean that the code should not cross
tree scope boundary (e.g. shadow root -> shadow host, which used
to be in the same |context.scope| but in different treescopes)
but now a shadow root and its shadow host stays in a different
|context.scope| and we do not need this condition any more.
(and context.scope and treescope can/will be cleaned up later)

https://codereview.chromium.org/334263020/diff/1/Source/core/css/SelectorChec...
File Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/334263020/diff/1/Source/core/css/SelectorChec...
Source/core/css/SelectorChecker.cpp:84: if (context.scope && context.scope ==
context.element->containingShadowRoot())
On 2014/06/27 08:05:51, esprehn wrote:
> Why don't we need the ScopeIsShadowRoot stuff anymore?

The flag used to mean that a context's scope was shadow *host*
(i.e. it pointed to the host element, instead of shadow *root*),
because it was needed to discriminate a scope of its shadow root
child from a scope of its own (<style scoped>).
Now <style scoped> is gone, and scope points to its shadow root,
we can tell a scope is shadow or not by just checking whether
a context's scope is shadow root or not, and we do not need the
flag any more.

[kochi, 2014-06-30 02:44:12]

The CQ bit was checked by kochi@chromium.org

[commit-bot: I haz the power, 2014-06-30 02:44:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kochi@chromium.org/334263020/1

[commit-bot: I haz the power, 2014-06-30 03:37:26]

Change committed as 177183