AffectedByHover not always properly set on display:contents
Reported by
r...@opera.com,
Sep 25 2017
|
||||||||
Issue descriptionIf we recalculate the computed style of a display:contents element, and the ComputedStyle doesn't change, but the AffectedBy* bits changed because other selectors are matched, we don't store the new ComputedStyle which means we don't store a ComputedStyle with the AffectedBy* flags correctly set. See attached demo.
,
Sep 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b37246c8f89310cb5f8d94c385627b6b596a0c6 commit 5b37246c8f89310cb5f8d94c385627b6b596a0c6 Author: Rune Lillesveen <rune@opera.com> Date: Tue Sep 26 04:43:44 2017 Store new ComputedStyle object if style did not change. The comparison of ComputedStyle does not take additional flags into account. For instance, the AffectedBy* flags used for updating pseudo classes like :hover. We used to call SetStyleInternal, but this was removed because the previous comment said it was because of style sharing which is now removed. The display:contents case (768406) never worked because the code path for StoreNonLayoutObjectComputedStyle() was always skipped when computed style compared to be equal. Bug: 768406 , 767832 Change-Id: Iac4708e3cd3a6451d99c1bb2bb69efb74289b8eb Reviewed-on: https://chromium-review.googlesource.com/681755 Reviewed-by: nainar <nainar@chromium.org> Commit-Queue: Rune Lillesveen <rune@opera.com> Cr-Commit-Position: refs/heads/master@{#504285} [add] https://crrev.com/5b37246c8f89310cb5f8d94c385627b6b596a0c6/third_party/WebKit/LayoutTests/fast/dynamic/hover-after-affected-by-change.html [modify] https://crrev.com/5b37246c8f89310cb5f8d94c385627b6b596a0c6/third_party/WebKit/Source/core/dom/Element.cpp
,
Sep 26 2017
,
Sep 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8ac19ac76519aaa0e4ffd228629d4488d5efea79 commit 8ac19ac76519aaa0e4ffd228629d4488d5efea79 Author: Marc Treib <treib@chromium.org> Date: Tue Sep 26 08:18:11 2017 Revert "Store new ComputedStyle object if style did not change." This reverts commit 5b37246c8f89310cb5f8d94c385627b6b596a0c6. Reason for revert: hover-after-affected-by-change.html consistently fails on WebKit Linux Trusty Leak: https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak Original change's description: > Store new ComputedStyle object if style did not change. > > The comparison of ComputedStyle does not take additional flags into > account. For instance, the AffectedBy* flags used for updating pseudo > classes like :hover. We used to call SetStyleInternal, but this was > removed because the previous comment said it was because of style > sharing which is now removed. > > The display:contents case (768406) never worked because the code path > for StoreNonLayoutObjectComputedStyle() was always skipped when > computed style compared to be equal. > > Bug: 768406 , 767832 > Change-Id: Iac4708e3cd3a6451d99c1bb2bb69efb74289b8eb > Reviewed-on: https://chromium-review.googlesource.com/681755 > Reviewed-by: nainar <nainar@chromium.org> > Commit-Queue: Rune Lillesveen <rune@opera.com> > Cr-Commit-Position: refs/heads/master@{#504285} TBR=rune@opera.com,nainar@chromium.org Change-Id: I34a54b694bb719b278426f9a76076fa77cd9e82f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 768406 , 767832 Reviewed-on: https://chromium-review.googlesource.com/684275 Reviewed-by: Marc Treib <treib@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#504313} [delete] https://crrev.com/a3f3699bedf7166984c5c4664f673eecb7486aaa/third_party/WebKit/LayoutTests/fast/dynamic/hover-after-affected-by-change.html [modify] https://crrev.com/8ac19ac76519aaa0e4ffd228629d4488d5efea79/third_party/WebKit/Source/core/dom/Element.cpp
,
Sep 26 2017
,
Sep 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9f0c480da9cb188c82e16e6c6751875a60a114cf commit 9f0c480da9cb188c82e16e6c6751875a60a114cf Author: Rune Lillesveen <rune@opera.com> Date: Wed Sep 27 02:59:59 2017 Reland "Store new ComputedStyle object if style did not change." This is a reland of https://chromium-review.googlesource.com/681755 The comparison of ComputedStyle does not take additional flags into account. For instance, the AffectedBy* flags used for updating pseudo classes like :hover. We used to call SetStyleInternal, but this was removed because the previous comment said it was because of style sharing which is now removed. The display:contents case (768406) never worked because the code path for StoreNonLayoutObjectComputedStyle() was always skipped when computed style compared to be equal. TBR=nainar@chromium.org,treib@chromium.org Bug: 768406 , 767832 , 768790 Change-Id: Iee509e43e5fcc29d04944655d3d966ea61cc2adf Reviewed-on: https://chromium-review.googlesource.com/684186 Commit-Queue: Rune Lillesveen <rune@opera.com> Reviewed-by: Rune Lillesveen <rune@opera.com> Cr-Commit-Position: refs/heads/master@{#504561} [add] https://crrev.com/9f0c480da9cb188c82e16e6c6751875a60a114cf/third_party/WebKit/LayoutTests/fast/dynamic/hover-after-affected-by-change.html [modify] https://crrev.com/9f0c480da9cb188c82e16e6c6751875a60a114cf/third_party/WebKit/Source/core/dom/Element.cpp
,
Sep 27 2017
,
Nov 30 2017
Re-opened due to revert: https://crrev.com/a1bff011ab3fc8f42feb1d27f796cabfc2eef7da
,
Dec 6 2017
,
Jan 17 2018
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/394e62d9a30231f2d4a226e5e2f585ec36397b81 commit 394e62d9a30231f2d4a226e5e2f585ec36397b81 Author: Rune Lillesveen <futhark@chromium.org> Date: Fri Jan 19 10:14:49 2018 Use new ComputedStyle instance for kNoChange. Even if the computed style doesn't change, AffectedBy* flags may have changed, which means we need to set the current ComputedStyle to the new instance. We can however skip any invalidation diffs. This is a re-land of the revert of this functionality which did not make us recover from the memory regression reported in 771294. Bug: 768406 , 771294 Change-Id: Icc7eeee8a982eed53243d52d04d45e313d79a10d Reviewed-on: https://chromium-review.googlesource.com/870391 Reviewed-by: nainar <nainar@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#530468} [add] https://crrev.com/394e62d9a30231f2d4a226e5e2f585ec36397b81/third_party/WebKit/LayoutTests/external/wpt/css/selectors/hover-002-manual.html [add] https://crrev.com/394e62d9a30231f2d4a226e5e2f585ec36397b81/third_party/WebKit/LayoutTests/external/wpt_automation/css/selectors/hover-002-manual-automation.js [modify] https://crrev.com/394e62d9a30231f2d4a226e5e2f585ec36397b81/third_party/WebKit/LayoutTests/resources/testharnessreport.js [modify] https://crrev.com/394e62d9a30231f2d4a226e5e2f585ec36397b81/third_party/WebKit/Source/core/dom/Element.cpp [modify] https://crrev.com/394e62d9a30231f2d4a226e5e2f585ec36397b81/third_party/WebKit/Source/core/layout/LayoutObject.cpp [modify] https://crrev.com/394e62d9a30231f2d4a226e5e2f585ec36397b81/third_party/WebKit/Source/core/style/ComputedStyle.h
,
Jan 24 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by shend@chromium.org
, Sep 25 2017