Get rid of OriginAttributes::Inherit

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: baku, Assigned: baku)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
We had this method because the addonId value was not supposed to be copied when OriginAttributes was constructed from another one. Finally addonId is gone, and we can remove OriginAttributes::Inherits
(Assignee)

Comment 1

2 years ago
Created attachment 8844540 [details] [diff] [review]
inherit.patch
Assignee: nobody → amarchesini
Attachment #8844540 - Flags: review?(tom)

Comment 2

2 years ago
Comment on attachment 8844540 [details] [diff] [review]
inherit.patch

Review of attachment 8844540 [details] [diff] [review]:
-----------------------------------------------------------------

I had two nits, but looks good to me.

::: netwerk/base/LoadInfo.cpp
@@ +141,5 @@
>        }
>      }
>    }
>  
> +  // If CSP requires SRI (require-sri-for), then store that information

I think the unindent would have been better separated, but I manually diffed it.

@@ +258,2 @@
>  
> +#ifdef DEBUG

I thought MOZ_ASSERT was only compiled into DEBUG builds, and the if() would fall away as empty, so I think the ifdef is unneeded?

::: netwerk/base/Predictor.cpp
@@ +2282,5 @@
>  
>        if (loadContext) {
> +        OriginAttributes attrs;
> +        loadContext->GetOriginAttributes(attrs);
> +        originAttributes = attrs;

Is there a reason this couldn't just be loadContext->GetOriginAttributes(originAttributes) ?
Attachment #8844540 - Flags: review?(tom) → review+
(Assignee)

Comment 3

2 years ago
> I think the unindent would have been better separated, but I manually diffed
> it.

Right. Done.

> I thought MOZ_ASSERT was only compiled into DEBUG builds, and the if() would
> fall away as empty, so I think the ifdef is unneeded?

This is true, but I don't like to have an empty if().

Comment 4

2 years ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3de78a28ad06
Fix the indentation in LoadInfo.cpp, r=tjr
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffed210e1e21
Get rid of OriginAttributes::Inherit, r=tjr

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3de78a28ad06
https://hg.mozilla.org/mozilla-central/rev/ffed210e1e21
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.