Closed
Bug 1297064
Opened 8 years ago
Closed 8 years ago
stylo: Add bindings for filling nsStyleImageLayers
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: manishearth, Assigned: manishearth)
References
Details
Attachments
(1 file)
While we can do this directly from servo, we'd have to manually call copy constructors.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → manishearth
Status: NEW → ASSIGNED
Comment 2•8 years ago
|
||
mozreview-review |
Comment on attachment 8783528 [details]
Bug 1297064 - stylo: Add bindings for filling nsStyleImageLayers lists;
https://reviewboard.mozilla.org/r/73320/#review71102
Not a formal review at all, just passed by and saw the style nits :P
::: layout/style/ServoBindings.cpp:281
(Diff revision 1)
> }
> return attr->GetServoCSSDeclarationValue();
> }
>
> +void
> +Gecko_FillAllBackgroundLists(nsStyleImageLayers* aLayers, uint32_t aMaxLen) {
nit: brace in its own line.
::: layout/style/nsRuleNode.cpp:7189
(Diff revision 1)
> aLayers[destLayer].mPosition.*aResultLocation =
> aLayers[sourceLayer].mPosition.*aResultLocation;
> }
> }
>
> +void FillAllBackgroundLists(nsStyleImageLayers& aImage,
nit: return type in its own line, brace too.
Comment hidden (mozreview-request) |
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8783528 [details]
Bug 1297064 - stylo: Add bindings for filling nsStyleImageLayers lists;
https://reviewboard.mozilla.org/r/73320/#review72334
::: layout/style/ServoBindings.h:244
(Diff revision 2)
> void Gecko_CopyClipPathValueFrom(mozilla::StyleClipPath* dst, const mozilla::StyleClipPath* src);
>
> void Gecko_DestroyClipPath(mozilla::StyleClipPath* clip);
> mozilla::StyleBasicShape* Gecko_NewBasicShape(mozilla::StyleBasicShapeType type);
>
> +void Gecko_FillAllBackgroundLists(nsStyleImageLayers* layers, uint32_t maxLen);
Nit: "max_len" so we generate a Rust friendly argument name?
::: layout/style/nsRuleNode.h:1070
(Diff revision 2)
> +void FillAllBackgroundLists(nsStyleImageLayers& aLayers,
> + uint32_t aMaxItemCount);
Please make this a public static method of nsRuleNode. (We should avoid having global namespace functions. Putting it in |namespace mozilla| would be OK too, but I prefer sticking it on nsRuleNode so that callers can see where it's coming from / what it's related to.)
::: layout/style/nsRuleNode.cpp:7192
(Diff revision 2)
> }
>
> +void
> +FillAllBackgroundLists(nsStyleImageLayers& aImage, uint32_t aMaxItemCount)
> +{
> + // Delete any extra items. We need to keep layers in which any
Two space indent.
Attachment #8783528 -
Flags: review?(cam) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Pushed by manishearth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/69fcb7b0e100
stylo: Add bindings for filling nsStyleImageLayers lists; r=heycam
Comment 9•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Updated•8 years ago
|
Blocks: stylo-nightly
You need to log in
before you can comment on or make changes to this bug.
Description
•