Closed Bug 1321056 Opened 3 years ago Closed 3 years ago

Preemptively fix unified bustage in layout/svg

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file)

When reviewing the patches in bug 1304598, I was inspired to preemptively fix unified bustage in a few places, so that changes like bug 1304598 will be easier in the future.

We can reveal latent-unified-bustage by simply tweaking moz.build files to replace UNIFIED_SOURCES with SOURCES. I did that in layout/svg and fixed the resulting bustage; patch coming up.
TYLin, I'll tag you to review this -- hope that's OK with you!
Assignee: nobody → dholbert
* All of the new #includes here are for types of the same name that are directly used in the file that I'm tweaking. We either call functions on a variable of that type (e.g. SVGSVGElement.h in one case, for a call to svgElement->GetViewportSize()), or we return that type by value (e.g. for DrawResult), or we call static methods on that type (e.g. for nsSVGUtils))
* The "using namespace mozilla::image" sprinkled throughout the patch is for DrawResult usages (which live in mozilla::image)
* I'm adding explicit an namespace to IntPoint in nsSVGClipPathFrame.h (which is currently accidentally-relying on "using namespace" declarations that get picked up from other .cpp files right now).

Let me know if anything else isn't clear here. Thanks!
(In reply to Daniel Holbert [:dholbert] from comment #0)
> We can reveal latent-unified-bustage by simply tweaking moz.build files to
> replace UNIFIED_SOURCES with SOURCES. 

This is a good idea!
Comment on attachment 8815429 [details]
Bug 1321056: Add includes/namespaces to fix latent unified bustage in layout/svg.

https://reviewboard.mozilla.org/r/96326/#review96658

The explanation in comment 3 is clear to me. r=TYLin

::: layout/svg/nsCSSClipPathInstance.cpp:16
(Diff revision 1)
> +#include "nsSVGElement.h"
> +#include "nsSVGUtils.h"
> +#include "nsSVGViewBox.h"

Nit: Put the #includes after "nsRuleNode.h" so that they are in alphabetic order.
Attachment #8815429 - Flags: review?(tlin) → review+
Comment on attachment 8815429 [details]
Bug 1321056: Add includes/namespaces to fix latent unified bustage in layout/svg.

https://reviewboard.mozilla.org/r/96326/#review96658

> Nit: Put the #includes after "nsRuleNode.h" so that they are in alphabetic order.

I got the alphabet backwards in my head. :D

Fixed -- thanks!
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9953c71eb9ba
Add includes/namespaces to fix latent unified bustage in layout/svg. r=TYLin
https://hg.mozilla.org/mozilla-central/rev/9953c71eb9ba
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.