Closed Bug 1351957 Opened 3 years ago Closed 3 years ago

stylo: Use a single thread-safe holder for base uri, referrer, and principal

Categories

(Core :: CSS Parsing and Computation, enhancement, P1)

53 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Blocks: 1352025
Priority: -- → P1
Comment on attachment 8852842 [details]
Bug 1351957 - Create URLExtraData for holding base uri, referrer, and principal.

https://reviewboard.mozilla.org/r/125000/#review128340

::: layout/style/ServoBindings.cpp:1533
(Diff revision 1)
>  {
>    MOZ_ASSERT(NS_IsMainThread());
>    MOZ_ASSERT(aLoader, "Should've catched this before");
>    MOZ_ASSERT(aParent, "Only used for @import, so parent should exist!");
>    MOZ_ASSERT(aURLString, "Invalid URLs shouldn't be loaded!");
> -  MOZ_ASSERT(aBaseURI, "Need a base URI");
> +  MOZ_ASSERT(aBaseURLData, "Need a base URL data");

Nit: s/a base/base/

::: layout/style/nsCSSValue.h:134
(Diff revision 1)
>    // For both constructors aString must not be null.
>    // For both constructors aOriginPrincipal must not be null.
>    // Construct with a base URI; this will create the actual URI lazily from
>    // aString and aBaseURI.

Please update this comment to talk about the requirements of the URLExtraData object.

::: layout/style/nsCSSValue.cpp:2864
(Diff revision 1)
> -          (mOriginPrincipal == aOther.mOriginPrincipal ||
> -           self.mOriginPrincipal.get()->Equals(other.mOriginPrincipal.get())) &&
> +          (self->GetPrincipal() == other->GetPrincipal() ||
> +           self->GetPrincipal()->Equals(other->GetPrincipal())) &&

Does this mean the principal can never be null?
Attachment #8852842 - Flags: review?(cam) → review+
Comment on attachment 8852842 [details]
Bug 1351957 - Create URLExtraData for holding base uri, referrer, and principal.

https://reviewboard.mozilla.org/r/125000/#review128340

> Does this mean the principal can never be null?

The assertions in constructors of `URLValueData` should enforce that. We cannot make `URLExtraData` check that because there seem to be cases where the principal of `URLExtraData` can be null. (I forgot the details, but if I check that at the constructor of `URLExtraData`, it asserts in many tests.)
Attachment #8852843 - Attachment is obsolete: true
servo/servo#16235
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/21926c2cc38d
Create URLExtraData for holding base uri, referrer, and principal. r=heycam
https://hg.mozilla.org/mozilla-central/rev/21926c2cc38d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.