Stop sorting StyleEngine::m_activeTreeScopes

Stop sorting StyleEngine::m_activeTreeScopes

Blink no longer has code that depends on StyleEngine::m_activeTreeScopes
being sorted in the document order.

The sorting was introduced in crbug.com/244314#c18, in preparation to
support "Order of Appearance" in tree-of-trees. This hasn't been really
implemented, and is no longer needed in the current spec.

Still the sorting takes ~5% of ContainerNode::insertBefore() when
inserting <style> elements and there are a large number of TreeScopes
in the document.

This patch gets rid of the sorting.

BUG=401359, 433225
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196831

[kojii, 2015-06-09 03:34:43]

kojii@chromium.org changed reviewers:
+ esprehn@chromium.org, hayato@chromium.org, kochi@chromium.org, rune@opera.com,
tasak@chromium.org

[kojii, 2015-06-09 03:34:44]

PTAL.

[hayato, 2015-06-09 03:54:25]

lgtm

[hayato, 2015-06-09 03:56:27]

On 2015/06/09 03:54:25, hayato wrote:
> lgtm

Before landing, could you summarize how blink used sorted activeTreeScopes so
far in the description of CL?

[kochi, 2015-06-09 04:26:25]

lgtm

[kojii, 2015-06-09 05:27:35]

On 2015/06/09 at 03:56:27, hayato wrote:
> 
> Before landing, could you summarize how blink used sorted activeTreeScopes so
far in the description of CL?

Added a brief paragraph. I actually don't have more background than this; when I
started digging into this, there doesn't seem to be any dependencies, so they're
already gone, or not added yet. I did:
* Read and traced through all code that uses m_activeTreeScopes and see if the
order matters. Couldn't find any obvious ones.
* esprehen and rune suggested they don't believe it's needed in
https://codereview.chromium.org/1131493008.
* Discussed with tasak to confirm he agrees with them.
* Discussed with pfeldman to conclude that Inspector team is fine not to sort.
Code reading + all these experts agreeing is the proof of no further plans to
add dependencies on the sorted order for me.

As far as I read the code, I think we can reduce the needs of the list further
as we move more form StyleResolver to ScopedStyleResolver, but that'd be in
future CLs.

[hayato, 2015-06-09 06:06:52]

On 2015/06/09 05:27:35, kojii wrote:
> On 2015/06/09 at 03:56:27, hayato wrote:
> > 
> > Before landing, could you summarize how blink used sorted activeTreeScopes
so
> far in the description of CL?
> 
> Added a brief paragraph. I actually don't have more background than this; when
I
> started digging into this, there doesn't seem to be any dependencies, so
they're
> already gone, or not added yet. I did:
> * Read and traced through all code that uses m_activeTreeScopes and see if the
> order matters. Couldn't find any obvious ones.
> * esprehen and rune suggested they don't believe it's needed in
> https://codereview.chromium.org/1131493008.
> * Discussed with tasak to confirm he agrees with them.
> * Discussed with pfeldman to conclude that Inspector team is fine not to sort.
> Code reading + all these experts agreeing is the proof of no further plans to
> add dependencies on the sorted order for me.
> 
> As far as I read the code, I think we can reduce the needs of the list further
> as we move more form StyleResolver to ScopedStyleResolver, but that'd be in
> future CLs.

Thanks.

Yeah, I was just in curious that how cascading order, Order of Appearance, can
be achieved without sorting.
I guess Blink doesn't implement it correctly.

I'm totally fine with this CL if no one uses the sorting as of now. We can think
again about the cascading order later.

[kojii, 2015-06-10 03:50:41]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2015-06-10 03:50:47]

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

[commit-bot: I haz the power, 2015-06-10 05:05:53]

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