Closed Bug 1209349 Opened 10 years ago Closed 9 years ago

Audit the callers of the two-argument OriginAttributes constructor

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: bholley, Assigned: allstars.chh)

References

Details

Attachments

(1 file, 1 obsolete file)

Looking through the code, I see a lot of callers that follow the letter of OriginAttributes but not the spirit: https://dxr.mozilla.org/mozilla-central/rev/6256ec9113c115141aab089c45ee69438884b680/dom/quota/QuotaManager.cpp#5291 Stuff like that isn't going to work when we add signedPkg etc. We should audit and fix these consumers, and possibly remove that constructor.
Assignee: nobody → allstars.chh
Comment on attachment 8699420 [details] [diff] [review] Patch. Review of attachment 8699420 [details] [diff] [review]: ----------------------------------------------------------------- Hi, Janv Could you help to review QuotaManager part for me? I only updated OriginProps and some code in OriginParser. Thanks
Attachment #8699420 - Flags: review?(jvarga)
Comment on attachment 8699420 [details] [diff] [review] Patch. Review of attachment 8699420 [details] [diff] [review]: ----------------------------------------------------------------- r=me if you address my comments ::: dom/quota/ActorsParent.cpp @@ +5988,5 @@ > nsCString spec; > + PrincipalOriginAttributes attrs; > + bool result = OriginParser::ParseOrigin(NS_ConvertUTF16toUTF8(leafName), > + spec, &attrs); > + NS_ENSURE_TRUE(result, NS_ERROR_FAILURE); please don't use NS_ENSURE_TRUE(), see my comment below @@ +6170,5 @@ > } > > + nsCOMPtr<nsIPrincipal> principal = > + BasePrincipal::CreateCodebasePrincipal(uri, originProps.mAttrs); > + NS_ENSURE_TRUE(principal, NS_ERROR_FAILURE); please no more NS_ENSURE_TRUE(), use this instead: if (NS_WARN_IF(!principal)) { return NS_ERROR_FAILURE; }
Attachment #8699420 - Flags: review?(jvarga) → review+
Attached patch Patch. v2Splinter Review
addressed janv's comment.
Attachment #8699420 - Attachment is obsolete: true
Attachment #8699808 - Flags: review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: