Page MenuHomePhabricator
Differential D32726

Bug 1554716 - Remove nsStyleColor moving the color property to nsStyleText. r=#style,jfkthame
ClosedPublic
Actions

Authored by emilio on May 27 2019, 3:36 PM.

Details

Reviewers
jfkthame
Group Reviewers
Restricted Project
Commits
rMOZILLACENTRAL48cf67b7be7d: Bug 1554716 - Remove nsStyleColor moving the color property to nsStyleText.
Bugzilla Bug ID
1554716
Summary

I think this is a good change regardless of other discussion in bug 1552587. If
we decide to move mColor to the top-level of the struct that can be done
separately.

Diff Detail

Repository
rMOZILLACENTRAL mozilla-central
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

emilio created this revision.May 27 2019, 3:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2019, 3:36 PM
Harbormaster completed remote builds in B76945: Diff 110489.May 27 2019, 3:36 PM
phab-bot requested review of this revision.May 27 2019, 3:37 PM
phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: Restricted Project.
emilio updated this revision to Diff 110595.May 27 2019, 7:40 PM

Fix windows build

emilio edited the summary of this revision. (Show Details)May 27 2019, 7:40 PM
Harbormaster completed remote builds in B77032: Diff 110595.May 27 2019, 7:40 PM
emilio updated this revision to Diff 110604.May 27 2019, 8:06 PM
emilio edited the summary of this revision. (Show Details)

Fix rust test size

emilio edited the summary of this revision. (Show Details)May 27 2019, 8:06 PM
Harbormaster completed remote builds in B77036: Diff 110604.May 27 2019, 8:06 PM
jfkthame added a comment.May 30 2019, 2:49 PM

The one thing I wonder is whether this means we'll be computing an entire Text struct in cases where previously we didn't need it. Have you tried to figure out (either by auditing usage of StyleColor() vs StyleText() or by instrumentation) how often that happens? I'm sure in many cases we use both anyway, but that may not always be true.

jfkthame accepted this revision.May 30 2019, 3:17 PM

Emilio reminded me on irc that we always compute all the structs, so that's not really a concern here. So this LGTM.

This revision is now accepted and ready to land.May 30 2019, 3:17 PM
emilio updated this revision to Diff 112762.May 31 2019, 2:48 PM
emilio edited the summary of this revision. (Show Details)

Update property database since color appears in a different order now

emilio edited the summary of this revision. (Show Details)May 31 2019, 2:48 PM
Harbormaster completed remote builds in B78616: Diff 112762.May 31 2019, 2:48 PM
Closed by commit rMOZILLACENTRAL48cf67b7be7d: Bug 1554716 - Remove nsStyleColor moving the color property to nsStyleText. (authored by emilio). · Explain WhyMay 31 2019, 2:55 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
accessible/
base/
6 lines
windows/
ia2/
2 lines
devtools/
shared/
css/
generated/
2 lines
dom/
canvas/
2 lines
editor/
libeditor/
2 lines
layout/
forms/
2 lines
generic/
2 lines
4 lines
2 lines
painting/
2 lines
style/
1 line
4 lines
4 lines
17 lines
41 lines
1 line
svg/
2 lines
xul/
4 lines
tree/
4 lines
servo/
components/
style/
2 lines
properties/
1 line
3 lines
longhands/
10 lines
2 lines
values/
specified/
2 lines
ports/
geckolib/
tests/
2 lines

Diff 112766

View Options

accessible/base/TextAttrs.cpp

Loading...
View Options

accessible/windows/ia2/ia2AccessibleComponent.cpp

Loading...
View Options

devtools/shared/css/generated/properties-db.js

Loading...
View Options

dom/canvas/CanvasRenderingContext2D.cpp

Loading...
View Options

editor/libeditor/HTMLAbsPositionEditor.cpp

Loading...
View Options

layout/forms/nsComboboxControlFrame.cpp

Loading...
View Options

layout/generic/nsBulletFrame.cpp

Loading...
View Options

layout/generic/nsCanvasFrame.cpp

Loading...
View Options

layout/generic/nsImageFrame.cpp

Loading...
View Options

layout/painting/nsDisplayList.cpp

Loading...
View Options

layout/style/ComputedStyle.cpp

Loading...
View Options

layout/style/StyleColor.cpp

Loading...
View Options

layout/style/nsCSSVisitedDependentPropList.h

Loading...
View Options

layout/style/nsStyleStruct.h

Loading...
View Options

layout/style/nsStyleStruct.cpp

Loading...
View Options

layout/style/nsStyleStructList.h

Loading...
View Options

layout/svg/nsFilterInstance.cpp

Loading...
View Options

layout/xul/nsTextBoxFrame.cpp

Loading...
View Options

layout/xul/tree/nsTreeBodyFrame.cpp

Loading...
View Options

servo/components/style/matching.rs

Loading...
View Options

servo/components/style/properties/build.py

Loading...
View Options

servo/components/style/properties/gecko.mako.rs

Loading...
View Options

servo/components/style/properties/longhands/color.mako.rs

Loading...
View Options

servo/components/style/properties/longhands/inherited_text.mako.rs

Loading...
View Options

servo/components/style/properties/properties.mako.rs

Loading...
View Options

servo/components/style/values/specified/color.rs

Loading...
View Options

servo/ports/geckolib/tests/size_of.rs

Loading...
Privacy · Cookies · Legal