diff --git a/dom/base/nsAttrValue.cpp b/dom/base/nsAttrValue.cpp index 98b479fb7ce4..d3469dc01746 100644 --- a/dom/base/nsAttrValue.cpp +++ b/dom/base/nsAttrValue.cpp @@ -1735,8 +1735,8 @@ nsAttrValue::LoadImage(nsIDocument* aDocument) MiscContainer* cont = GetMiscContainer(); mozilla::css::URLValue* url = cont->mValue.mURL; mozilla::css::ImageValue* image = - new css::ImageValue(url->GetURI(), url->mString, url->mBaseURI, - url->mReferrer, url->mOriginPrincipal, aDocument); + new css::ImageValue(url->GetURI(), url->mString, url->mReferrer, + url->mOriginPrincipal, aDocument); NS_ADDREF(image); cont->mValue.mImage = image; diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index 268379a5171c..05a94e90416d 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -981,7 +981,7 @@ nsGenericHTMLElement::ParseBackgroundAttribute(int32_t aNamespaceID, } mozilla::css::URLValue *url = - new mozilla::css::URLValue(uri, buffer, baseURI, doc->GetDocumentURI(), + new mozilla::css::URLValue(uri, buffer, doc->GetDocumentURI(), NodePrincipal()); aResult.SetTo(url, &aValue); return true; diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index e349fb8b120f..2c176dfc7b35 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -9175,12 +9175,11 @@ nsCSSFrameConstructor::CaptureStateForFramesOf(nsIContent* aContent, } } -static bool -DefinitelyEqualURIsAndPrincipal(mozilla::css::URLValue* aURI1, - mozilla::css::URLValue* aURI2) +static bool EqualURIs(mozilla::css::URLValue *aURI1, + mozilla::css::URLValue *aURI2) { - return aURI1 == aURI2 || - (aURI1 && aURI2 && aURI1->DefinitelyEqualURIsAndPrincipal(*aURI2)); + return aURI1 == aURI2 || // handle null==null, and optimize + (aURI1 && aURI2 && aURI1->URIEquals(*aURI2)); } nsStyleContext* @@ -9216,8 +9215,7 @@ nsCSSFrameConstructor::MaybeRecreateFramesForElement(Element* aElement) return newContext; } const nsStyleDisplay* oldDisp = oldContext->PeekStyleDisplay(); - if (oldDisp && - DefinitelyEqualURIsAndPrincipal(disp->mBinding, oldDisp->mBinding)) { + if (oldDisp && EqualURIs(disp->mBinding, oldDisp->mBinding)) { return newContext; } } diff --git a/layout/style/StyleAnimationValue.cpp b/layout/style/StyleAnimationValue.cpp index 92f4350ba7b4..ffcf558f18a8 100644 --- a/layout/style/StyleAnimationValue.cpp +++ b/layout/style/StyleAnimationValue.cpp @@ -291,13 +291,9 @@ FragmentOrURLToURLValue(FragmentOrURL* aUrl, nsIDocument* aDoc) nsString path; aUrl->GetSourceString(path); RefPtr uriStringBuffer = nsCSSValue::BufferFromString(path); - // XXXheycam We should store URLValue objects inside FragmentOrURLs so that - // we can extract the original base URI, referrer and principal to pass in - // here. RefPtr result = new mozilla::css::URLValue(aUrl->GetSourceURL(), uriStringBuffer, - aUrl->GetSourceURL(), aDoc->GetDocumentURI(), - aDoc->NodePrincipal()); + aDoc->GetDocumentURI(), aDoc->NodePrincipal()); return result.forget(); } diff --git a/layout/style/nsCSSValue.cpp b/layout/style/nsCSSValue.cpp index 0fd8a6b5e2a3..3496c746b396 100644 --- a/layout/style/nsCSSValue.cpp +++ b/layout/style/nsCSSValue.cpp @@ -761,7 +761,6 @@ void nsCSSValue::StartImageLoad(nsIDocument* aDocument) const mozilla::css::ImageValue* image = new mozilla::css::ImageValue(mValue.mURL->GetURI(), mValue.mURL->mString, - mValue.mURL->mBaseURI, mValue.mURL->mReferrer, mValue.mURL->mOriginPrincipal, aDocument); @@ -2593,21 +2592,17 @@ nsCSSValue::Array::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) cons css::URLValueData::URLValueData(already_AddRefed> aURI, nsStringBuffer* aString, - already_AddRefed> aBaseURI, already_AddRefed> aReferrer, already_AddRefed> aOriginPrincipal) : mURI(Move(aURI)) - , mBaseURI(Move(aBaseURI)) , mString(aString) , mReferrer(Move(aReferrer)) , mOriginPrincipal(Move(aOriginPrincipal)) , mURIResolved(true) , mLocalURLFlag(IsLocalRefURL(aString)) { - MOZ_ASSERT(mString); - MOZ_ASSERT(mBaseURI); - MOZ_ASSERT(mOriginPrincipal); + MOZ_ASSERT(mOriginPrincipal, "Must have an origin principal"); } css::URLValueData::URLValueData(nsStringBuffer* aString, @@ -2615,16 +2610,14 @@ css::URLValueData::URLValueData(nsStringBuffer* aString, already_AddRefed> aReferrer, already_AddRefed> aOriginPrincipal) - : mBaseURI(Move(aBaseURI)) + : mURI(Move(aBaseURI)) , mString(aString) , mReferrer(Move(aReferrer)) , mOriginPrincipal(Move(aOriginPrincipal)) , mURIResolved(false) , mLocalURLFlag(IsLocalRefURL(aString)) { - MOZ_ASSERT(aString); - MOZ_ASSERT(mBaseURI); - MOZ_ASSERT(mOriginPrincipal); + MOZ_ASSERT(mOriginPrincipal, "Must have an origin principal"); } bool @@ -2640,44 +2633,50 @@ css::URLValueData::operator==(const URLValueData& aOther) const (mURI && aOther.mURI && NS_SUCCEEDED(mURI->Equals(aOther.mURI, &eq)) && eq)) && - (mBaseURI == aOther.mBaseURI || - (NS_SUCCEEDED(self.mBaseURI.get()->Equals(other.mBaseURI.get(), &eq)) && - eq)) && (mOriginPrincipal == aOther.mOriginPrincipal || - self.mOriginPrincipal.get()->Equals(other.mOriginPrincipal.get())) && - mLocalURLFlag == aOther.mLocalURLFlag; + self.mOriginPrincipal.get()->Equals(other.mOriginPrincipal.get())); } bool -css::URLValueData::DefinitelyEqualURIs(const URLValueData& aOther) const +css::URLValueData::MaybeUnresolvedURIEquals(const URLValueData& aOther) const { - return mBaseURI == aOther.mBaseURI && - (mString == aOther.mString || - NS_strcmp(nsCSSValue::GetBufferValue(mString), - nsCSSValue::GetBufferValue(aOther.mString))); + if (!mURIResolved || !aOther.mURIResolved) { + return false; + } + + return URIEquals(aOther); } bool -css::URLValueData::DefinitelyEqualURIsAndPrincipal( - const URLValueData& aOther) const +css::URLValueData::URIEquals(const URLValueData& aOther) const { - return mOriginPrincipal == aOther.mOriginPrincipal && - DefinitelyEqualURIs(aOther); + MOZ_ASSERT(mURIResolved && aOther.mURIResolved, + "How do you know the URIs aren't null?"); + bool eq; + // Cast away const so we can call nsIPrincipal::Equals. + auto& self = *const_cast(this); + auto& other = const_cast(aOther); + // Worth comparing GetURI() to aOther.GetURI() and mOriginPrincipal to + // aOther.mOriginPrincipal, because in the (probably common) case when this + // value was one of the ones that in fact did not change this will be our + // fast path to equality + return (mURI == aOther.mURI || + (NS_SUCCEEDED(mURI->Equals(aOther.mURI, &eq)) && eq)) && + (mOriginPrincipal == aOther.mOriginPrincipal || + self.mOriginPrincipal.get()->Equals(other.mOriginPrincipal.get())); } nsIURI* css::URLValueData::GetURI() const { - MOZ_ASSERT(NS_IsMainThread()); - if (!mURIResolved) { - MOZ_ASSERT(!mURI); + mURIResolved = true; + // Be careful to not null out mURI before we've passed it as the base URI nsCOMPtr newURI; NS_NewURI(getter_AddRefs(newURI), NS_ConvertUTF16toUTF8(nsCSSValue::GetBufferValue(mString)), - nullptr, const_cast(mBaseURI.get())); + nullptr, mURI); mURI = new PtrHolder(newURI.forget()); - mURIResolved = true; } return mURI; @@ -2707,11 +2706,10 @@ URLValue::URLValue(nsStringBuffer* aString, nsIURI* aBaseURI, nsIURI* aReferrer, MOZ_ASSERT(NS_IsMainThread()); } -URLValue::URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aBaseURI, - nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal) +URLValue::URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aReferrer, + nsIPrincipal* aOriginPrincipal) : URLValueData(do_AddRef(new PtrHolder(aURI)), aString, - do_AddRef(new PtrHolder(aBaseURI)), do_AddRef(new PtrHolder(aReferrer)), do_AddRef(new PtrHolder(aOriginPrincipal))) { @@ -2731,12 +2729,10 @@ css::URLValue::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const } css::ImageValue::ImageValue(nsIURI* aURI, nsStringBuffer* aString, - nsIURI* aBaseURI, nsIURI* aReferrer, - nsIPrincipal* aOriginPrincipal, + nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal, nsIDocument* aDocument) : URLValueData(do_AddRef(new PtrHolder(aURI)), aString, - do_AddRef(new PtrHolder(aBaseURI, false)), do_AddRef(new PtrHolder(aReferrer)), do_AddRef(new PtrHolder(aOriginPrincipal))) { diff --git a/layout/style/nsCSSValue.h b/layout/style/nsCSSValue.h index 609ff781b565..2387abef152c 100644 --- a/layout/style/nsCSSValue.h +++ b/layout/style/nsCSSValue.h @@ -106,24 +106,20 @@ struct URLValueData // Construct with the actual URI. URLValueData(already_AddRefed> aURI, nsStringBuffer* aString, - already_AddRefed> aBaseURI, already_AddRefed> aReferrer, already_AddRefed> aOriginPrincipal); bool operator==(const URLValueData& aOther) const; - // Returns true iff we know for sure, by comparing the mBaseURI pointer, - // the specified url() value mString, and the mLocalURLFlag, that these - // two URLValueData objects represent the same computed url() value. - // - // Doesn't look at mReferrer or mOriginPrincipal. - // - // Safe to call from any thread. - bool DefinitelyEqualURIs(const URLValueData& aOther) const; + // URIEquals only compares URIs and principals (unlike operator==, which + // also compares the original strings). URIEquals also assumes that the + // mURI member of both URL objects is non-null. Do NOT call this method + // unless you're sure this is the case. + bool URIEquals(const URLValueData& aOther) const; - // Smae as DefinitelyEqualURIs but additionally compares the nsIPrincipal - // pointers of the two URLValueData objects. - bool DefinitelyEqualURIsAndPrincipal(const URLValueData& aOther) const; + // Pretty much like URIEquals, but allows comparing unresolved URIs (returning + // false in that case). + bool MaybeUnresolvedURIEquals(const URLValueData& aOther) const; nsIURI* GetURI() const; @@ -132,11 +128,11 @@ struct URLValueData bool GetLocalURLFlag() const { return mLocalURLFlag; } private: - // mURI stores the lazily resolved URI. This may be null if the URI is - // invalid, even once resolved. + // If mURIResolved is false, mURI stores the base URI. + // If mURIResolved is true, mURI stores the URI we resolve to; this may be + // null if the URI is invalid. mutable PtrHandle mURI; public: - PtrHandle mBaseURI; RefPtr mString; PtrHandle mReferrer; PtrHandle mOriginPrincipal; @@ -155,8 +151,8 @@ struct URLValue : public URLValueData // These two constructors are safe to call only on the main thread. URLValue(nsStringBuffer* aString, nsIURI* aBaseURI, nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal); - URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aBaseURI, - nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal); + URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aReferrer, + nsIPrincipal* aOriginPrincipal); // This constructor is safe to call from any thread. URLValue(nsStringBuffer* aString, @@ -186,9 +182,8 @@ struct ImageValue : public URLValueData // aString must not be null. // // This constructor is only safe to call from the main thread. - ImageValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aBaseURI, - nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal, - nsIDocument* aDocument); + ImageValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aReferrer, + nsIPrincipal* aOriginPrincipal, nsIDocument* aDocument); ImageValue(const ImageValue&) = delete; ImageValue& operator=(const ImageValue&) = delete; diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index 7b4f5c3f94f2..d20a47e548a4 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -60,10 +60,10 @@ EqualURIs(nsIURI *aURI1, nsIURI *aURI2) } static bool -DefinitelyEqualURIsAndPrincipal(css::URLValue* aURI1, css::URLValue* aURI2) +MaybeUnresolvedURIEquals(css::URLValue *aURI1, css::URLValue *aURI2) { - return aURI1 == aURI2 || - (aURI1 && aURI2 && aURI1->DefinitelyEqualURIsAndPrincipal(*aURI2)); + return aURI1 == aURI2 || // handle null==null, and optimize + (aURI1 && aURI2 && aURI1->MaybeUnresolvedURIEquals(*aURI2)); } static @@ -3108,7 +3108,7 @@ nsStyleDisplay::CalcDifference(const nsStyleDisplay& aNewData) const { nsChangeHint hint = nsChangeHint(0); - if (!DefinitelyEqualURIsAndPrincipal(mBinding, aNewData.mBinding) + if (!MaybeUnresolvedURIEquals(mBinding, aNewData.mBinding) || mPosition != aNewData.mPosition || mDisplay != aNewData.mDisplay || mContain != aNewData.mContain