Avoid unnecessary invalidation scheduling.

Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440
Committed: https://crrev.com/0d735b3ac72c85222599617a0aae571eb8f9170e
Cr-Commit-Position: refs/heads/master@{#365188}

[rune, 2015-12-10 00:20:53]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-10 00:22:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1514733002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1514733002/1

[rune, 2015-12-10 00:23:12]

Description was changed from

==========
Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440
==========

to

==========
*** NOT FOR LANDING ***

Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440
==========

[rune, 2015-12-10 00:34:12]

Needs testing.

[commit-bot: I haz the power, 2015-12-10 01:28:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-10 01:28:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[esprehn, 2015-12-10 02:48:32]

esprehn@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2015-12-10 02:48:32]

Looks like you fail a ton of tests, but the spirit of this change looks good.

[rune, 2015-12-10 08:07:12]

On 2015/12/10 02:48:32, esprehn wrote:
> Looks like you fail a ton of tests, but the spirit of this change looks good.

Yes, we tried to schedule invalidations on elements in import documents. I guess
that the styleResolver check was done before the ASSERT(isMaster()) before this
change.

[rune, 2015-12-10 08:08:52]

https://codereview.chromium.org/1514733002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/1514733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/StyleEngine.cpp:633: return true;
Should I rely on non-master documents having a null-resolver and
ASSERT(isMaster()) below the resolver() check instead of this if-check?

[esprehn, 2015-12-10 08:53:47]

https://codereview.chromium.org/1514733002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/1514733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/StyleEngine.cpp:633: return true;
On 2015/12/10 at 08:08:52, rune wrote:
> Should I rely on non-master documents having a null-resolver and
ASSERT(isMaster()) below the resolver() check instead of this if-check?

the non master will also not be active, so it'll fail the inActiveDocument()

https://codereview.chromium.org/1514733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/StyleEngine.cpp:645: if
(shouldSkipInvalidationFor(element))
classChangedForElement is guarded by:

    bool testShouldInvalidateStyle = inActiveDocument() && styleResolver;


idAttribute and others are guarded the same since accessing the styleEngine()
will crash if you're not in an active document.

I don't think you need either of those checks in the skip method, it should
perhaps just assert?

[esprehn, 2015-12-10 08:57:01]

ignore that last one, I missed the other changes. This looks fine, you don't
need the isMaster check though, isActiveDocument covers it even if we didn't
have the resolver() check.

lgtm

https://codereview.chromium.org/1514733002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/1514733002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/StyleEngine.cpp:645: if
(shouldSkipInvalidationFor(element))
Oh I see you removed all those checks, this seems fine then.

[rune, 2015-12-10 13:47:11]

Description was changed from

==========
*** NOT FOR LANDING ***

Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440
==========

to

==========
Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440
==========

[rune, 2015-12-14 21:41:55]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-14 21:44:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1514733002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1514733002/40001

[commit-bot: I haz the power, 2015-12-15 04:20:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-15 04:20:40]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rune, 2015-12-15 06:41:30]

The CQ bit was checked by rune@opera.com

[rune, 2015-12-15 06:41:30]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1514733002/#ps40001
(title: "Removed unnecessary isMaster check")

[commit-bot: I haz the power, 2015-12-15 06:41:58]

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

[commit-bot: I haz the power, 2015-12-15 06:46:25]

Description was changed from

==========
Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440
==========

to

==========
Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440
==========

[commit-bot: I haz the power, 2015-12-15 06:46:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-15 06:47:08]

Description was changed from

==========
Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440
==========

to

==========
Avoid unnecessary invalidation scheduling.

We skip scheduling invalidation sets for an element when:

* StyleResolver is null
* Element is not inActiveDocument()
* Element does not have a parent
* Element parent has SubtreeStyleChange or ReattachStyleChange

Additionally we skip descendant invalidations when:

* Element has SubtreeStyleChange or ReattachStyleChange

and sibling invalidations when:

* Element.nextSibling is null

Removed an unnecessary SubtreeStyleChange when StyleResolver is null for
attribute changes.

BUG=557440

Committed: https://crrev.com/0d735b3ac72c85222599617a0aae571eb8f9170e
Cr-Commit-Position: refs/heads/master@{#365188}
==========

[commit-bot: I haz the power, 2015-12-15 06:47:09]

Patchset 3 (id:??) landed as
https://crrev.com/0d735b3ac72c85222599617a0aae571eb8f9170e
Cr-Commit-Position: refs/heads/master@{#365188}