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)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: florian, Assigned: florian)

Details

Attachments

(1 file, 1 obsolete file)

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 v2Splinter 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
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.