Bug 1311921 - Store base and relative URIs explicitly in nsStyleImageRequests for comparison purposes, rather than use css::ImageValues. r?bholley
MozReview-Commit-ID: 5aArKCI7Rhx
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -1908,126 +1908,135 @@ nsStyleGradient::HasCalc()
return mBgPosX.IsCalcUnit() || mBgPosY.IsCalcUnit() || mAngle.IsCalcUnit() ||
mRadiusX.IsCalcUnit() || mRadiusY.IsCalcUnit();
}
// --------------------
// nsStyleImageRequest
+static void
+MaybeUntrackAndUnlock(nsStyleImageRequest::Mode aModeFlags,
+ imgRequestProxy* aRequestProxy,
+ ImageTracker* aImageTracker)
+{
+ MOZ_ASSERT(NS_IsMainThread());
+ MOZ_ASSERT(aRequestProxy);
+ MOZ_ASSERT(aImageTracker);
+
+ if (aModeFlags & nsStyleImageRequest::Mode::Lock) {
+ aRequestProxy->UnlockImage();
+ }
+
+ if (aModeFlags & nsStyleImageRequest::Mode::Discard) {
+ aRequestProxy->RequestDiscard();
+ }
+
+ if (aModeFlags & nsStyleImageRequest::Mode::Track) {
+ aImageTracker->Remove(aRequestProxy);
+ }
+}
+
/**
* Runnable to release the nsStyleImageRequest's mRequestProxy,
- * mImageValue and mImageValue on the main thread, and to perform
+ * mImageValue and mImageTracker on the main thread, and to perform
* any necessary unlocking and untracking of the image.
*/
class StyleImageRequestCleanupTask : public mozilla::Runnable
{
public:
typedef nsStyleImageRequest::Mode Mode;
StyleImageRequestCleanupTask(Mode aModeFlags,
already_AddRefed<imgRequestProxy> aRequestProxy,
already_AddRefed<css::ImageValue> aImageValue,
already_AddRefed<ImageTracker> aImageTracker)
: mModeFlags(aModeFlags)
, mRequestProxy(aRequestProxy)
, mImageValue(aImageValue)
, mImageTracker(aImageTracker)
{
+ MOZ_ASSERT(!!mRequestProxy == !!mImageTracker);
}
NS_IMETHOD Run() final
{
- MOZ_ASSERT(NS_IsMainThread());
-
- if (!mRequestProxy) {
- return NS_OK;
+ if (mRequestProxy) {
+ MaybeUntrackAndUnlock(mModeFlags, mRequestProxy, mImageTracker);
}
-
- MOZ_ASSERT(mImageTracker);
-
- if (mModeFlags & Mode::Lock) {
- mRequestProxy->UnlockImage();
- }
-
- if (mModeFlags & Mode::Discard) {
- mRequestProxy->RequestDiscard();
- }
-
- if (mModeFlags & Mode::Track) {
- mImageTracker->Remove(mRequestProxy);
- }
-
return NS_OK;
}
protected:
virtual ~StyleImageRequestCleanupTask() { MOZ_ASSERT(NS_IsMainThread()); }
private:
Mode mModeFlags;
- // Since we always dispatch this runnable to the main thread, these will be
- // released on the main thread when the runnable itself is released.
+ // These will be released on the main thread when the
+ // StyleImageRequestCleanupTask is deleted.
RefPtr<imgRequestProxy> mRequestProxy;
RefPtr<css::ImageValue> mImageValue;
RefPtr<ImageTracker> mImageTracker;
};
nsStyleImageRequest::nsStyleImageRequest(Mode aModeFlags,
imgRequestProxy* aRequestProxy,
css::ImageValue* aImageValue,
ImageTracker* aImageTracker)
: mRequestProxy(aRequestProxy)
- , mImageValue(aImageValue)
, mImageTracker(aImageTracker)
+ , mBaseURI(aImageValue->mBaseURI)
+ , mURIString(aImageValue->mString)
, mModeFlags(aModeFlags)
, mResolved(true)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aRequestProxy);
- MOZ_ASSERT(aImageValue);
MOZ_ASSERT(aImageTracker);
MaybeTrackAndLock();
}
nsStyleImageRequest::nsStyleImageRequest(
Mode aModeFlags,
nsStringBuffer* aURLBuffer,
already_AddRefed<PtrHolder<nsIURI>> aBaseURI,
already_AddRefed<PtrHolder<nsIURI>> aReferrer,
already_AddRefed<PtrHolder<nsIPrincipal>> aPrincipal)
: mModeFlags(aModeFlags)
, mResolved(false)
{
mImageValue = new css::ImageValue(aURLBuffer, Move(aBaseURI),
Move(aReferrer), Move(aPrincipal));
+ mBaseURI = mImageValue->mBaseURI;
+ mURIString = mImageValue->mString;
}
nsStyleImageRequest::~nsStyleImageRequest()
{
- // We may or may not be being destroyed on the main thread. To clean
- // up, we must untrack and unlock the image (depending on mModeFlags),
- // and release mRequestProxy and mImageValue, all on the main thread.
- {
- RefPtr<StyleImageRequestCleanupTask> task =
- new StyleImageRequestCleanupTask(mModeFlags,
- mRequestProxy.forget(),
- mImageValue.forget(),
- mImageTracker.forget());
- if (NS_IsMainThread()) {
- task->Run();
- } else {
+ if (NS_IsMainThread()) {
+ // In the main thread case, mRequestProxy, mImageValue and
+ // mImageTracker will be released as normal after the nsStyleImageRequest
+ // has run.
+ if (mRequestProxy) {
+ MaybeUntrackAndUnlock(mModeFlags, mRequestProxy, mImageTracker);
+ }
+ } else {
+ // In the OMT case, we transfer ownership of these objects to the
+ // cleanup task runnable, which will call MaybeUntrackAndUnlock and
+ // then release the objects.
+ if (mRequestProxy || mImageValue) {
+ RefPtr<StyleImageRequestCleanupTask> task =
+ new StyleImageRequestCleanupTask(mModeFlags,
+ mRequestProxy.forget(),
+ mImageValue.forget(),
+ mImageTracker.forget());
NS_DispatchToMainThread(task.forget());
}
}
-
- MOZ_ASSERT(!mRequestProxy);
- MOZ_ASSERT(!mImageValue);
- MOZ_ASSERT(!mImageTracker);
}
bool
nsStyleImageRequest::Resolve(nsPresContext* aPresContext)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(!IsResolved(), "already resolved");
@@ -2039,16 +2048,19 @@ nsStyleImageRequest::Resolve(nsPresConte
// document.
mImageValue->Initialize(aPresContext->Document());
nsCSSValue value;
value.SetImageValue(mImageValue);
mRequestProxy = value.GetPossiblyStaticImageValue(aPresContext->Document(),
aPresContext);
+ // We no longer need the ImageValue.
+ mImageValue = nullptr;
+
if (!mRequestProxy) {
// The URL resolution or image load failed.
return false;
}
mImageTracker = aPresContext->Document()->ImageTracker();
MaybeTrackAndLock();
return true;
@@ -2066,20 +2078,32 @@ nsStyleImageRequest::MaybeTrackAndLock()
mImageTracker->Add(mRequestProxy);
}
if (mModeFlags & Mode::Lock) {
mRequestProxy->LockImage();
}
}
+static const char16_t*
+GetBufferValue(nsStringBuffer* aBuffer)
+{
+ // Since the nsStringBuffers we work with come from a css::ImageValue, we
+ // can assume (just like nsCSSValue::GetBufferValue does) that we have
+ // a 16 bit, null terminated string.
+ return static_cast<char16_t*>(aBuffer->Data());
+}
+
bool
nsStyleImageRequest::DefinitelyEquals(const nsStyleImageRequest& aOther) const
{
- return DefinitelyEqualURIs(mImageValue, aOther.mImageValue);
+ return mBaseURI == aOther.mBaseURI &&
+ (mURIString == aOther.mURIString ||
+ NS_strcmp(GetBufferValue(mURIString),
+ GetBufferValue(aOther.mURIString)) == 0);
}
// --------------------
// CachedBorderImageData
//
void
CachedBorderImageData::SetCachedSVGViewportSize(
const mozilla::Maybe<nsSize>& aSVGViewportSize)
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -300,22 +300,28 @@ private:
* any thread. The nsStyleImageRequest is not considered "resolved"
* at this point, and the Resolve() method must be called later
* to initiate the image load and make calls to get() valid.
*
* Calls to TrackImage(), UntrackImage(), LockImage(), UnlockImage() and
* RequestDiscard() are made to the imgRequestProxy and ImageTracker as
* appropriate, according to the mode flags passed in to the constructor.
*
- * The main thread constructor takes a pointer to the css::ImageValue that
- * is the specified url() value, while the off-main-thread constructor
- * creates a new css::ImageValue to represent the url() information passed
- * to the constructor. This ImageValue is held on to for the comparisons done
- * in DefinitelyEquals(), so that we don't need to call into the non-OMT-safe
- * Equals() on the nsIURI objects returned from imgRequestProxy::GetURI().
+ * The main thread constructor takes a pointer to the already-created
+ * imgRequestProxy, and the css::ImageValue that was used while creating it.
+ * The ImageValue object is only used to grab the URL details to store
+ * into mBaseURI and mURIString.
+ *
+ * The off-main-thread constructor creates a new css::ImageValue to
+ * hold all the data required to resolve the imgRequestProxy later. This
+ * constructor also stores the URL details into mbaseURI and mURIString.
+ * The ImageValue is held on to in mImageTracker until the Resolve call.
+ *
+ * We use mBaseURI and mURIString so that we can perform nsStyleImageRequest
+ * equality comparisons without needing an imgRequestProxy.
*/
class nsStyleImageRequest
{
public:
// Flags describing whether the imgRequestProxy must be tracked in the
// ImageTracker, whether LockImage/UnlockImage calls will be made
// when obtaining and releasing the imgRequestProxy, and whether
// RequestDiscard will be called on release.
@@ -347,32 +353,39 @@ public:
MOZ_ASSERT(IsResolved(), "Resolve() must be called first");
MOZ_ASSERT(NS_IsMainThread());
return mRequestProxy.get();
}
const imgRequestProxy* get() const {
return const_cast<nsStyleImageRequest*>(this)->get();
}
- // Returns whether the ImageValue objects in the two nsStyleImageRequests
- // return true from URLValueData::DefinitelyEqualURIs.
+ // Returns whether the mBaseURI and mURIString values in the two
+ // nsStyleImageRequests are equal.
bool DefinitelyEquals(const nsStyleImageRequest& aOther) const;
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsStyleImageRequest);
private:
~nsStyleImageRequest();
nsStyleImageRequest& operator=(const nsStyleImageRequest& aOther) = delete;
void MaybeTrackAndLock();
RefPtr<imgRequestProxy> mRequestProxy;
RefPtr<mozilla::css::ImageValue> mImageValue;
RefPtr<mozilla::dom::ImageTracker> mImageTracker;
+ // Components that are (or were) used to produce the nsIURI for resolving
+ // the imgRequestProxy. We use these in DefinitelyEquals, rather than
+ // comparing the URI information in the imgRequestProxy objects, since
+ // they may not yet be resolved (or might have failed to resolve).
+ mozilla::PtrHandle<nsIURI> mBaseURI;
+ RefPtr<nsStringBuffer> mURIString;
+
Mode mModeFlags;
bool mResolved;
};
MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(nsStyleImageRequest::Mode)
enum nsStyleImageType {
eStyleImageType_Null,