1345696 - stylo: Implement access to CSSFontFaceRule

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P1)

Comment 1

[Bobby Holley (:bholley)]

Xidorn - Jonathan is going to be looking at the @font-face stuff in bug 1281962. IIUC he'll need the CSSOM bits in this bug as well, though they're otherwise mostly-separate tasks. Does that sound right?

If so, are the CSSOM bits something you can knock out without much trouble? Assuming Jonathan needs it to test is stuff in bug 1281962, it's probably worth doing this on the sooner side.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

I believe you mean bug 1290237.

From my previous investigation (bug 1290237 comment 3), it seems to me our internal usage of @font-face basically depends on CSSOM interfaces of CSSFontFaceRule, so I would say bug 1290237 is going to be completely blocked by this bug, and maybe I can do all of them in this bug together, since I don't think there are much remaining work after implementing the CSSOM bits.

Comment 3

[Bobby Holley (:bholley)]

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #2)
> I believe you mean bug 1290237.

Sorry, clipboard fail. Too many bugs flying around.
 
> From my previous investigation (bug 1290237 comment 3), it seems to me our
> internal usage of @font-face basically depends on CSSOM interfaces of
> CSSFontFaceRule, so I would say bug 1290237 is going to be completely
> blocked by this bug, and maybe I can do all of them in this bug together,
> since I don't think there are much remaining work after implementing the
> CSSOM bits.

Ok sounds good. Let's have you implement the CSSOM bits, and then if there's any non-trivial amount of followup work we can hand that off the Jonathan.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

It seems to me that @font-face rules are mainly used by Gecko side, and Servo side doesn't really need that information, so I'm considering a different approach for integrating it.

I want to have this rule backed by Gecko class in Servo side. That says, I'd like to have Servo code fill the nsCSSValues of Gecko's nsCSSFontFaceRule directly, and make Servo's FontFaceRule just hold a pointer to the corresponding Gecko rule object (and own that object).

This is quite different from our previous approach that data is hold in Servo side, and Gecko class is just a thin wrapper of Servo object to C++.

Do you think there would be any problem with this approach?

The main problem to be solved is that, we need to support collecting all effective @font-face rules, and mapping them to FontFace object if existed when flushing user font set. And flushing user font set could happen quite frequently (although I suspect whether it really needs to, which might be something optimizable in Gecko side).

For supporting that, we have several choices:
(1) traverse the Gecko CSSOM tree for collecting
(2) traverse by Servo code, and we create C++ wrapper for them when collecting
(3) traverse by Servo code, and we make FontFace object able to be backed by Servo rule object
(4) hold data in C++ object, traverse by Servo code, and Servo returns C++ objects directly

Choice (3) is probably doable, but I'm a bit concerned about the performance consequence of getting descriptor value, especially font-family (which is a string) and src (which is a list of url / string). Generating string from Servo to Gecko is especially inefficient as we've seen in properties.

Choice (2) would be worse than (3) because it adds the extra cost to create wrapper object for every flushing.

Choice (1) is probably the worst that we need to duplicate the logic of stylesheet::effective_rules in C++ code, and we need to expand all group rules in CSSOM tree. Also it seems we don't have a fast way to check whether a media rule is active outside cascading at all.

So I think (4) would be the best choice, and may require the least code change in Gecko side since we can just reuse all the infrastructure based on the existing nsCSSFontFaceRule.

The only problem is that it is quite different than what we have been doing, so I want to confirm if there is any concern about this approach.

Comment 5

[Simon Sapin (:SimonSapin)]

Call "parse_font_face_block() -> Result<FontFaceRule, ()>", then instead of wrapping into Arc<Locked<FontFaceRule>> convert to nsCSSFontFaceRule at parse time and keep a gecko reference-counted pointer in CssRule? Sounds good.

I suppose CSSOM implementation for CSSFontFaceRule would be almost entirely on the Gecko side. Stylo can provide function for parsing values of individual descriptors, to avoid duplicating parsing code between CSSFontFaceRule setters and stylesheet parsing.

Comment 6

[Bobby Holley (:bholley)]

Sounds fine to me!

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Simon Sapin (:SimonSapin) from comment #5)
> I suppose CSSOM implementation for CSSFontFaceRule would be almost entirely
> on the Gecko side. Stylo can provide function for parsing values of
> individual descriptors, to avoid duplicating parsing code between
> CSSFontFaceRule setters and stylesheet parsing.

CSSFontFaceRule doesn't really have much CSSOM things to do at the moment. It doesn't support mutating at all in Gecko, either via CSSOM or via FontFace.

This is something we should fix, but not for now, I suppose.

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

Almost... https://treeherder.mozilla.org/#/jobs?repo=try&revision=bfe2638dee4aa20304cccf99047ef0b8e6946aa2

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 1 - Lots of fixup for the next patch.

The next patch moves nsCSSFontFaceRule into a separate header, which
somehow affects lots of header dependencies. I'm not completely sure
why this happens, though.

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

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 2 - Move nsCSSFontFaceRule to a separate header.

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

Comment 11

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 3 - Add FFI for nsCSSFontFaceRule.

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

Comment 12

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Rename font_face::FontFaceRule to FontFaceData.

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

Comment 13

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 4 - Add function for sugar of nsCSSValue.

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

Comment 14

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Use Gecko nsCSSFontFaceRule for font face rule.

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

Comment 15

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 5 - Support CSSOM access to @font-face rules.

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

Comment 16

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 6 - Fix cycle collection for font-face rule.

Having Servo's FontFaceRule owning Gecko's nsCSSFontFaceRule object opens an
untracked edge, because we generally don't track the reference through Servo
objects, so there is a chance that nsCSSFontFaceRule can form a undroppable
reference cycle.

This patch adds a workaround that we track @font-face rule twice in CSS rule
list. This is not perfect, but the idea is that, if someone wants to put some
reference on the @font-face rule, the rule itself and its parent list should
have been constructed in the CSSOM tree. If they are not there, we are
probably safe from cycle reference.

For @font-face rule, that assumption isn't strictly true. Script can still
get a font-face rule via layout inspector's nsIDOMFontFace::rule. If some
script puts an object which references the stylesheet of the rule or any of
its parent rule / rule list into font-face rule's expando, we would leak
again... But as far as that is an internal interface, we are probably safe?

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

Comment 17

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Record effective @font-face rules when updating stylist.

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

Comment 18

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 7 - Provide @font-face rules for stylo backend.

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

Comment 19

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 8 - Make test_redundant_font_download.html more robust.

I don't know why... but it seems we do not trigger the font download if
we don't ask a layout explicitly here for e10s, which makes this test
fail on e10s only with the patches before.

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

Comment 20

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1345696 part 8 - Update test expectations.

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

Comment 21

[Cameron McCormack (:heycam)]

Comment on attachment 8851908 [details]
Bug 1345696 part 1 - Lots of fixup for the next patch.

https://reviewboard.mozilla.org/r/124148/#review126682

Comment 22

[Cameron McCormack (:heycam)]

Comment on attachment 8851909 [details]
Bug 1345696 part 2 - Move nsCSSFontFaceRule to a separate header.

https://reviewboard.mozilla.org/r/124150/#review126684

::: layout/style/FontFace.cpp:16
(Diff revision 1)
>  #include "mozilla/dom/Promise.h"
>  #include "mozilla/dom/TypedArray.h"
>  #include "mozilla/dom/UnionTypes.h"
>  #include "mozilla/CycleCollectedJSContext.h"
>  #include "nsCSSParser.h"
> -#include "nsCSSRules.h"
> +#include "nsCSSFontFaceRule.h"

Nit: Move this one line up?

Comment 23

[Cameron McCormack (:heycam)]

Comment on attachment 8851917 [details]
Bug 1345696 part 7 - Provide @font-face rules for stylo backend.

https://reviewboard.mozilla.org/r/124166/#review126692

Comment 24

[Cameron McCormack (:heycam)]

Comment on attachment 8851918 [details]
Bug 1345696 part 8 - Make test_redundant_font_download.html more robust.

https://reviewboard.mozilla.org/r/124168/#review126694

I'm worried this is hiding a real bug. :(  Can you investigate to find out what's happening here?

Comment 25

[Cameron McCormack (:heycam)]

Comment on attachment 8851919 [details]
Bug 1345696 part 8 - Update test expectations.

https://reviewboard.mozilla.org/r/124170/#review126696

Nice...

Comment 26

[Jonathan Kew [:jfkthame]]

(In reply to Cameron McCormack (:heycam) from comment #24)
> Comment on attachment 8851918 [details]
> Bug 1345696 part 8 - Make test_redundant_font_download.html more robust.
> 
> https://reviewboard.mozilla.org/r/124168/#review126694
> 
> I'm worried this is hiding a real bug. :(  Can you investigate to find out
> what's happening here?

What I think _should_ be happening is that setting a new className on the test element triggers a reflow, which in turn will initiate the font-load. So if we need to explicitly flush layout in the test file here, it sounds like maybe we're not reflowing as expected in response to the className change?

Comment 27

[Xidorn Quan [:xidorn] UTC+10]

Or we do not reflow before timeout. I'll investigate this tomorrow, and either try to fix it or mark fail for stylo&e10s with a new bug filed.

Comment 28

[Xidorn Quan [:xidorn] UTC+10]

So, right, changing className doesn't trigger restyle on e10s no matter how long the timeout is, which seems like a real bug. If I move the pointer into the page, the restyle would be triggered, and the test would pass.

IIRC long ago we had a bug about that setting className doesn't trigger restyle? How was that going? And why is this bug happen only on e10s...?

Comment 29

[Andrew McCreight [:mccr8]]

Comment on attachment 8851915 [details]
Bug 1345696 part 6 - Fix cycle collection for font-face rule.

https://reviewboard.mozilla.org/r/124162/#review126868

Bobby asked me to take a look. This sounds reasonable to me, assuming that you know that this other Servo object will definitely always exist and definitely hold a reference to this rule. Reporting too many edges to an object will make things get unlinked early, which can cause weird crashes. Bobby should also check the assumptions in your patch, so I left his r? there.

Comment 30

[Bobby Holley (:bholley)]

Comment on attachment 8851915 [details]
Bug 1345696 part 6 - Fix cycle collection for font-face rule.

Given that cameron reviewed the rest of this, I'll let him double-check the assumption in comment 29.

Comment 31

[Manish Goregaokar [:manishearth]]

Comment on attachment 8851910 [details]
Bug 1345696 part 3 - Add FFI for nsCSSFontFaceRule.

https://reviewboard.mozilla.org/r/124152/#review126880

Comment 32

[Manish Goregaokar [:manishearth]]

Comment on attachment 8851911 [details]
Rename font_face::FontFaceRule to FontFaceData.

https://reviewboard.mozilla.org/r/124154/#review126890

Comment 33

[Manish Goregaokar [:manishearth]]

Comment on attachment 8851912 [details]
Bug 1345696 part 4 - Add function for sugar of nsCSSValue.

https://reviewboard.mozilla.org/r/124156/#review126892

Comment 34

[Manish Goregaokar [:manishearth]]

Comment on attachment 8851913 [details]
Use Gecko nsCSSFontFaceRule for font face rule.

https://reviewboard.mozilla.org/r/124158/#review126894

::: servo/components/style/gecko/rules.rs:34
(Diff revision 1)
> +            })
> +        }}
> +    }
> +
> +    // font-style
> +    map_enum!(mStyle = (style: font_style) {

I think we can use the `gecko_conversion` mechanisms here.

::: servo/components/style/gecko/rules.rs:42
(Diff revision 1)
> +        oblique => NS_FONT_STYLE_OBLIQUE,
> +    });
> +
> +    // font-weight
> +    use computed_values::font_weight::T as FontWeight;
> +    descriptors.mWeight.set_integer(match data.weight {

Can this be consolidated as a method on FontWeight so that it can be used by gecko.mako.rs as well?

::: servo/components/style/gecko/rules.rs:55
(Diff revision 1)
> +        FontWeight::Weight800 => 800,
> +        FontWeight::Weight900 => 900,
> +    });
> +
> +    // font-stretch
> +    map_enum!(mStretch = (stretch: font_stretch) {

ditto re `gecko_conversion`

::: servo/components/style/gecko/rules.rs:82
(Diff revision 1)
> +    for src in data.sources.iter() {
> +        match *src {
> +            Source::Url(ref url) => {
> +                target_srcs.next().unwrap().set_url(&url.url);
> +                for hint in url.format_hints.iter() {
> +                    target_srcs.next().unwrap().set_font_format(&hint);

use `expect`s here?

Comment 35

[Manish Goregaokar [:manishearth]]

Comment on attachment 8851914 [details]
Bug 1345696 part 5 - Support CSSOM access to @font-face rules.

https://reviewboard.mozilla.org/r/124160/#review126898

Comment 36

[Manish Goregaokar [:manishearth]]

Comment on attachment 8851916 [details]
Record effective @font-face rules when updating stylist.

https://reviewboard.mozilla.org/r/124164/#review126902

Would like some more docs/comments on this one.

Comment 37

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851908 [details]
Bug 1345696 part 1 - Lots of fixup for the next patch.

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

Comment 38

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851909 [details]
Bug 1345696 part 2 - Move nsCSSFontFaceRule to a separate header.

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

Comment 39

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851910 [details]
Bug 1345696 part 3 - Add FFI for nsCSSFontFaceRule.

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

Comment 40

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851911 [details]
Rename font_face::FontFaceRule to FontFaceData.

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

Comment 41

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851912 [details]
Bug 1345696 part 4 - Add function for sugar of nsCSSValue.

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

Comment 42

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851913 [details]
Use Gecko nsCSSFontFaceRule for font face rule.

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

Comment 43

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851914 [details]
Bug 1345696 part 5 - Support CSSOM access to @font-face rules.

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

Comment 44

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851915 [details]
Bug 1345696 part 6 - Fix cycle collection for font-face rule.

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

Comment 45

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851916 [details]
Record effective @font-face rules when updating stylist.

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

Comment 46

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851917 [details]
Bug 1345696 part 7 - Provide @font-face rules for stylo backend.

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

Comment 47

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851919 [details]
Bug 1345696 part 8 - Update test expectations.

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

Comment 48

[Cameron McCormack (:heycam)]

Comment on attachment 8851915 [details]
Bug 1345696 part 6 - Fix cycle collection for font-face rule.

https://reviewboard.mozilla.org/r/124162/#review127114

OK, this isn't great, but I can't think of a better solution that doesn't involve hooking up CC through Rust style sheet and rule objects.  And I think the assumptions are OK.

From code inspection, I can't see devtools setting any expandos on the rule object that gets returned from inDOMUtils::GetUsedFontFaces.  It might be that devtools doesn't really need to hold on to these objects, and instead we could make the API return copies of the descriptor values that were on the @font-face rules.  Can you file a followup a CC/needinfo the appropriate devtools person to ask about that?

Comment 49

[Xidorn Quan [:xidorn] UTC+10]

Probably we can do this: when nsRange::GetUsedFontFaces is invoked, we force a CSSOM tree build-up in the current document to construct all @font-face rules, and set a flag on the document that we need aggressive CSSOM tree build-up for @font-face rules.

Whenever a new stylesheet or new group rule is inserted, we check this flag, and if it is set, we trigger the CSSOM tree build-up.

This should avoid leak from cycle reference on nsCSSFontFaceRules. And we can reuse this mechanism whenever a new API is added which needs some rule owned by servo code.

Alternatively, we can eventually migrate FontFace to be backed by Servo data, so that we can get rid of this hack.

But I guess it is not a priority anyway. It doesn't seem to me devtools is going to leak at the moment. I'll file a followup bug for tracking this, and we can think about this latter...

Comment 50

[Xidorn Quan [:xidorn] UTC+10]

Filed bug 1351662.

Comment 51

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851908 [details]
Bug 1345696 part 1 - Lots of fixup for the next patch.

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

Comment 52

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851909 [details]
Bug 1345696 part 2 - Move nsCSSFontFaceRule to a separate header.

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

Comment 53

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851910 [details]
Bug 1345696 part 3 - Add FFI for nsCSSFontFaceRule.

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

Comment 54

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851912 [details]
Bug 1345696 part 4 - Add function for sugar of nsCSSValue.

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

Comment 55

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851914 [details]
Bug 1345696 part 5 - Support CSSOM access to @font-face rules.

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

Comment 56

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851915 [details]
Bug 1345696 part 6 - Fix cycle collection for font-face rule.

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

Comment 57

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851917 [details]
Bug 1345696 part 7 - Provide @font-face rules for stylo backend.

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

Comment 58

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8851919 [details]
Bug 1345696 part 8 - Update test expectations.

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

Comment 59

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e84a3cc3233
part 1 - Lots of fixup for the next patch. r=heycam
https://hg.mozilla.org/integration/autoland/rev/badeba9aaf98
part 2 - Move nsCSSFontFaceRule to a separate header. r=heycam
https://hg.mozilla.org/integration/autoland/rev/0d1bd870cc10
part 3 - Add FFI for nsCSSFontFaceRule. r=manishearth
https://hg.mozilla.org/integration/autoland/rev/54491183b64b
part 4 - Add function for sugar of nsCSSValue. r=manishearth
https://hg.mozilla.org/integration/autoland/rev/59d93e337f9c
part 5 - Support CSSOM access to @font-face rules. r=manishearth
https://hg.mozilla.org/integration/autoland/rev/f24a273f1f7c
part 6 - Fix cycle collection for font-face rule. r=heycam,mccr8
https://hg.mozilla.org/integration/autoland/rev/1f4531b2ba5f
part 7 - Provide @font-face rules for stylo backend. r=heycam
https://hg.mozilla.org/integration/autoland/rev/3c163f0ba4b7
part 8 - Update test expectations. r=heycam

Comment 60

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f28a5dfdfc03
followup - Update more test expectation.

Comment 61

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2e84a3cc3233
https://hg.mozilla.org/mozilla-central/rev/badeba9aaf98
https://hg.mozilla.org/mozilla-central/rev/0d1bd870cc10
https://hg.mozilla.org/mozilla-central/rev/54491183b64b
https://hg.mozilla.org/mozilla-central/rev/59d93e337f9c
https://hg.mozilla.org/mozilla-central/rev/f24a273f1f7c
https://hg.mozilla.org/mozilla-central/rev/1f4531b2ba5f
https://hg.mozilla.org/mozilla-central/rev/3c163f0ba4b7
https://hg.mozilla.org/mozilla-central/rev/f28a5dfdfc03