Closed Bug 660525 Opened 9 years ago Closed 9 years ago
Useless uses of Promise
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.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #535924 - Flags: review?(joe)
Comment on attachment 535924 [details] [diff] [review] Proposed patch Review of attachment 535924 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxGDIFontList.cpp @@ +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.
Attachment #535924 - Flags: review?(joe) → review+
Pushed changeset 322d3d456f5b to mozilla-central with the change as requested.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Backed out because of red on Windows desktop mobile build: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1307650903.1307652099.19955.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1307650903.1307652792.22182.gz > 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)
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: http://hg.mozilla.org/integration/mozilla-inbound/rev/b65e22bc0c28
(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: http://hg.mozilla.org/integration/mozilla-inbound/rev/078113c73963
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Verified the files gfx/layers/opengl/LayerManagerOGLProgram.h gfx/thebes/GLContext.cpp gfx/thebes/gfxFontUtils.cpp gfx/thebes/gfxGDIFontList.cpp were updated in mozilla-central repository. Is this enough to verify the fix? Thank you
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.)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.