Closed Bug 1333505 Opened 3 years ago Closed 3 years ago

Do some cleanup from bug 1328602

Categories

(Core :: Graphics: WebRender, defect, P3)

53 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla54

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(2 files)

I did a pass through the code added in bug 1328602 (extracting the WR render thread) and had a bunch of nits and stuff. This bug is to fix those.
Comment on attachment 8829961 [details]
Bug 1333505 - Some cosmetic cleanup (comments/whitespace/alpha-sorting).

https://reviewboard.mozilla.org/r/106920/#review108006
Attachment #8829961 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 8829962 [details]
Bug 1333505 - Some minor code cleanup.

https://reviewboard.mozilla.org/r/106922/#review108014

::: gfx/webrender_bindings/WebRenderAPI.cpp:86
(Diff revision 1)
>                                              mBridge);
>  
>      aRenderThread.AddRenderer(aWindowId, Move(renderer));
>    }
>  
> +private:

I don't think it's worth using private in a class that is only visible to a C++ file (I much prefer file/module-level encapsulation than class-level).
But that doesn't make much different in this case.
Attachment #8829962 - Flags: review?(nical.bugzilla) → review+
(In reply to Nicolas Silva [:nical] from comment #4)
> I don't think it's worth using private in a class that is only visible to a
> C++ file (I much prefer file/module-level encapsulation than class-level).
> But that doesn't make much different in this case.

In general I agree that it doesn't make much difference here. I was just thinking that over time as we add more of these we might want to extract them to another file and it's better to put the private in now than forget to do it later. There doesn't seem to be any harm from adding it, at least.
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/7115c4882705
Some cosmetic cleanup (comments/whitespace/alpha-sorting). r=nical
https://hg.mozilla.org/projects/graphics/rev/68b72e1b2195
Some minor code cleanup. r=nical
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.