removeAttribute("sizes") explodes (covered in picture mutation tests)

Enhances this helper to also let us avoid this dance: aValue ? aValue->GetStringValue() : EmptyString()

Use enhanced nsAttrValueOrString helper to avoid this dance. (I'm not sure how I managed to get this right three-of-four times in this function)

Comment on attachment 8498593 [details] [diff] [review] Allow nsAttrValueOrString helper to accept potentially-null pointers r=me, though it seems like a ctor overload explicitly taking nullptr and setting mStringPtr to &mCheapString would work just as well and not need the extra runtime check in String().

Comment on attachment 8498594 [details] [diff] [review] Avoid null deref in HTMLImageElement::AfterSetAttr with <picture> enabled r=me

(In reply to Boris Zbarsky [:bz] from comment #3) > Comment on attachment 8498593 [details] [diff] [review] > Allow nsAttrValueOrString helper to accept potentially-null pointers > > r=me, though it seems like a ctor overload explicitly taking nullptr and > setting mStringPtr to &mCheapString would work just as well and not need the > extra runtime check in String(). Not sure I understand -- If we had an |explicit nsAttrValueOrString(std::nullptr_t)| constructor we'd still need a runtime null check at the declaration site to select it, vs a lazy-null check only when .String() is called

> we'd still need a runtime null check at the declaration site to select it Ah, because you're not actually constructing with nullptr, just with a might-be-null pointer? Right, never mind then!

Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/03b6a747eac1 https://hg.mozilla.org/integration/mozilla-inbound/rev/6345f1767ed7 Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8af04bb0dc51

https://hg.mozilla.org/mozilla-central/rev/03b6a747eac1 https://hg.mozilla.org/mozilla-central/rev/6345f1767ed7