Closed
Bug 1296186
Opened 8 years ago
Closed 8 years ago
stylo: Make ServoDeclarationBlock refcounted
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
References
Details
Attachments
(2 files)
css::Declaration is refcounted, so that multiple nsAttrValues can hold the same css::Declaration at the same time. The existing code path heavily relies on this, so having a sole owned ServoDeclarationBlock sharing the same path could be very dangerous. Thus we should create a refcounted wrapper for it and have it owned by nsAttrValue instead.
Assignee | ||
Updated•8 years ago
|
Summary: stylo: Create a refcounted wrapper for ServoDeclarationBlock → stylo: Make ServoDeclarationBlock refcounted
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•8 years ago
|
||
Comment on attachment 8782784 [details] Bug 1296186 part 1 - Make borrowed type just alias of corresponding pointer type. I haven't looked at this patch yet, but Manish just implemented this stuff, so I'll let him weigh in here on the proposed changes before I do so.
Attachment #8782784 -
Flags: feedback?(manishearth)
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8782784 [details] Bug 1296186 part 1 - Make borrowed type just alias of corresponding pointer type. https://reviewboard.mozilla.org/r/72834/#review70886 ::: layout/style/ServoBindings.h:59 (Diff revision 1) > class nsStyleCoord; > struct nsStyleDisplay; > struct ServoDeclarationBlock; > > +#define DECL_REF_TYPE_FOR(type_) \ > + typedef type_* type_##Borrowed; \ I had a new type because there's a bit of extra safety on the Rust side -- the bindings will show a different type rather than a pointer typedef, and you deal with that explicitly (by adding it to regen.py). But that's not much additional safety, so this is ok.
Comment 5•8 years ago
|
||
Comment on attachment 8782784 [details] Bug 1296186 part 1 - Make borrowed type just alias of corresponding pointer type. While this is slightly less explicit, I'm okay with this. It's also more interoperable, which is good.
Attachment #8782784 -
Flags: feedback?(manishearth) → feedback+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8782784 [details] Bug 1296186 part 1 - Make borrowed type just alias of corresponding pointer type. https://reviewboard.mozilla.org/r/72834/#review70886 > I had a new type because there's a bit of extra safety on the Rust side -- the bindings will show a different type rather than a pointer typedef, and you deal with that explicitly (by adding it to regen.py). But that's not much additional safety, so this is ok. As far as we give it a different name (even with typedef), we can deal with it explicitly on the Rust side as well. (I've made bindgen support blacklisting type alias.)
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8782784 [details] Bug 1296186 part 1 - Make borrowed type just alias of corresponding pointer type. https://reviewboard.mozilla.org/r/72834/#review71122 Ok then.
Attachment #8782784 -
Flags: review?(bobbyholley) → review+
Comment 10•8 years ago
|
||
mozreview-review |
Comment on attachment 8782785 [details] Bug 1296186 part 2 - Make ServoDeclarationBlock refcounted. https://reviewboard.mozilla.org/r/72836/#review71126 This one looks fine as well. Stealing review.
Attachment #8782785 -
Flags: review+
Updated•8 years ago
|
Attachment #8782785 -
Flags: review?(cam)
Comment 11•8 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8c7a0cfd5ae1 part 1 - Make borrowed type just alias of corresponding pointer type. r=bholley https://hg.mozilla.org/integration/mozilla-inbound/rev/db66eec34981 part 2 - Make ServoDeclarationBlock refcounted. r=heycam
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8c7a0cfd5ae1 https://hg.mozilla.org/mozilla-central/rev/db66eec34981
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•