Closed
Bug 1306212
Opened 8 years ago
Closed 8 years ago
Simplify StyleSheetInfo
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla52
| Tracking | Status | |
|---|---|---|
| firefox52 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
References
Details
Attachments
(3 files)
After bug 1304302, StyleSheetInfo becomes a simple aggregate data struct. We can simplify that struct, removing its visibility restriction as well as friend classes, and make it a member rather than a base class of ServoStyleSheet.
|
Assignee | |
Updated•8 years ago
|
Assignee: nobody → xidorn+moz
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 4•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8796005 [details] Bug 1306212 part 1 - Make StyleSheetInfo a member of ServoStyleSheet. https://reviewboard.mozilla.org/r/81984/#review80888 ::: layout/style/ServoStyleSheet.h:60 (Diff revision 1) > > private: > void DropSheet(); > > RefPtr<RawServoStyleSheet> mSheet; > + StyleSheetInfo mInner; Can we call this something other than mInner? Maybe mSheetInfo? Although the symmetry in StyleSheet::SheetInfo() looks nice, I think a "style sheet inner" is different from what the StyleSheetInfo is. (The former is nearly the entire data related to the style sheet, and the latter is just a few pieces of metadata.)
Attachment #8796005 -
Flags: review?(cam) → review+
|
|
||
Comment 5•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8796006 [details] Bug 1306212 part 2 - Remove visibility restriction of StyleSheetInfo. https://reviewboard.mozilla.org/r/81986/#review80890
Attachment #8796006 -
Flags: review?(cam) → review+
|
|
||
Comment 6•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8796007 [details] Bug 1306212 part 3 - Move the only constructor of StyleSheetInfo to StyleSheet.cpp. https://reviewboard.mozilla.org/r/81988/#review80892 I'm not sure what the advantage is of moving this constructor away from the file whose name matches the class name, but OK.
Attachment #8796007 -
Flags: review?(cam) → review+
|
|
Assignee | |
Comment 7•8 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8796007 [details] Bug 1306212 part 3 - Move the only constructor of StyleSheetInfo to StyleSheet.cpp. https://reviewboard.mozilla.org/r/81988/#review80892 That constructor is so trivial that I almost want to merge it into the header directly... But I don't want to add another dependency to that header, so moved it to another cpp.
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 11•8 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/37ea2043dfb9 part 1 - Make StyleSheetInfo a member of ServoStyleSheet. r=heycam https://hg.mozilla.org/integration/autoland/rev/6e3d1f53047f part 2 - Remove visibility restriction of StyleSheetInfo. r=heycam https://hg.mozilla.org/integration/autoland/rev/7833dc154154 part 3 - Move the only constructor of StyleSheetInfo to StyleSheet.cpp. r=heycam
|
||
Comment 12•8 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/37ea2043dfb9 https://hg.mozilla.org/mozilla-central/rev/6e3d1f53047f https://hg.mozilla.org/mozilla-central/rev/7833dc154154
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•