nsUserFontSet::UpdateRules aborts woff load only to start the same woff load again

RESOLVED FIXED in mozilla26

Status

()

Core
Layout: Text
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mcmanus, Assigned: jfkthame)

Tracking

20 Branch
mozilla26
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-seep], URL)

Attachments

(7 attachments, 3 obsolete attachments)

2.76 KB, patch
David Baron
: review+
Details | Diff | Splinter Review
8.03 KB, patch
David Baron
: review+
Details | Diff | Splinter Review
10.86 KB, patch
jfkthame
: review+
Details | Diff | Splinter Review
1.18 KB, patch
jtd
: review+
Details | Diff | Splinter Review
1.31 KB, patch
jtd
: review+
Details | Diff | Splinter Review
5.25 KB, patch
jtd
: review+
Details | Diff | Splinter Review
7.50 KB, patch
jtd
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
ilya @google sent this along, and I reduced it to a fairly small test case at http://www.ducksong.com/font/index.html

afaict, the html head links a stylesheet that does a font-face attached to an external woff.

later on after /html (and inside a noscript element if that matters) the same link stylesheet is referenced again.

This results in nsUSerFontSet canceling the in progress necko loads for the woff file, and then resubmitting the same request.

the result is delayed use of the font (it didn't fully download the first time) and wasted bandwidth at a time when bandwidth was being shared without other important things. (e.g. images and such that I've removed from the reduced test case).

There seems to be some logic in UpdateRules() for not throwing away old rules if they are complete, but not if they are still loading.

If this is in the wrong component - I'm sorry!
(Assignee)

Comment 1

5 years ago
So there are two issues here, I think.

(1) First, there's nsUserFontSet::UpdateRules(), which is responsible for mutating the user font set when the style rules are modified in some way. As you noted, this function tries to preserve existing font entries for which the font resource has been downloaded, to avoid throwing them away and re-downloading.

Currently, it doesn't do this for in-progress downloads; it starts out by cancelling any current loaders. I think we can get away without doing that: instead of pre-emptively cancelling them at the start of UpdateRules(), we can only cancel loaders for any font entries that end up "orphaned" after the font set has been updated. (To make this work, we'll need to take ensure that we also preserve the existing font-family objects, rather than discarding them and creating new ones, so that when the loader eventually completes, it can still find the family in which to replace the proxy with the loaded font.)

I've got a patch that seems to work for this, and it should improve behavior in the case where the style rules are modified, causing us to update the font set, yet the existing rules still apply and so downloads that were already initiated are still what we want.

(2) *However*, there's a second issue at play in your example. The initial font set you get as a result of linking the google-fonts stylesheet has a family "Open Sans" with two font face entries (regular and bold). The document wants to use the regular face from this family, so we kick off that font load.

Then you add a second link to the google-fonts stylesheet, thus introducing a second pair of @font-face rules that define two faces for the same font-family "Open Sans". But these new font-face definitions do not *overwrite* the existing ones; they are *added* to the family, so it now has four faces. And when we reflow and do font matching for the <span> in the document, we find the *second* of the "regular" faces in the Open Sans family. (This is correct behavior per spec[*]. Note that it's valid for a user font family to contain multiple entries with the same weight/width/style; they might have different unicode-range descriptors, or they might have different src descriptors that end up covering different character repertoires, so both need to be considered.)

So now your <span> calls for the *second* of the "regular" faces in the Open Sans family, which is not the same font entry as before, and so we ask this new entry to download its first available source. It happens to be the exact same resource as the already-in-progress one, but AFAIK we don't have any code that can detect this and somehow "share" the download.

Which all means that in this particular example, fixing part (1) above will, if anything, actually make things slightly *worse*, because the first download - which is no longer going to be used, because of the new font entry that will "shadow" it during font matching - no longer gets cancelled by UpdateRules(), but instead runs to completion, alongside the new download that we're really going to use. In the end, we have a font family with two (of its four) faces having completed downloads, but only one of them is actually used.

Note that if the first download had *completed* before the new rules were added, we would successfully share the downloaded font, thanks to the "user font cache" from bug 816483 that allows user fonts to be shared when the same resource is used, even by a new font set. But this only applies to completed downloads, not those still in progress.

I can think of a couple of ways we could try to mitigate this:

(a) Allow the user font cache to handle proxy entries (potentially with in-progress downloads) as well as "real" font entries for which the resource has been loaded. This would be a bit messy, however, as nsFontFaceLoader relies on holding references to its font family and font set, so that it can notify them when the load completes. Making proxy entries - and thus loaders - sharable via the user font cache would complicate this.

(b) Alternatively, when we append new font entries to the family (during UpdateRules()), we could check to see if there is an existing entry that matches exactly, and if so, just move the existing entry (which might already be loading a font resource) to the end, instead of adding a new entry that is guaranteed to make the existing one inaccessible. This should be relatively simple, and will avoid the duplicate downloads for the common real-world case where the same rules are loaded twice, although there will still be cases where the same *resource* may be unnecessarily loaded by rules that have some difference in their other descriptors.

I expect (b) to be much simpler and to cover the vast majority of use cases, so that's what I suggest we try.


[*] http://dev.w3.org/csswg/css-fonts/#composite-fonts
Component: Layout → Layout: Text
(Assignee)

Comment 2

5 years ago
Created attachment 759095 [details] [diff] [review]
part 1 - preserve in-progress font loaders when updating the user font set.

This addresses issue (1) above; it should improve behavior when there are in-progress downloads, and (unrelated) style rule changes cause us to update the font set. The only negative, AFAICT, is when a new face is added to the family that "shadows" the existing face for which we're downloading a resource, so that the old face will not be used, but that download will no longer be cancelled. I propose to address this as described in (2.b) above.
Attachment #759095 - Flags: review?(dbaron)
(Assignee)

Updated

5 years ago
Assignee: nobody → jfkthame
(Assignee)

Comment 3

5 years ago
Created attachment 759151 [details] [diff] [review]
part 2 - avoid adding duplicate face entries to a user font family.

And here's part 2, which avoids duplicate downloads in scenarios such as the testcase here, where the same @font-face rules are (perhaps inadvertently) loaded multiple times. Seems to work as expected locally; tryserver run in progress at https://tbpl.mozilla.org/?tree=Try&rev=eb3fa362fe30.
Attachment #759151 - Flags: review?(dbaron)

Comment 4

5 years ago
I'll try to look into this further soon (though I should really have redirected the review request to jdaggett the week before last).  But I know I'm going to have one comment, which is that this should have a test.  I think this is the sort of thing that is relatively straightforward to test in the mochitest harness (with sjs, which doesn't need to return a font that actually *works*), and the sort of thing that's particularly easy to break in refactorings and thus *ought* to have a test.

(And when you write the test, you should of course make sure that it fails without the patch and passes with it.)
Flags: needinfo?(jfkthame)
(Assignee)

Comment 5

5 years ago
It looks like we can actually test this alongside the rest of the @font-face testcases in the reftest suite. The idea is to point our @font-face rules at an .sjs that keeps track of what requests it has received, modify the document styles in such a way as to encounter this bug, and then use an XMLHttpRequest to the same .sjs to retrieve its "log" of requests and insert it into the testcase. Then we can simply compare it with a reference that shows the desired result.
Flags: needinfo?(jfkthame)
(Assignee)

Comment 6

5 years ago
Created attachment 765954 [details] [diff] [review]
reftest for redundant downloading of a repeated @font-face resource

This test fails with current trunk code (https://tbpl.mozilla.org/?tree=Try&rev=b560794057ba), and passes once the patches here are applied (https://tbpl.mozilla.org/?tree=Try&rev=4d4fc5c52f3f).
Attachment #765954 - Flags: review?(dbaron)

Comment 7

5 years ago
Comment on attachment 759095 [details] [diff] [review]
part 1 - preserve in-progress font loaders when updating the user font set.

So is there something here that still stops the font loads if they are *not* needed any more?  If so, could you explain in a code comment?  If not... I'd be a little concerned, since it seems like it may well be important to stop loads that aren't relevant anymore.

anyway, r=dbaron
Attachment #759095 - Flags: review?(dbaron) → review+

Comment 8

5 years ago
Comment on attachment 759151 [details] [diff] [review]
part 2 - avoid adding duplicate face entries to a user font family.

Changing stretch from uint32_t to int32_t is good, but should have been
in a separate patch in the stack.  (Not worth it now, though.)

>+    bool italic =
>+        (aItalicStyle & (NS_FONT_STYLE_ITALIC | NS_FONT_STYLE_OBLIQUE)) != 0;

Does it make more sense now to pass this, rather than aItalicStyle,
to the constructor of gfxProxyFontEntry?  (Could be in a patch before
this patch.)  It seems odd to have this conversion happen in two places,
and such a change could bring it back to being in one place.

I also think using a single |proxyEntry| variable here is probably
bad form (and could lead to confusion if more things get added to
the function).  There was no good reason to have the declaration up
at the top in the old code rather than at first use, either.  Maybe
move the |proxyEntry| declaration down to first (old) use, and name
the new variable |existingProxyEntry|?

>+        proxyEntry = static_cast<gfxProxyFontEntry*>(fontList[i].get());
>+        if (proxyEntry->mWeight != aWeight ||
>+            proxyEntry->mStretch != aStretch ||
>+            proxyEntry->mItalic != italic ||
>+            proxyEntry->mFeatureSettings != aFeatureSettings ||
>+            proxyEntry->mLanguageOverride != languageOverride ||
>+            proxyEntry->mSrcList != aFontFaceSrcList
>+            // XXX once we support unicode-range, we'll need to compare that
>+            // here as well
>+            ) {
>+            continue;
>+        }

Perhaps this comparison should be a member of gfxProxyFontEntry to
make it more likely that people modifying the class will see that they'd
need to modify it?  It seems a bit fragile to have so many ways of 
comparing these sorts of pieces of data.
Attachment #759151 - Flags: review?(dbaron) → review+

Comment 9

5 years ago
Comment on attachment 765954 [details] [diff] [review]
reftest for redundant downloading of a repeated @font-face resource

I think these timeouts are going to be too fragile.

I think you should write this as a mochitest rather than as a reftest, and instead of using a 50ms timeout, poll regularly for the state being what you expect at that point (both in the middle (before continuing) and at the end).  Then give up on the polling after 10s or so so that the failure condition for the test happens before the overall suite timeout.
Attachment #765954 - Flags: review?(dbaron) → review-
(Assignee)

Comment 10

5 years ago
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #7)
> Comment on attachment 759095 [details] [diff] [review]
> part 1 - preserve in-progress font loaders when updating the user font set.
> 
> So is there something here that still stops the font loads if they are *not*
> needed any more?  If so, could you explain in a code comment?  If not... I'd
> be a little concerned, since it seems like it may well be important to stop
> loads that aren't relevant anymore.

If the user font set is torn down entirely rather than updated (e.g. the presShell goes away), then its loaders are cancelled (by nsUserFontSet::Destroy()). And for any font entries that are -not- migrated to the new font set (because the corresponding @font-face rules are no longer present), any load-in-progress will be cancelled when we iterate over the oldRules list at the end of nsUserFontSet::UpdateRules. I'll add some comments to try and make this clearer.

There are certainly scenarios where we could start a font load, but before it completes, something about the document (either content or style) changes in such a way that it is no longer relevant. We won't necessarily abandon the in-progress load in this case - if the @font-face rule concerned is still present, we'll continue loading its resource once we've started.

(FWIW, there are already such scenarios without the changes here, because it's possible to make a user font become no-longer-relevant after starting to download it -without- needing to mutate the actual user font set.)

Comment 11

5 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #10)
> There are certainly scenarios where we could start a font load, but before
> it completes, something about the document (either content or style) changes
> in such a way that it is no longer relevant. We won't necessarily abandon
> the in-progress load in this case - if the @font-face rule concerned is
> still present, we'll continue loading its resource once we've started.
> 
> (FWIW, there are already such scenarios without the changes here, because
> it's possible to make a user font become no-longer-relevant after starting
> to download it -without- needing to mutate the actual user font set.)

Ah, true, I guess not mutating the user font set is probably the more common case there -- removing elements is probably more common than messing with style sheets.  So never mind.
(Assignee)

Comment 12

5 years ago
Created attachment 776460 [details] [diff] [review]
test for redundant downloading of a repeated @font-face resource

OK, here's a mochitest version of the test that ought to be more reliable, I think. Seems to work fine for me locally; once tryserver is reset, I'll push a couple of try jobs to confirm that it consistently fails on current trunk and passes with the patches here across various platforms.
Attachment #776460 - Flags: review?(dbaron)
(Assignee)

Updated

5 years ago
Attachment #765954 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Created attachment 776681 [details] [diff] [review]
test for redundant downloading of a repeated @font-face resource

Giving the helper .sjs file a "test_*.sjs" name was a bad idea (duh!). Fixed that.
Attachment #776681 - Flags: review?(dbaron)
(Assignee)

Updated

5 years ago
Attachment #776460 - Attachment is obsolete: true
Attachment #776460 - Flags: review?(dbaron)
(Assignee)

Comment 14

5 years ago
Tryserver run in progress: https://tbpl.mozilla.org/?tree=Try&rev=b43659eeb009

Comment 15

5 years ago
Comment on attachment 776681 [details] [diff] [review]
test for redundant downloading of a repeated @font-face resource

I think it would also be good to test that the request log is the *length* you expect it to be at each step.  That should cover checking everything in it.

Seems straightforward to have something like getLogLength() on top of getRequestLog().split("\n"), that you could check at the start of each of the pieces in addition to checking getLastRequest().

r=dbaron with that or something like it
Attachment #776681 - Flags: review?(dbaron) → review+
(Assignee)

Comment 16

5 years ago
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #8)
> Comment on attachment 759151 [details] [diff] [review]
> >+    bool italic =
> >+        (aItalicStyle & (NS_FONT_STYLE_ITALIC | NS_FONT_STYLE_OBLIQUE)) != 0;
> 
> Does it make more sense now to pass this, rather than aItalicStyle,
> to the constructor of gfxProxyFontEntry?

Perhaps so, but I decided against making this change for the time being. The real "problem" here is that we don't distinguish oblique from italic in the font entry. We need to change that field from a single bool to an enum or something, and then we -will- need to pass the proper aItalicStyle setting (not distilled into a single boolean). For now, it seems cleaner not to "leak" this internal limitation of our font entry class out through the gfxProxyFontEntry API.

Bug 543715 should clean this up properly.

> Perhaps this comparison should be a member of gfxProxyFontEntry to
> make it more likely that people modifying the class will see that they'd
> need to modify it?  It seems a bit fragile to have so many ways of 
> comparing these sorts of pieces of data.

Sounds good. I've moved it to a gfxProxyFontEntry::Matches(...) method that takes the same parameters as the gfxProxyFontEntry constructor. That should help us remember to coordinate any changes needed when gfxProxyFontEntry is enhanced in the future.
(Assignee)

Comment 17

5 years ago
Created attachment 777056 [details] [diff] [review]
part 2 - avoid adding duplicate face entries to a user font family.

Updated part 2 as per comments above; carrying forward r=dbaron.
(Assignee)

Updated

5 years ago
Attachment #759151 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #777056 - Flags: review+
(Assignee)

Comment 20

5 years ago
Re-landed, with an added waitForRequest() to make sure the server has actually received the 'init' request before we proceed with the rest of the test. Also bumped the timeouts for the subsequent requests a bit, in case of really slow test machines.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4a3befee43f1
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/09c9359bdd43
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/32ffcd6db605

Updated

5 years ago
Depends on: 896592
(Assignee)

Comment 22

5 years ago
Given that this seems to have triggered unexpected crashiness (bug 896200), as well as an odd behavior regression (bug 896592), I'm intending to back it out for now (once I can catch inbound in an open state).
Depends on: 896200

Updated

5 years ago
Depends on: 896225
(Assignee)

Comment 23

5 years ago
Backed out (all 3 changesets) pending further investigation:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9fc53053c489
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Jonathan Kew (:jfkthame) from comment #23)
> Backed out (all 3 changesets) pending further investigation:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/9fc53053c489

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/9fc53053c489
(Assignee)

Comment 25

5 years ago
Well, with both a topcrash and other behavior regressions (fonts failing to show up on several sites), that didn't go too well. :(

However, I think I have figured out each of the regressions, and have follow-up patches to fix them. I'll post the fixes for review as three separate patches, but for relanding we should fold them all together with the original patches here.

First, the crash (bug 896200). This occurred because font loaders were not being correctly removed from the font set's mLoaders hashtable when they are cancelled. This was a pre-existing bug in the code at the end of nsUserFontSet::UpdateRules, but was masked by the fact that we were eagerly destroying -all- loaders at the beginning of the function, so the faulty code was not actually being used.

Second, it's possible that more than one font-face rule will refer to the same gfxProxyFontEntry. I assume this happens, for example, if there are several stylesheets that all contain the same @font-face definition; these will become distinct rules, but the proxy-matching added in the Part 2 patch here will cause them to share a single proxy if all the descriptors match. This means that when the download completes, we need to ensure that we update -all- those rules (not just the first one found) to refer to the real font. Fixing this resolves the missing-text regression on http://nation.foxnews.com/ (bug 897181), and -some- but not all of the missing text on http://www.mtv.de/ (bug 896592).

And finally, the Part 2 patch aimed to avoid multiple copies of the same proxy ending up in the gfxMixedFontFamily's list of faces, but it only handled this when initially setting up the font set; it failed to check for duplicates when migrating old rules to the updated rule set. To fix this, I'm consolidating the duplicate-checking into AddFontEntry, rather than relying on its callers to check ahead of time. This will also allow us to generally clean up gfxMixedFontFamily, which no longer needs a RemoveFontEntry method at all. This fixes the other cases of missing text on http://www.mtv.de/, as well as the similar case on http://rpg.stackexchange.com/ (bug 896592).

I'll have a go at constructing unit tests that hit each of these cases; having understood how they're failing, I think it should be fairly easy to reproduce similar scenarios.
(Assignee)

Comment 26

5 years ago
Created attachment 781633 [details] [diff] [review]
[crash fix] nsFontFaceLoader::Cancel clears the proxy's mLoader pointer, so we cannot use it afterward to remove the loader from the font set's hashtable
Attachment #781633 - Flags: review?(dbaron)
(Assignee)

Comment 27

5 years ago
Created attachment 781634 [details] [diff] [review]
[regression fix 1] - make sure to update all rules that refer to a given proxy when it is replaced by a real font
Attachment #781634 - Flags: review?(dbaron)
(Assignee)

Comment 28

5 years ago
Created attachment 781635 [details] [diff] [review]
[regression fix 2] - consolidate duplicate-face-prevention into gfxMixedFontFamily::AddFontEntry, so it applies regardless of how the entry is added; also clean up redundant methods in the class
Attachment #781635 - Flags: review?(dbaron)
(Assignee)

Comment 29

5 years ago
Created attachment 781741 [details] [diff] [review]
reftests for regressions caused when multiple rules refer to the same proxy

These testcases hit both kinds of "missing-text" regression that occurred with the original patches here, and are fixed by the regression-fix followups.
Attachment #781741 - Flags: review?(dbaron)
(Assignee)

Comment 30

5 years ago
Tryserver run, relanding the original patches plus the followup fixes here:
https://tbpl.mozilla.org/?tree=Try&rev=38d2a0821591

Comment 31

5 years ago
John, would you mind doing these reviews instead of me?
Flags: needinfo?(jdaggett)

Updated

5 years ago
Attachment #781633 - Flags: review?(dbaron) → review?(jdaggett)

Updated

5 years ago
Attachment #781634 - Flags: review?(dbaron) → review?(jdaggett)

Updated

5 years ago
Attachment #781635 - Flags: review?(dbaron) → review?(jdaggett)

Updated

5 years ago
Attachment #781741 - Flags: review?(dbaron) → review?(jdaggett)

Comment 32

5 years ago
Yes, I can do these reviews.  Jonathan, given all the patches bouncing in and out, what's the state of the testcase?  With trunk code will the testcase still fail?  Or is there another testcase or set of testcases that replicates the crash behavior?
Flags: needinfo?(jdaggett)
(Assignee)

Comment 33

5 years ago
Everything that landed here is currently backed-out, so we're back in the original trunk state; the testcase (attachment 776681 [details] [diff] [review]) for the original issue will fail on trunk.

The two previously-reviewed patches fix that, so that the testcase will pass (and doesn't crash, AFAIK; we didn't see any problems on TBPL while this bug was landed).

The crashiness was reported in bug 896200, and seems to be fairly readily reproducible with the STR there, especially on Windows, though we don't have a small standalone testcase for it. This was backed out in order to fix that crash on Nightly, pending a real fix. Attachment 781633 [details] [diff] is the proper fix for the crash (a latent bug in older code that was exposed by the changes here).

And the rendering regressions (blank text) reported in bug 897181 and bug 896592 are addressed by attachment 781634 [details] [diff] [review] and attachment 781635 [details] [diff] [review], which fix errors in the original patches here. The additional reftests in attachment 781741 [details] [diff] [review] focus on those regressions; so they pass on current trunk, fail with the original patches here, and pass again once the regression-fix patches are added.

(If we hadn't backed this out for the topcrash, I'd have simply attached the regression-fix patches to those separate bugs, but as it -was- all backed out, we should simply re-land it here including the corrections.)

Hope that makes it clear what each piece is. The "fixes" apply on top of the original patches, and the additional unit tests target the behavior regressions we saw after the original landing.

Updated

5 years ago
Attachment #781633 - Flags: review?(jdaggett) → review+

Updated

5 years ago
Attachment #781634 - Flags: review?(jdaggett) → review+

Updated

5 years ago
Attachment #781741 - Flags: review?(jdaggett) → review+

Comment 34

5 years ago
Comment on attachment 781635 [details] [diff] [review]
[regression fix 2] - consolidate duplicate-face-prevention into gfxMixedFontFamily::AddFontEntry, so it applies regardless of how the entry is added; also clean up redundant methods in the class

#          for (i = 0; i < numFonts; i++) {
#              gfxFontEntry *fe = mAvailableFonts[i];
# -            if (fe == aOldFontEntry) {
# -                // note that this may delete aOldFontEntry, if there's no
# -                // other reference to it except from its family
# -                aNewFontEntry->mFamilyName = Name();
# -                mAvailableFonts[i] = aNewFontEntry;
# -                break;
# -            }
# -        }
# -        NS_ASSERTION(i < numFonts, "font entry not found in family!");
# -        ResetCharacterMap();

Your patch removes the assertion, but it seems like not finding a font
entry is still a bad situation, no?

Overall, I'm not really ecstatic about the way so much of the updating
logic has been pushed down into gfxMixedFontFamily.  This makes the
code very hard to follow, I think it would actually be better to keep
these methods simple and move any complex search/replace type logic in
to UpdateRules, since it's only when old rules are being replaced that
complicated logic comes into play.

But for now, I guess I'm okay with this.  If more regressions occur, I
think we should try and think of a way to achieve the needed
efficiency when updating without maintaining so much complicated logic
spread across multiple classes.  It makes it hard to get a precise
understanding of the invariants at any one point within the code and
this is the reason crashes occur in odd situations.
Attachment #781635 - Flags: review?(jdaggett) → review+
(Assignee)

Comment 35

5 years ago
Ah, yes - I restored the assertion, thanks. (I think I removed that during an earlier stage before enforcing uniqueness of the entries in the family; and so Replace had to search all the way to the end and potentially update multiple entries.)

Folded the followup fixes into the original patches, so as not to leave broken intermediate stages in the tree here, and (re-)landed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba45b2a024f9 (patches)
https://hg.mozilla.org/integration/mozilla-inbound/rev/6009fb97314a (tests)
Target Milestone: mozilla25 → mozilla26
https://hg.mozilla.org/mozilla-central/rev/ba45b2a024f9
https://hg.mozilla.org/mozilla-central/rev/6009fb97314a
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 37

5 years ago
Oops... apparently I fail at mercurial. I intended to land all the tests in the second changeset there (6009fb97314a), but omitted to actually fold the regression reftests into it.

So here are those additional tests, for real this time:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7cca8cd58d5b
You need to log in before you can comment on or make changes to this bug.