New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 633210 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
NOT IN USE
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 628488



Sign in to add a comment

StyleSheetContents cache for style elements issue when mutating disconnected sheets

Reported by r...@opera.com, Aug 1 2016

Issue description

If you have two stylesheets sharing StyleSheetContents based on <style> element text, we unregisterClient the style element when removed from the document but still keep a reference to the StyleSheetContents. If that StyleSheetContents is referenced from javascript and mutated, the contents is still shared with the in-document style element, yet we think we don't need to copy-on-write because the client count is 1 due to unregisterClient.

 

Comment 1 by r...@opera.com, Aug 1 2016

stylecache2.html
344 bytes View Download

Comment 2 by r...@opera.com, Aug 1 2016

Blocking: 628488
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/473ac0866d46d55bf57293ce3bb27870f672dc40

commit 473ac0866d46d55bf57293ce3bb27870f672dc40
Author: rune <rune@opera.com>
Date: Wed Aug 03 12:18:01 2016

Use weak members to cache StyleSheetContents.

We used the client count to detect if we could remove a
StyleSheetContents from the StyleEngine cache or not. The problem is
that the client references are removed when the element is removed from
the DOM, but the StyleSheetContents is still referenced from the
CSSStyleSheet which is accessible from CSSOM. That caused bugs with
StyleSheetContents being marked as mutable without removing it from the
cache causing assertions, and mutating the sheet without copy-on-write
because we thought we only had a single client for the contents.

Instead use weak members in the cache and let garbage collection delete
the StyleSheetContents when no longer referenced. Also, add a flag to
StyleSheetContents to say that it is referenced by multiple sheets when
we use and already cached object instead of incorrectly relying on
client count.

R=timloh@chromium.org,haraken@chromium.org
BUG= 633210 , 628488 

Review-Url: https://codereview.chromium.org/2205843003
Cr-Commit-Position: refs/heads/master@{#409495}

[add] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/LayoutTests/fast/css/modify-cached-detached-sheet-2.html
[add] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/LayoutTests/fast/css/modify-cached-detached-sheet.html
[modify] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/Source/core/css/CSSStyleSheet.cpp
[modify] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/Source/core/css/StyleSheetContents.cpp
[modify] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/Source/core/css/StyleSheetContents.h
[modify] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/Source/core/dom/StyleElement.h
[modify] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/Source/core/dom/StyleEngine.cpp
[modify] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/Source/core/dom/StyleEngine.h
[modify] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp
[modify] https://crrev.com/473ac0866d46d55bf57293ce3bb27870f672dc40/third_party/WebKit/Source/core/html/HTMLStyleElement.h

Comment 4 by r...@opera.com, Aug 3 2016

Status: Fixed (was: Started)

Sign in to add a comment