There's no point calling PromiseFlatString on a known flat string.
There's also little point calling it on a known substring, you might as well assign it directly into an nsAutoString.
Created attachment 535924 [details] [diff] [review]
Comment on attachment 535924 [details] [diff] [review]
Review of attachment 535924 [details] [diff] [review]:
@@ +550,5 @@
> LOGFONTW logFont;
> memset(&logFont, 0, sizeof(LOGFONTW));
> logFont.lfCharSet = DEFAULT_CHARSET;
> logFont.lfPitchAndFamily = 0;
> + PRUint32 l = PR_MIN(mName.Length() + 1, LF_FACESIZE);
If you can, can you split this out into a separate patch and get John Daggett to review it? It's not directly related to the nsPromiseFlatString work and I'm not confident in what its effects will be.
Pushed changeset 322d3d456f5b to mozilla-central with the change as requested.
Backed out because of red on Windows desktop mobile build:
Not worth my while checking things in any more, some other loser can do it.
(In reply to comment #5)
> Not worth my while checking things in any more, some other loser can do it.
If I understand correctly, "some other loser" will hit the exact same problem when they check this in. The red in comment 4 was due to an issue with this patch.
The failure was:
> gfx/thebes/gfxGDIFontList.cpp(438) : error C2039: 'get' : is not a member of 'nsAString_internal'
...which is a compile error on a line touched by this bug's patch. (the memcpy one, I believe)
Created attachment 538372 [details] [diff] [review]
Updated for bitrot
Conveniently the contentious hunk was the one that bitrotted anyway.
Fixed newly-introduced bitrot, added committer & commit message metadata (please make sure to include these when using checkin-needed), & pushed:
(In reply to comment #8)
> Fixed newly-introduced bitrot
odd -- I definitely qrefreshed after fixing that bitrot (I diffed the new patch against the patch posted here as a sanity-check), but somehow when I pushed, the pushed cset (in comment 8) ended up being empty.
I pushed again, and this one caught the code changes:
Verified the files
were updated in mozilla-central repository.
Is this enough to verify the fix?
It's possible that the compiler can optimise the old code away anyway so there wouldn't actually be any other difference that you can check.
(In other words: Yes, comment 11 is sufficient for marking as verified. --> Marking as such.)