Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(37)

Issue 2546393002: Make setNeedsActiveStyleUpdate mark treescope dirty only. (Closed)

Created:
4 years ago by rune
Modified:
4 years ago
Reviewers:
meade_UTC10
CC:
chromium-reviews, Yoav Weiss, blink-reviews-html_chromium.org, gavinp+prerender_chromium.org, blink-reviews-style_chromium.org, blink-reviews-css, sof, eae+blinkwatch, blink-reviews-dom_chromium.org, dglazkov+blink, apavlov+blink_chromium.org, darktears, blink-reviews, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make setNeedsActiveStyleUpdate mark treescope dirty only. Pass the treeScope instead of a stylesheet pointer and remove the synchronous resolverChanged() call. Instead add the resolveChanged() calls where currently necessary and mark them for removal. This makes the setNeedsActiveStyleUpdate implementation like what we want to end up with [1]. I've done it this way to make the following CLs easier to review. Also, setNeedsActiveStyleUpdate calls are removed where we call removeStyleSheetCandidateNode() since that method already marks the tree-scope dirty. [1] https://codereview.chromium.org/1913833002/ R=meade@chromium.org BUG=567021 Committed: https://crrev.com/11bf99aa6426685ed8bf15b98a2680b02a841455 Cr-Commit-Position: refs/heads/master@{#437212}

Patch Set 1 #

Total comments: 3

Patch Set 2 : Rebased and removed unnecessary calls to setNeedsActiveStyleUpdate #

Patch Set 3 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -35 lines) Patch
M third_party/WebKit/Source/core/css/CSSStyleSheet.cpp View 1 chunk +6 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/ProcessingInstruction.cpp View 1 2 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleElement.cpp View 1 1 chunk +7 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.h View 1 2 2 chunks +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/core/dom/StyleEngine.cpp View 1 2 1 chunk +2 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLLinkElement.cpp View 1 1 chunk +3 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLStyleElement.cpp View 1 chunk +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/LinkStyle.cpp View 1 chunk +4 lines, -3 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 24 (17 generated)
meade_UTC10
I looked through this a bit to try to see why the DCHECK is being ...
4 years ago (2016-12-05 23:57:23 UTC) #5
rune
https://codereview.chromium.org/2546393002/diff/1/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp File third_party/WebKit/Source/core/html/HTMLLinkElement.cpp (right): https://codereview.chromium.org/2546393002/diff/1/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp#newcode224 third_party/WebKit/Source/core/html/HTMLLinkElement.cpp:224: document().styleEngine().setNeedsActiveStyleUpdate(treeScope()); On 2016/12/05 23:57:22, Eddy wrote: > It looks ...
4 years ago (2016-12-06 14:53:22 UTC) #6
rune
On 2016/12/06 14:53:22, rune wrote: > Anyway, needed to fix https://crbug.com/671596 first: > > https://codereview.chromium.org/2551973003/ ...
4 years ago (2016-12-06 14:55:05 UTC) #7
meade_UTC10
lgtm
4 years ago (2016-12-07 22:57:15 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2546393002/40001
4 years ago (2016-12-08 08:42:35 UTC) #19
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years ago (2016-12-08 08:47:24 UTC) #22
commit-bot: I haz the power
4 years ago (2016-12-08 08:48:44 UTC) #24
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/11bf99aa6426685ed8bf15b98a2680b02a841455
Cr-Commit-Position: refs/heads/master@{#437212}

Powered by Google App Engine
This is Rietveld 408576698