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

Issue 591599 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
NOT IN USE
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Changing text of inspector stylesheet calls resolverChanged(FullStyleUpdate) three times

Reported by r...@opera.com, Mar 3 2016

Issue description

InspectorStyleSheet::setText() causes resolverChanged() to be called three times through an explicit call and two mutation scopes.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 3 2016

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

commit 5958ca8683dd9478a780d516205355a36c48c5aa
Author: rune <rune@opera.com>
Date: Thu Mar 03 07:41:19 2016

One instead of three resolverChanged replacing source in inspector.

resolverChanged() was called three times when replacing the stylesheet
text from the inspector. Two mutation scopes and an explicit call at
the end.

Kept one of the mutation scopes. Two shouldn't be necessary, and I have
confirmed that the crash tests for which this was justified earlier
don't crash when removing one of the scopes.

Moved the stylesheet modifications into CSSStyleSheet::setText().

Also moved clearing of the CSSOM wrappers before the mutation scope
declaration as the mutation scope constructor would unnecessarily
re-attach wrappers which would then be cleared right after.

R=esprehn@chromium.org,pfeldman@chromium.org
BUG= 591599 

Review URL: https://codereview.chromium.org/1765463002

Cr-Commit-Position: refs/heads/master@{#378978}

[modify] https://crrev.com/5958ca8683dd9478a780d516205355a36c48c5aa/third_party/WebKit/Source/core/css/CSSStyleSheet.cpp
[modify] https://crrev.com/5958ca8683dd9478a780d516205355a36c48c5aa/third_party/WebKit/Source/core/css/CSSStyleSheet.h
[modify] https://crrev.com/5958ca8683dd9478a780d516205355a36c48c5aa/third_party/WebKit/Source/core/inspector/InspectorStyleSheet.cpp

Comment 2 by r...@opera.com, Mar 3 2016

Status: Fixed (was: Started)
Project Member

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

Labels: merge-merged-2666
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5958ca8683dd9478a780d516205355a36c48c5aa

commit 5958ca8683dd9478a780d516205355a36c48c5aa
Author: rune <rune@opera.com>
Date: Thu Mar 03 07:41:19 2016

One instead of three resolverChanged replacing source in inspector.

resolverChanged() was called three times when replacing the stylesheet
text from the inspector. Two mutation scopes and an explicit call at
the end.

Kept one of the mutation scopes. Two shouldn't be necessary, and I have
confirmed that the crash tests for which this was justified earlier
don't crash when removing one of the scopes.

Moved the stylesheet modifications into CSSStyleSheet::setText().

Also moved clearing of the CSSOM wrappers before the mutation scope
declaration as the mutation scope constructor would unnecessarily
re-attach wrappers which would then be cleared right after.

R=esprehn@chromium.org,pfeldman@chromium.org
BUG= 591599 

Review URL: https://codereview.chromium.org/1765463002

Cr-Commit-Position: refs/heads/master@{#378978}

[modify] https://crrev.com/5958ca8683dd9478a780d516205355a36c48c5aa/third_party/WebKit/Source/core/css/CSSStyleSheet.cpp
[modify] https://crrev.com/5958ca8683dd9478a780d516205355a36c48c5aa/third_party/WebKit/Source/core/css/CSSStyleSheet.h
[modify] https://crrev.com/5958ca8683dd9478a780d516205355a36c48c5aa/third_party/WebKit/Source/core/inspector/InspectorStyleSheet.cpp

Sign in to add a comment