Bug 1370705 - Move attribute change effects from HTMLImageElement::BeforeMaybeChangeAttr to HTMLImageElement::AfterMaybeChangeAttr r=bz

It logically makes more sense for these effects to happen after the attribute has actually been changed and moving them allows us to get rid of the member variable HTMLImageElement::mForceReload.

MozReview-Commit-ID: IJBF3AHVb0U

--HG--
extra : rebase_source : fe3ae2a0cc55ded9702fb7654261ffee83a52057
This commit is contained in:
Kirk Steuber 2017-06-09 09:46:54 -07:00
parent def2ca72cb
commit 0312c4f54a
4 changed files with 46 additions and 60 deletions

View File

@ -4030,9 +4030,16 @@ Element::GetReferrerPolicyAsEnum()
{
if (IsHTMLElement()) {
const nsAttrValue* referrerValue = GetParsedAttr(nsGkAtoms::referrerpolicy);
if (referrerValue && referrerValue->Type() == nsAttrValue::eEnum) {
return net::ReferrerPolicy(referrerValue->GetEnumValue());
}
return ReferrerPolicyFromAttr(referrerValue);
}
return net::RP_Unset;
}
net::ReferrerPolicy
Element::ReferrerPolicyFromAttr(const nsAttrValue* aValue)
{
if (aValue && aValue->Type() == nsAttrValue::eEnum) {
return net::ReferrerPolicy(aValue->GetEnumValue());
}
return net::RP_Unset;
}

View File

@ -1337,6 +1337,7 @@ public:
float FontSizeInflation();
net::ReferrerPolicy GetReferrerPolicyAsEnum();
net::ReferrerPolicy ReferrerPolicyFromAttr(const nsAttrValue* aValue);
/*
* Helpers for .dataset. This is implemented on Element, though only some

View File

@ -118,7 +118,6 @@ private:
HTMLImageElement::HTMLImageElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo)
: nsGenericHTMLElement(aNodeInfo)
, mForm(nullptr)
, mForceReload(false)
, mInDocResponsiveContent(false)
, mCurrentDensity(1.0)
{
@ -378,10 +377,6 @@ HTMLImageElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
const nsAttrValueOrString* aValue,
bool aNotify)
{
if (aValue) {
BeforeMaybeChangeAttr(aNameSpaceID, aName, *aValue, aNotify);
}
if (aNameSpaceID == kNameSpaceID_None && mForm &&
(aName == nsGkAtoms::name || aName == nsGkAtoms::id)) {
// remove the image from the hashtable as needed
@ -402,8 +397,11 @@ HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
const nsAttrValue* aValue,
const nsAttrValue* aOldValue, bool aNotify)
{
nsAttrValueOrString attrVal(aValue);
if (aValue) {
AfterMaybeChangeAttr(aNameSpaceID, aName, aNotify);
AfterMaybeChangeAttr(aNameSpaceID, aName, attrVal, aOldValue, true,
aNotify);
}
if (aNameSpaceID == kNameSpaceID_None && mForm &&
@ -420,8 +418,6 @@ HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
// parser or some such place; we'll get bound after all the attributes have
// been set, so we'll do the image load from BindToTree.
nsAttrValueOrString attrVal(aValue);
if (aName == nsGkAtoms::src &&
aNameSpaceID == kNameSpaceID_None &&
!aValue) {
@ -466,18 +462,19 @@ HTMLImageElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName,
const nsAttrValueOrString& aValue,
bool aNotify)
{
BeforeMaybeChangeAttr(aNamespaceID, aName, aValue, aNotify);
AfterMaybeChangeAttr(aNamespaceID, aName, aNotify);
AfterMaybeChangeAttr(aNamespaceID, aName, aValue, nullptr, false, aNotify);
return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName,
aValue, aNotify);
}
void
HTMLImageElement::BeforeMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
const nsAttrValueOrString& aValue,
bool aNotify)
HTMLImageElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
const nsAttrValueOrString& aValue,
const nsAttrValue* aOldValue,
bool aValueMaybeChanged, bool aNotify)
{
bool forceReload = false;
// We need to force our image to reload. This must be done here, not in
// AfterSetAttr or BeforeSetAttr, because we want to do it even if the attr is
// being set to its existing value, which is normally optimized away as a
@ -488,9 +485,6 @@ HTMLImageElement::BeforeMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
// spec.
//
// Both cases handle unsetting src in AfterSetAttr
//
// Much of this should probably happen in AfterMaybeChangeAttr.
// See Bug 1370705
if (aNamespaceID == kNameSpaceID_None &&
aName == nsGkAtoms::src) {
@ -515,10 +509,14 @@ HTMLImageElement::BeforeMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
mNewRequestsWillNeedAnimationReset = true;
// Force image loading here, so that we'll try to load the image from
// network if it's set to be not cacheable... If we change things so that
// the state gets in Element's attr-setting happen around this
// LoadImage call, we could start passing false instead of aNotify
// here.
// network if it's set to be not cacheable.
// Potentially, false could be passed here rather than aNotify since
// UpdateState will be called by SetAttrAndNotify, but there are two
// obstacles to this: 1) LoadImage will end up calling
// UpdateState(aNotify), and we do not want it to call UpdateState(false)
// when aNotify is true, and 2) When this function is called by
// OnAttrSetButNotChanged, SetAttrAndNotify will not subsequently call
// UpdateState.
LoadImage(aValue.String(), true, aNotify, eImageLoadType_Normal);
mNewRequestsWillNeedAnimationReset = false;
@ -526,40 +524,31 @@ HTMLImageElement::BeforeMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
} else if (aNamespaceID == kNameSpaceID_None &&
aName == nsGkAtoms::crossorigin &&
aNotify) {
nsAttrValue attrValue;
ParseCORSValue(aValue.String(), attrValue);
if (GetCORSMode() != AttrValueToCORSMode(&attrValue)) {
if (aValueMaybeChanged && GetCORSMode() != AttrValueToCORSMode(aOldValue)) {
// Force a new load of the image with the new cross origin policy.
mForceReload = true;
forceReload = true;
}
} else if (aName == nsGkAtoms::referrerpolicy &&
aNamespaceID == kNameSpaceID_None &&
aNotify) {
ReferrerPolicy referrerPolicy = AttributeReferrerPolicyFromString(aValue.String());
ReferrerPolicy referrerPolicy = GetImageReferrerPolicy();
if (!InResponsiveMode() &&
referrerPolicy != RP_Unset &&
referrerPolicy != GetImageReferrerPolicy()) {
aValueMaybeChanged &&
referrerPolicy != ReferrerPolicyFromAttr(aOldValue)) {
// XXX: Bug 1076583 - We still use the older synchronous algorithm
// Because referrerPolicy is not treated as relevant mutations, setting
// the attribute will neither trigger a reload nor update the referrer
// policy of the loading channel (whether it has previously completed or
// not). Force a new load of the image with the new referrerpolicy.
mForceReload = true;
forceReload = true;
}
}
return;
}
void
HTMLImageElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
bool aNotify)
{
// Because we load image synchronously in non-responsive-mode, we need to do
// reload after the attribute has been set if the reload is triggerred by
// cross origin changing.
if (mForceReload) {
mForceReload = false;
if (forceReload) {
// Mark channel as urgent-start before load image if the image load is
// initaiated by a user interaction.
mUseUrgentStartForChannel = EventStateManager::IsHandlingUserInput();
@ -575,8 +564,6 @@ HTMLImageElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
ForceReload(aNotify);
}
}
return;
}
nsresult

View File

@ -361,34 +361,25 @@ private:
static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
GenericSpecifiedValues* aGenericData);
/**
* This function is called by BeforeSetAttr and OnAttrSetButNotChanged.
* This function is called by AfterSetAttr and OnAttrSetButNotChanged.
* It will not be called if the value is being unset.
*
* @param aNamespaceID the namespace of the attr being set
* @param aName the localname of the attribute being set
* @param aValue the value it's being set to represented as either a string or
* a parsed nsAttrValue.
* @param aNotify Whether we plan to notify document observers.
*/
void BeforeMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
const nsAttrValueOrString& aValue,
bool aNotify);
/**
* This function is called by AfterSetAttr and OnAttrSetButNotChanged.
* It will not be called if the value is being unset.
*
* @param aNamespaceID the namespace of the attr being set
* @param aName the localname of the attribute being set
* @param aOldValue the value previously set. Will be null if no value was
* previously set. This value should only be used when
* aValueMaybeChanged is true; when aValueMaybeChanged is false,
* aOldValue should be considered unreliable.
* @param aValueMaybeChanged will be false when this function is called from
* OnAttrSetButNotChanged to indicate that the value was not changed.
* @param aNotify Whether we plan to notify document observers.
*/
void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
bool aNotify);
/**
* Used by BeforeMaybeChangeAttr and AfterMaybeChangeAttr to keep track of
* whether a reload needs to be forced after an attribute change that is
* currently in progress.
*/
bool mForceReload;
const nsAttrValueOrString& aValue,
const nsAttrValue* aOldValue,
bool aValueMaybeChanged, bool aNotify);
bool mInDocResponsiveContent;
RefPtr<ImageLoadTask> mPendingImageLoadTask;