Closed
Bug 828176
Opened 11 years ago
Closed 11 years ago
Make RasterImage::GetURIString work again
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: justin.lebar+bug, Assigned: seth)
References
Details
Attachments
(1 file, 2 obsolete files)
4.34 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•11 years ago
|
||
It currently always returns the empty string.
Assignee | ||
Comment 2•11 years ago
|
||
Proposed patch to add GetURIString() back, with a different signature. I'm not wild about this fix since we already have GetURI(), but for now this is the simplest solution due to the use of this function in sample labels. If we forced the use of GetURI() we'd have to have appropriate #ifdef's around the code that calls GetURI()->GetSpec(), since GetSpec() is an XPCOM function and can't be rolled into the same expression as the sample label. Long term the right solution is to add a non-XPCOM version of GetSpec() to nsIURI that returns an nsCString normally instead of through an out parameter. That would eliminate the need for GetURIString() totally, and would probably be useful in other places as well.
Attachment #700026 -
Flags: review?(joe)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → seth
Reporter | ||
Comment 3•11 years ago
|
||
Comment on attachment 700026 [details] [diff] [review] Make RasterImage::GetURIString work again. > + SAMPLE_LABEL_PRINTF("RasterImage", "SyncDecode", "%s", GetURIString().get());; Does this work? I expect that %s expects a char*, not a PRUnichar*. > + nsString GetURIString() { I don't know if this is kosher, but if it works, that's great.
Updated•11 years ago
|
Attachment #700026 -
Flags: review?(joe) → review+
Assignee | ||
Comment 4•11 years ago
|
||
I believe that returning an nsString by value is just fine. You're right that SAMPLE_LABEL_PRINTF wants a const char*, though. Sorry for the oversight. I'll move the conversion to UTF16 back out of GetURIString(), which will resolve the issue. Will post an updated patch shortly.
Assignee | ||
Comment 5•11 years ago
|
||
(The original code didn't handle this correctly either BTW, although the error was in a different place. Gotta love text encodings.)
Assignee | ||
Comment 6•11 years ago
|
||
Updated patch with fixes from review.
Assignee | ||
Updated•11 years ago
|
Attachment #700026 -
Attachment is obsolete: true
Assignee | ||
Comment 7•11 years ago
|
||
Try job here: https://tbpl.mozilla.org/?tree=Try&rev=05f44da9ec50
Assignee | ||
Comment 8•11 years ago
|
||
Looks like a null pointer dereference in that last push. I've added a check for a null GetURI() in GetURIString(). New try job here: https://tbpl.mozilla.org/?tree=Try&rev=6f2805c21fb4
Assignee | ||
Comment 9•11 years ago
|
||
OK, I think we're good to go here. Try looks fine. Requesting checkin.
Keywords: checkin-needed
Comment 10•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/25ea9ac37380
Keywords: checkin-needed
Comment 11•11 years ago
|
||
Backed out for crashes. https://hg.mozilla.org/integration/mozilla-inbound/rev/838146e5e46a https://tbpl.mozilla.org/php/getParsedLog.php?id=19298820&tree=Mozilla-Inbound TEST-UNEXPECTED-FAIL | /tests/browser/base/content/test/test_bug395533.html | Exited with code 1 during test run INFO | automation.py | Application ran for: 0:00:41.478997 INFO | automation.py | Reading PID log: /var/folders/Hs/HsDn6a9SG8idoIya6p9mtE+++TI/-Tmp-/tmp0ccQeBpidlog PROCESS-CRASH | /tests/browser/base/content/test/test_bug395533.html | application crashed [@ mozilla::image::RasterImage::SyncDecode()] Crash dump filename: /var/folders/Hs/HsDn6a9SG8idoIya6p9mtE+++TI/-Tmp-/tmp0Wv_NJ/minidumps/A22234CE-683E-42E0-A864-978612CDBDAE.dmp Operating system: Mac OS X 10.6.8 10K549 CPU: amd64 family 6 model 23 stepping 10 2 CPUs Crash reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS Crash address: 0x0 Thread 0 (crashed) 0 XUL!mozilla::image::RasterImage::SyncDecode() [RasterImage.h:25ea9ac37380 : 295 + 0xc] rbx = 0x000000014b677c60 r12 = 0x00007fff5fbfcbf8 r13 = 0x000000014b677c60 r14 = 0x0000000000000000 r15 = 0x00007fff5fbfcbd8 rip = 0x00000001012a99fe rsp = 0x00007fff5fbfcb40 rbp = 0x00007fff5fbfcc60 Found by: given as instruction pointer in context 1 XUL!mozilla::image::RasterImage::CopyFrame(unsigned int, unsigned int, gfxImageSurface**) [RasterImage.cpp:25ea9ac37380 : 994 + 0x7] rbx = 0x0000000000000000 r12 = 0x0000000000000001 r13 = 0x000000014b677c60 r14 = 0x0000000000000001 r15 = 0x0000000080070057 rip = 0x00000001012aa3cd rsp = 0x00007fff5fbfcc70 rbp = 0x00007fff5fbfcd90 Found by: call frame info 2 XUL!imgTools::GetFirstImageFrame(imgIContainer*, gfxImageSurface**) [imgTools.cpp:25ea9ac37380 : 265 + 0xe] rbx = 0x0000000000000010 r12 = 0x0000000000000010 r13 = 0x000000014b677c60 r14 = 0x00007fff5fbfceb0 r15 = 0x0000000000000010 rip = 0x00000001012d2da8 rsp = 0x00007fff5fbfcda0 rbp = 0x00007fff5fbfcdc0 Found by: call frame info 3 XUL!imgTools::EncodeScaledImage(imgIContainer*, nsACString_internal const&, int, int, nsAString_internal const&, nsIInputStream**) [imgTools.cpp:25ea9ac37380 : 134 + 0x4] rbx = 0x0000000000000010 r12 = 0x0000000000000010 r13 = 0x000000014b677c60 r14 = 0x000000014b63b030 r15 = 0x0000000000000010 rip = 0x00000001012d31cd rsp = 0x00007fff5fbfcdd0 rbp = 0x00007fff5fbfcee0 Found by: call frame info 4 XUL!nsFaviconService::OptimizeFaviconImage(unsigned char const*, unsigned int, nsACString_internal const&, nsACString_internal&, nsACString_internal&) [nsFaviconService.cpp:25ea9ac37380 : 1070 + 0x1c] rbx = 0x0000000000000000 r12 = 0x0000000000000010 r13 = 0x000000014b677c60 r14 = 0x000000014b63b030
Assignee | ||
Comment 12•11 years ago
|
||
Whoops. Foolishly forgot to upload the updated version of the patch (which has an all-green try run, as seen above).
Assignee | ||
Updated•11 years ago
|
Attachment #700805 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 13•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f42e4ce2f5c
Keywords: checkin-needed
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2f42e4ce2f5c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•