Closed Bug 1340677 Opened 3 years ago Closed 3 years ago

Stop supporting ORIGIN_IS_FULL_SPEC for Firefox

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

(Whiteboard: [necko-active])

Attachments

(1 file, 1 obsolete file)

The check that we do for this flag in NS_SecurityCompareURIs sometimes shows up in profiles.  This flag was added only for Thunderbird and the Mozilla suite and was never used in mozilla-central, so we are just paying its performance cost.  We should remove this from Firefox and only use it for those other products.
Component: Security: CAPS → Networking
This is only used in Thunderbird and SeaMonkey and makes origin
comparisons needlessly slow in Firefox.
Attachment #8838735 - Flags: review?(mcmanus)
Whiteboard: [necko-active]
Comment on attachment 8838735 [details] [diff] [review]
Stop supporting ORIGIN_IS_FULL_SPEC for Firefox

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

awesome. thanks!

::: netwerk/base/nsIProtocolHandler.idl
@@ +304,2 @@
>       */
>      const unsigned long ORIGIN_IS_FULL_SPEC = (1 << 20);

I think you should ifdef this one too - so any addon users or whatever will get an exception and notice
Attachment #8838735 - Flags: review?(mcmanus) → review+
(In reply to :Ehsan Akhgari from comment #0)
> The check that we do for this flag in NS_SecurityCompareURIs sometimes shows
> up in profiles.  This flag was added only for Thunderbird and the Mozilla
> suite and was never used in mozilla-central, so we are just paying its
> performance cost.  We should remove this from Firefox and only use it for
> those other products.

Ehsan, if you are removing this check on non-comm-cental then we MUST MOZ_CRASH when we find a protocol that defines that flag on non-comm-cental.  Don't you think?
Flags: needinfo?(ehsan)
(In reply to Patrick McManus [:mcmanus] from comment #2)
> ::: netwerk/base/nsIProtocolHandler.idl
> @@ +304,2 @@
> >       */
> >      const unsigned long ORIGIN_IS_FULL_SPEC = (1 << 20);
> 
> I think you should ifdef this one too - so any addon users or whatever will
> get an exception and notice

Unfortunately it's not possible to preprocess XPIDL files...  That being said I did check and no extension are using this thankfully.

(In reply to Honza Bambas (:mayhemer) from comment #3)
> Ehsan, if you are removing this check on non-comm-cental then we MUST
> MOZ_CRASH when we find a protocol that defines that flag on non-comm-cental.
> Don't you think?

Sure, that's a good idea.  Will do.
Flags: needinfo?(ehsan)
This is only used in Thunderbird and SeaMonkey and makes origin
comparisons needlessly slow in Firefox.
Attachment #8838874 - Flags: review?(mcmanus)
Attachment #8838735 - Attachment is obsolete: true
Attachment #8838874 - Flags: review?(mcmanus) → review+
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3699678d46e
Stop supporting ORIGIN_IS_FULL_SPEC for Firefox; r=mcmanus
https://hg.mozilla.org/mozilla-central/rev/e3699678d46e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.