This test (and it appears some others around it) is annotated for the following assertion: 02:15:42 INFO - [Parent 2094] ###!!! ASSERTION: Empty aExtension parameter!: '!aExtension.IsEmpty()', file ../../../uriloader/exthandler/nsExternalHelperAppService.cpp, line 2768 For whatever reason, we didn't hit it on this run. Dave, can you help find an owner for this? Maybe just fixing the assertion in these tests outright? https://tbpl.mozilla.org/php/getParsedLog.php?id=23149507&tree=Birch&full=1#error0 Ubuntu VM 12.04 x64 birch debug test mochitest-other on 2013-05-20 01:58:12 PDT for push a9547c16624f slave: tst-linux64-ec2-449
Dave, any ideas on this?
I have no idea what that assertion is or why the download manager might trigger it. Paolo?
Component: General → Download Manager
Flags: needinfo?(dtownsend+bugmail) → needinfo?(paolo.mozmail)
A prerequisite for triggering this particular assertion is to call nsIMIMEService::GetTypeFromExtension with an empty extension parameter. Based on how the operating system is configured, this assertion may or may not happen. This is the code that makes the call with the empty argument in this case: http://mxr.mozilla.org/mozilla-central/source/image/decoders/icon/gtk/nsIconChannel.cpp#460 There are a few other places where GetTypeFromExtension may be called with an empty parameter. I'd be happy to review a patch that simply makes the function return no match early, if the argument is empty.
So add something like: if (aFileExt.isEmpty()) return NS_ERROR_NOT_AVAILABLE; in the beginning of nsExternalHelperAppService::GetTypeFromExtension in nsExternalHelperAppService.cpp?
Sounds good, as long as all regression tests pass (fixing the assertion count).
https://tbpl.mozilla.org/?tree=Try&rev=8a12a6136544 That's a lotta green :)
Assignee: nobody → ryanvm
Status: NEW → ASSIGNED
Attachment #770275 - Flags: review?(paolo.mozmail)
Comment on attachment 770275 [details] [diff] [review] add early return Looks good!
Attachment #770275 - Flags: review?(paolo.mozmail) → review+
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment on attachment 770275 [details] [diff] [review] add early return [Approval Request Comment] Bug caused by (feature/regressing bug #): old bug User impact if declined: sheriff pain for the next year on esr24 Testing completed (on m-c, etc.): on m-c for over a month now Risk to taking this patch (and alternatives if risky): very minimal, just adds an early return String or IDL/UUID changes made by this patch: none
Attachment #770275 - Flags: approval-mozilla-beta?
Attachment #770275 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.