820230 - Move PaintSVGGlyph and GetSVGGlyphExtents from nsContentUtils to nsSVGUtils

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Mats Palmgren (inactive)]

tdlr: moving them to avoid nsContentUtils.h requiring gfx/ headers

PaintSVGGlyph and GetSVGGlyphExtents requires includes of
#include "gfxContext.h"
#include "gfxFont.h"

which makes any file including nsContentUtils.h depending on GFX.
For example js/xpconnect/loader/mozJSComponentLoader.cpp

It seems to me js/xpconnect depending on gfx/ is a layer violation.

Also, it makes fixing bug 786533 very hard, since std::max requires
<algorithm> which we wrap and thus fails to compile when building
in some configurations.   See bug 786533 comment 28 for details.

Comment 1

[Mats Palmgren (inactive)]

Attachment: fix

... well, also just because they really belong in nsSVGUtils.h :-)

Comment 2

[Mats Palmgren (inactive)]

I should point out that I cleaned up the code somewhat also, so you don't
miss to review that.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Mats Palmgren [:mats] from comment #0)
> It seems to me js/xpconnect depending on gfx/ is a layer violation.

Really it's js/xpconnect depending on nsContentUtils that's a layering violation.

Comment 4

[Mats Palmgren (inactive)]

OK, the only use of nsContentUtils is this:
    MOZ_ASSERT(nsContentUtils::IsCallerChrome());
(mozJSComponentLoader.cpp compiles without the #include if I remove that line)

Comment 5

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2044222a3f1d

Comment 6

[Ed Morley [:emorley]]

Backed out on suspicion of being the changeset in that push that caused:
https://tbpl.mozilla.org/php/getParsedLog.php?id=17825409&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17825388&tree=Mozilla-Inbound
etc

https://hg.mozilla.org/integration/mozilla-inbound/rev/db376fcebe62

Comment 7

[Mats Palmgren (inactive)]

Sorry about that, I had neglected to add some "b2g platforms" to my default
Try patch (fixed now).  The build problem was that gfxFont.h brings in numerous
other headers so when I removed it from nsContentUtils.h it exposed a couple of
dom/ files that didn't include the correct headers for their used APIs
(depending on gfxFont.h for mozilla::services etc).

I'll reland in a moment (with a green Try).  (It's just a an added #include
in the affected files so no need to re-review.)
https://hg.mozilla.org/try/rev/ea713cb603fd

Comment 8

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbae3f16225b
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d816bf6e05e

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/cbae3f16225b
https://hg.mozilla.org/mozilla-central/rev/1d816bf6e05e