Closed Bug 1407753 Opened 4 years ago Closed 4 years ago

nsDisplayFieldSetBorder falls back on Gmail

Categories

(Core :: Graphics: WebRender, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- unaffected
firefox58 --- unaffected

People

(Reporter: jrmuizel, Assigned: Gankra)

References

Details

(Whiteboard: [wr-mvp])

Attachments

(3 files)

No description provided.
Blocks: 1407744
Whiteboard: [wr-mvp] [triage]
Priority: P3 → P2
Whiteboard: [wr-mvp] [triage] → [wr-mvp]
Assignee: nobody → kechen
Status: NEW → ASSIGNED
Priority: P2 → P1
Alexis has actually already been looking at this, but was refactoring the border code first. I'll let him choose whether he wants to steal the assignment. That being said, this should probably still block on the border code refactor.
Flags: needinfo?(a.beingessner)
Yeah I'd like to at least land the border refactoring first. To do that we need to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=1408690.

This is also partially blocked on https://github.com/servo/webrender/pull/1867, in that we can't implement FieldSets with Legends without it.
Flags: needinfo?(a.beingessner)
After some discussion, I've done the refactor in this bug, with border-images explicitly disabled. I've filed bugs under https://bugzilla.mozilla.org/show_bug.cgi?id=1408690 to track re-enabling them.

try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b4665d609203e449a80ca5661ff11536503736d
Assignee: kechen → a.beingessner
Comment on attachment 8919824 [details]
Bug 1407753 - Factor out webrender border code for reuse.

https://reviewboard.mozilla.org/r/190760/#review195938


C/C++ static analysis found 1 defect in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: layout/painting/nsCSSRendering.cpp:700
(Diff revision 1)
>  
> +
> +bool
> +nsCSSRendering::CreateWebRenderCommandsForBorder(nsDisplayItem* aItem,
> +                                                 nsIFrame* aForFrame,
> +                                                 nsRect aBorderArea,

Warning: The parameter 'aborderarea' is copied for each invocation but only used as a const reference; consider making it a const reference [clang-tidy: performance-unnecessary-value-param]
Comment on attachment 8919825 [details]
Bug 1407753 - Port nsFieldSetFrame to webrender.

https://reviewboard.mozilla.org/r/190762/#review195940


C/C++ static analysis found 1 defect in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: layout/forms/nsFieldSetFrame.cpp:99
(Diff revision 1)
>                       gfxContext* aCtx) override;
>    virtual nsDisplayItemGeometry* AllocateGeometry(nsDisplayListBuilder* aBuilder) override;
>    virtual void ComputeInvalidationRegion(nsDisplayListBuilder* aBuilder,
>                                           const nsDisplayItemGeometry* aGeometry,
>                                           nsRegion *aInvalidRegion) const override;
> +  virtual bool CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,

Warning: 'virtual' is redundant since the function is already declared 'override' [clang-tidy: modernize-use-override]
Fixed up static analysis
Comment on attachment 8919850 [details]
Bug 1407753 - Adjust test expectations for fieldset impl.

https://reviewboard.mozilla.org/r/190790/#review195994
Attachment #8919850 - Flags: review?(bugmail) → review+
Comment on attachment 8919824 [details]
Bug 1407753 - Factor out webrender border code for reuse.

https://reviewboard.mozilla.org/r/190760/#review196070
Attachment #8919824 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8919825 [details]
Bug 1407753 - Port nsFieldSetFrame to webrender.

https://reviewboard.mozilla.org/r/190762/#review196072
Attachment #8919825 - Flags: review?(jmuizelaar) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3e8f17021e80
Factor out webrender border code for reuse. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/d0aec70092eb
Port nsFieldSetFrame to webrender. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/7b5942e835f8
Adjust test expectations for fieldset impl. r=kats
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.