Closed Bug 1194153 Opened 4 years ago Closed 4 years ago

Chrome Registry's convertChromeURL shouldn't crash when given a null url

Categories

(Toolkit :: Startup and Profile System, defect)

Product:
Toolkit
Component:
Startup and Profile System
Type:
defect
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla43
Tracking Flags:
Tracking Status
firefox43 --- fixed

People

(Reporter: florian, Assigned: florian)

Details

Attachments

(1 file, 1 obsolete file)

Patch v2
961 bytes, patch
benjamin
: review+
Details | Diff | Splinter Review
Attached patch Fix (obsolete) — — Splinter Review 
I just had a case while hacking the search service code where gChromeReg.convertChromeURL(uri); (where I later discovered uri was null) caused the browser to crash at startup.

This wasn't nice at all and I had to make a debug build to figure out what was going on. The debug build printed:
[2323] ###!!! ASSERTION: null url!: 'aChromeURI', file /Users/florian/buildhg/fx-team/chrome/nsChromeRegistry.cpp, line 267

I think this function should throw (ie. return NS_ERROR_something) when given invalid parameters, not crash.
Attachment #8647427 - Flags: review?(benjamin)
Comment on attachment 8647427 [details] [diff] [review]
Fix

I think crashing is reasonable, but in any case NS_ENSURE_* is forbidden in new code. This should probably be:

if (NS_WARN_IF(!aChromeURI)) {
  return NS_ERROR_INVALID_ARG;
}
Attachment #8647427 - Flags: review?(benjamin) → review-
Attached patch Patch v2 — — Splinter Review
Attachment #8647427 - Attachment is obsolete: true
Attachment #8656478 - Flags: review?(benjamin)
Attachment #8656478 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/6bde454912d5
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.