1321056 - Preemptively fix unified bustage in layout/svg

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

TYLin, I'll tag you to review this -- hope that's OK with you!

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: Bug 1321056: Add includes/namespaces to fix latent unified bustage in layout/svg.

Review commit: https://reviewboard.mozilla.org/r/96326/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96326/

Comment 3

[Daniel Holbert [: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!

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

(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 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

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.

Comment 6

[Daniel Holbert [:dholbert]]

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!

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 8815429 [details]
Bug 1321056: Add includes/namespaces to fix latent unified bustage in layout/svg.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96326/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9953c71eb9ba
Add includes/namespaces to fix latent unified bustage in layout/svg. r=TYLin

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9953c71eb9ba