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

Issue 796713002: Turn StyleSharing to 11. (Closed)

Created:
6 years ago by esprehn
Modified:
6 years ago
Reviewers:
eseidel, ojan
CC:
abarth-chromium, mojo-reviews_chromium.org, ojan
Base URL:
git@github.com:domokit/mojo.git@master
Project:
mojo
Visibility:
Public.

Description

Turn StyleSharing to 11. In Sky we can style share if our TreeScopes have the same styles, our :host styles are the same, and we'd inherit the same styles. This allows a lot of simplification to the style sharing logic since we don't need to deal with descendant selectors or tree boundary crossing rules: - We can remove the logic that was checking that we were distributed to the same insertion points since there's no ::content selectors. - We can check the actual inherited values instead of looking at the parentOrSHadowHostNode(). We used to look at the node in Blink because we were checking that you'd get the same descendant selectors applied. In Sky we instead want to make sure you'd inherit the same values. This also means we don't need the element().parentOrShadowHostElement() != parent case in the SharedStyleFinder which was trying to deal with descendant selectors again. I also removed the checks that were redundant with the checks inside supportsStyleSharing() which we always check before adding sharing candidates. Finally by refactoring the code to make the TreeScope style check work it exposed that the Document::styleSheets() and TreeScope::styleSheets() APIs are now dead. A future patch will delete the now dead StyleSheetList class as well. This change makes the city-list application share between all the items in the list, and all of the headers of the same type now share as well. R=ojan@chromium.org Committed: https://chromium.googlesource.com/external/mojo/+/bbf64f5de00c7ef4e127e0a43ff9cb15e1e3470a

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -91 lines) Patch
M sky/engine/core/css/resolver/SharedStyleFinder.h View 2 chunks +2 lines, -0 lines 0 comments Download
M sky/engine/core/css/resolver/SharedStyleFinder.cpp View 2 chunks +12 lines, -34 lines 4 comments Download
M sky/engine/core/css/resolver/StyleResolver.cpp View 1 chunk +1 line, -0 lines 0 comments Download
M sky/engine/core/dom/Document.h View 2 chunks +0 lines, -4 lines 0 comments Download
M sky/engine/core/dom/Document.cpp View 2 chunks +0 lines, -9 lines 0 comments Download
M sky/engine/core/dom/Element.cpp View 1 chunk +1 line, -8 lines 0 comments Download
M sky/engine/core/dom/TreeScope.h View 2 chunks +2 lines, -1 line 0 comments Download
M sky/engine/core/dom/TreeScope.cpp View 2 chunks +19 lines, -0 lines 0 comments Download
M sky/engine/core/dom/shadow/ElementShadow.cpp View 1 chunk +5 lines, -17 lines 0 comments Download
M sky/engine/core/dom/shadow/ShadowRoot.h View 1 chunk +0 lines, -2 lines 0 comments Download
M sky/engine/core/dom/shadow/ShadowRoot.cpp View 2 chunks +0 lines, -11 lines 0 comments Download
M sky/engine/core/dom/shadow/ShadowRootRareData.h View 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
esprehn
6 years ago (2014-12-11 02:44:21 UTC) #2
ojan
lgtm https://codereview.chromium.org/796713002/diff/1/sky/engine/core/css/resolver/SharedStyleFinder.cpp File sky/engine/core/css/resolver/SharedStyleFinder.cpp (left): https://codereview.chromium.org/796713002/diff/1/sky/engine/core/css/resolver/SharedStyleFinder.cpp#oldcode135 sky/engine/core/css/resolver/SharedStyleFinder.cpp:135: if (candidate.inlineStyle()) Why don't we need this check? ...
6 years ago (2014-12-11 19:35:54 UTC) #3
esprehn
https://codereview.chromium.org/796713002/diff/1/sky/engine/core/css/resolver/SharedStyleFinder.cpp File sky/engine/core/css/resolver/SharedStyleFinder.cpp (left): https://codereview.chromium.org/796713002/diff/1/sky/engine/core/css/resolver/SharedStyleFinder.cpp#oldcode135 sky/engine/core/css/resolver/SharedStyleFinder.cpp:135: if (candidate.inlineStyle()) On 2014/12/11 at 19:35:54, ojan wrote: > ...
6 years ago (2014-12-11 23:09:59 UTC) #4
esprehn
6 years ago (2014-12-11 23:34:30 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
bbf64f5de00c7ef4e127e0a43ff9cb15e1e3470a (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698