ShadowDOM: StyleEngine.updateActiveStyleSheets is unreasonably fast-returning in certain shadowDOM cases |
|||||
Issue descriptionChrome Version : 57.0.2987.98 What steps will reproduce the problem? 1. Navigate to https://jsfiddle.net/5tz99m9p/1/ 2. Open devtools -> sources panel 3. Notice a 'style.css' stylesheet (which came from shadow dom). 4. Hit the "remove shadow dom" button in the jsfiddle What is the expected result? The stylesheet disappears in devtools. What happens instead of that? StyleSheet stays as-is. It turned out that StyleEngine::updateActiveStyleSheets() is fast-returning in this case (which AFAIU shouldn't happen since stylesheet actually got detached from DOM).
,
Mar 21 2017
,
Mar 21 2017
Rune, could you take a look as this is related to the asynchronous update of active stylesheets?
,
Mar 21 2017
Will do ...
,
Mar 22 2017
,
Mar 22 2017
,
Mar 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/23338f51f914e86486a5742184b8a793699b4157 commit 23338f51f914e86486a5742184b8a793699b4157 Author: rune <rune@opera.com> Date: Thu Mar 23 10:24:34 2017 Notify inspector of stylesheet changes when removing tree scopes. We did not update active stylesheet lists when removing a shadow since there is no collection to update. However, the inspector expects a notification since it receives a flat vector of active stylesheets from all scopes. Add a m_treeScopesRemoved member to not early return from updateActiveStyleSheets and still call probe::activeStyleSheetsUpdated. Unless other tree scopes are dirty, calling the inspector is all that happens. R=kochi@chromium.org BUG= 703427 Review-Url: https://codereview.chromium.org/2766373002 Cr-Commit-Position: refs/heads/master@{#459039} [add] https://crrev.com/23338f51f914e86486a5742184b8a793699b4157/third_party/WebKit/LayoutTests/inspector/elements/styles-3/remove-shadow-host-expected.txt [add] https://crrev.com/23338f51f914e86486a5742184b8a793699b4157/third_party/WebKit/LayoutTests/inspector/elements/styles-3/remove-shadow-host.html [modify] https://crrev.com/23338f51f914e86486a5742184b8a793699b4157/third_party/WebKit/Source/core/dom/StyleEngine.cpp [modify] https://crrev.com/23338f51f914e86486a5742184b8a793699b4157/third_party/WebKit/Source/core/dom/StyleEngine.h
,
Mar 23 2017
,
Mar 23 2017
Thanks for your quick action!
,
Mar 23 2017
lushnikov@, do you think we need merge to M57/M58? The fix is quite small and safe to merge, I guess, though the issue happens very rarely and maybe we can just wait for M59?
,
Mar 24 2017
Thanks for the fast fix! I don't think there's a need for merge. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by hayato@chromium.org
, Mar 21 2017Status: Assigned (was: Unconfirmed)