Reduced with m-c 20200130-f97c48da9cee

This was first seen with m-c 20200125-f7f534f08b48 and is happening frequently.

Assertion failure: nsContentUtils::IsSafeToRunScript() || mOwnerContent->OwnerDoc()->IsStaticDocument() (FrameLoader should never be initialized during document update or reflow!), at src/dom/base/nsFrameLoader.cpp:1977

#0 nsFrameLoader::AssertSafeToInit() src/dom/base/nsFrameLoader.cpp:1974:3
#1 nsFrameLoader::MaybeCreateDocShell() src/dom/base/nsFrameLoader.cpp:1989:3
#2 nsFrameLoader::GetBrowsingContext() src/dom/base/nsFrameLoader.cpp:3215:15
#3 nsGenericHTMLFrameElement::AfterMaybeChangeAttr(int, nsAtom*, nsAttrValueOrString const*, nsIPrincipal*, bool) src/dom/html/nsGenericHTMLFrameElement.cpp:344:40
#4 nsGenericHTMLFrameElement::AfterSetAttr(int, nsAtom*, nsAttrValue const*, nsAttrValue const*, nsIPrincipal*, bool) src/dom/html/nsGenericHTMLFrameElement.cpp:285:5
#5 mozilla::dom::Element::SetAttrAndNotify(int, nsAtom*, nsAtom*, nsAttrValue const*, nsAttrValue&, nsIPrincipal*, unsigned char, bool, bool, bool, mozilla::dom::Document*, mozAutoDocUpdate const&) src/dom/base/Element.cpp:2339:10
#6 mozilla::dom::Element::SetAttr(int, nsAtom*, nsAtom*, nsTSubstring<char16_t> const&, nsIPrincipal*, bool) src/dom/base/Element.cpp:2199:10
#7 SetAttr src/obj-firefox/dist/include/mozilla/dom/Element.h:830:12
#8 SetAttr src/obj-firefox/dist/include/mozilla/dom/Element.h:826:12
#9 SetAttr src/obj-firefox/dist/include/mozilla/dom/Element.h:1564:14
#10 SetHTMLAttr src/dom/html/nsGenericHTMLElement.h:718:5
#11 mozilla::dom::HTMLIFrameElement::SetName(nsTSubstring<char16_t> const&, mozilla::ErrorResult&) src/obj-firefox/dist/include/mozilla/dom/HTMLIFrameElement.h:67:5
#12 mozilla::dom::HTMLIFrameElement_Binding::set_name(JSContext*, JS::Handle<JSObject*>, void*, JSJitSetterCallArgs) src/obj-firefox/dom/bindings/HTMLIFrameElementBinding.cpp:266:24
#13 bool mozilla::dom::binding_detail::GenericSetter<mozilla::dom::binding_detail::NormalThisPolicy>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3099:8
#14 CallJSNative src/js/src/vm/Interpreter.cpp:450:13
#15 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:542:12
#16 InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:605:10
#17 js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:622:8
#18 js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) src/js/src/vm/Interpreter.cpp:760:10
#19 SetExistingProperty(JSContext*, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyResult>, JS::ObjectOpResult&) src/js/src/vm/NativeObject.cpp:2956:8
#20 bool js::NativeSetProperty<(js::QualifiedBool)1>(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) src/js/src/vm/NativeObject.cpp:2985:14
#21 js::SetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) src/js/src/vm/ObjectOperations-inl.h:283:10
#22 js::jit::DoSetPropFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICSetProp_Fallback*, JS::Value*, JS::Handle<JS::Value>, JS::Handle<JS::Value>) src/js/src/jit/BaselineIC.cpp:2704:10

A Pernosco session is available here: https://pernos.co/debug/Q3p6zrNZoFMl5aTnhLGJPA/index.html

I think this is the same as one existing bug, which I can't now find.
The behavior of nsFrameLoader::GetBrowsingContext() is basically nuts. It shouldn't create anything. Right now it is very error prone to use.
This is kind of a regression from bug 1578628.
The old code was explicitly using GetExistingDocShell()

Carrying over the tracking flag from bug 1611764.

Carrying over the crash signature from bug 1611764 so we don't loose the stability impact.

Bughunter found this on https://www.optumbank.com/

This brings back some of the behavior we had before bug 1578628.

Comment on attachment 9124387 [details]
Bug 1612431, it is enough to update existing browsing context name when changing iframe.name, r=kmag

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The crash is just an assertion, but that does hint about unexpected behavior.
  Because of the problematic behavior we may end up running chrome during a time when it is not expected. I haven't found a way to run content JS.
  Perhaps this is just sec-mod after all?
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: 71 and higher
• If not all supported branches, which bug introduced the flaw?: Bug 1578628
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: the patch should work with beta given how trivial it is.
• How likely is this patch to cause regressions; how much testing does it need?: The patch should bring back the old behavior

Comment on attachment 9124387 [details]
Bug 1612431, it is enough to update existing browsing context name when changing iframe.name, r=kmag

sec-approval+ dveditz

https://hg.mozilla.org/integration/autoland/rev/656b57e594def0996f3f5518f36c6919636d9209
https://hg.mozilla.org/mozilla-central/rev/656b57e594de