1. Remove this data member can reduce change need at stylo side in bug 1341667. 2. We can actually store url information right in nsStyleImage

Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee5aad506e775724c101c11cd91de969ee654e34

Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd9f94ab60ea7ffe63bac6ae0be2fb6ca0aebb2a

recook part 1 to make code even simpler

Comment on attachment 8853000 [details] Bug 1352096 - Part 1. Implement nsStyleImage::SetURL and GetURLValueData. https://reviewboard.mozilla.org/r/125122/#review130234 ::: layout/style/nsStyleStruct.h:367 (Diff revision 3) > - > + URLValueData* GetURLValueData() const { > + return mImageValue; > + } We don't really need this function, since the caller can just use GetImageValue() instead. ::: layout/style/nsStyleStruct.h:396 (Diff revision 3) > enum nsStyleImageType { > eStyleImageType_Null, > eStyleImageType_Image, > eStyleImageType_Gradient, > - eStyleImageType_Element > + eStyleImageType_Element, > + eStyleImageType_Url Nit: I would write this as "eStyleImageType_URL". ("Url" is more of a Rust-like way to write abbreviations.) ::: layout/style/nsStyleStruct.h:441 (Diff revision 3) > void SetNull(); > void SetImageRequest(already_AddRefed<nsStyleImageRequest> aImage); > void SetGradientData(nsStyleGradient* aGradient); > void SetElementId(const char16_t* aElementId); > void SetCropRect(mozilla::UniquePtr<nsStyleSides> aCropRect); > + void SetURLValueData(already_AddRefed<URLValueData> aData); I suggest making this "SetURLValue(already_AddRefed<URLValue>)", since we only ever pass in a concrete css::URLValue object. ::: layout/style/nsStyleStruct.h:562 (Diff revision 3) > + URLValueData* mUrlData; // See the comment in SetStyleImage's 'case > + // eCSSUnit_URL' section to know why we need to > + // keep url other then mImage. And then we can make this |URLValue* mURLValue|. ::: layout/style/nsStyleStruct.h:564 (Diff revision 3) > union { > nsStyleImageRequest* mImage; > nsStyleGradient* mGradient; > + URLValueData* mUrlData; // See the comment in SetStyleImage's 'case > + // eCSSUnit_URL' section to know why we need to > + // keep url other then mImage. s/keep url other then/store URLValues separately from mImage/. ::: layout/style/nsStyleStruct.cpp:2442 (Diff revision 3) > + case eStyleImageType_Url: > return true; I guess it is correct to always return true here (and in IsLoaded) because eStyleImageType_Url is only used for url() values that are fragment only or which refer to the same document, and so the document must always be loaded, right? (I see that IsComplete is used in nsSVGIntegrationUtils::IsMaskResourceReady.)

Comment on attachment 8853001 [details] Bug 1352096 - Part 2. Remove Layer::mSourceURI. https://reviewboard.mozilla.org/r/125124/#review130244 ::: commit-message-0ceec:3 (Diff revision 5) > +Bug 1352096 - Part 2. Remove Layer::mSourceURI. > + > +Now, remove Layer::mSourceURI, there are several benifit of doing this: benefit ::: commit-message-0ceec:4 (Diff revision 5) > +Bug 1352096 - Part 2. Remove Layer::mSourceURI. > + > +Now, remove Layer::mSourceURI, there are several benifit of doing this: > +1. Redue the size of nsStyleImage::Layer. Reduce ::: commit-message-0ceec:8 (Diff revision 5) > +Now, remove Layer::mSourceURI, there are several benifit of doing this: > +1. Redue the size of nsStyleImage::Layer. > +2. By storing style image and url information in nsStyleImage, we can remove > +many verbose comments. That is becasue there is no need to explain why we use > +mSourceURI here, or why we use nsStyleImage there anymore. > +3. Since all inforamtion store in on place, nsStyleImage, we can setup image "all information is stored in one place" ::: commit-message-0ceec:9 (Diff revision 5) > +1. Redue the size of nsStyleImage::Layer. > +2. By storing style image and url information in nsStyleImage, we can remove > +many verbose comments. That is becasue there is no need to explain why we use > +mSourceURI here, or why we use nsStyleImage there anymore. > +3. Since all inforamtion store in on place, nsStyleImage, we can setup image > +request or ulr by one single Gecko_SetUrlImageValue call. URLs ::: layout/style/nsStyleStruct.cpp:3011 (Diff revision 5) > - DefinitelyEqualURIs(mSourceURI, aOther.mSourceURI); > + DefinitelyEqualURIs(mImage.GetURLValueData(), > + aOther.mImage.GetURLValueData()); We don't need this, since we compare mImage above.

Pushed by cku@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1d1403165215 Part 1. Implement nsStyleImage::SetURL and GetURLValueData. r=heycam https://hg.mozilla.org/integration/autoland/rev/39a3c3fbfabd Part 2. Remove Layer::mSourceURI. r=heycam

https://hg.mozilla.org/mozilla-central/rev/1d1403165215 https://hg.mozilla.org/mozilla-central/rev/39a3c3fbfabd