1497601 - Crash when attempting to land a Custom Element definition for XUL label: "panicked at 'Resolving style on unstyled element'"

Assignee

Description

•

Last year

When attempting to migration <label> to a Custom Element I see crashes on talos tests like tresize (https://treeherder.mozilla.org/logviewer.html#?job_id=204090867&repo=try&lineNumber=1731):

14:10:03     INFO -  TEST-INFO | started process 24158 (/home/cltbld/workspace/build/application/firefox/firefox -profile /tmp/tmpv2x6iR/profile http://localhost:33163/startup_test/tresize/addon/content/tresize-test.html)
14:10:04     INFO -  PID 24158 | thread '<unnamed>' panicked at 'Resolving style on unstyled element', libcore/option.rs:989:5
14:10:04     INFO -  PID 24158 | stack backtrace:
14:10:04     INFO -  PID 24158 |    0:     0x7f5570191262 - std::sys::unix::backtrace::tracing::imp::unwind_backtrace::hced3a4ed290bc7f5
14:10:04     INFO -  PID 24158 |    1:     0x7f55701833ee - std::panicking::default_hook::{{closure}}::h15181aeeb7b4bc75
14:10:04     INFO -  PID 24158 |    2:     0x7f55701828ff - std::panicking::default_hook::h63ab9a1a074f1506
14:10:04     INFO -  PID 24158 |    3:     0x7f557018251e - std::panicking::rust_panic_with_hook::hba05450791beb933
14:10:04     INFO -  PID 24158 |    4:     0x7f55701844cd - std::panicking::continue_panic_fmt::hd2a4220d2e2291e2
14:10:04     INFO -  PID 24158 |    5:     0x7f5570184475 - rust_begin_unwind
14:10:04     INFO -  PID 24158 |    6:     0x7f55701a077b - core::panicking::panic_fmt::h2c383e888dce42c0
14:10:04     INFO -  PID 24158 |    7:     0x7f55701a0881 - core::option::expect_failed::h7d982926f8026caa
14:10:04     INFO -  PID 24158 |    8:     0x7f556fedd9cb - Servo_ResolveStyle
14:10:04     INFO -  PID 24158 |    9:     0x7f556e3394dc - _ZN21nsCSSFrameConstructor20ResolveComputedStyleEP10nsIContent
14:10:04     INFO -  PID 24158 |   10:     0x7f556e333e7f - _ZN21nsCSSFrameConstructor34ConstructAnonymousContentForCanvasER23nsFrameConstructorStateP8nsIFrameP10nsIContent
14:10:04     INFO -  PID 24158 |   11:     0x7f556e332f58 - _ZN21nsCSSFrameConstructor24ConstructDocElementFrameEPN7mozilla3dom7ElementEP21nsILayoutHistoryState
14:10:04     INFO -  PID 24158 |   12:     0x7f556e33d4b3 - _ZN21nsCSSFrameConstructor20ContentRangeInsertedEP10nsIContentS1_P21nsILayoutHistoryStateNS_13InsertionKindE
14:10:04     INFO -  PID 24158 |   13:     0x7f556e307b85 - _ZN7mozilla9PresShell10InitializeEv
14:10:04     INFO -  PID 24158 |   14:     0x7f556ce9850c - _ZN13nsContentSink11StartLayoutEb
14:10:04     INFO -  PID 24158 |   15:     0x7f556e01c371 - _ZN16nsXMLContentSink18HandleStartElementEPKDsPS1_jjjb
14:10:04     INFO -  PID 24158 |   16:     0x7f556e01c48a - _ZThn200_N16nsXMLContentSink18HandleStartElementEPKDsPS1_jjj
14:10:04     INFO -  PID 24158 |   17:     0x7f556c9bb1a1 - _ZN13nsExpatDriver18HandleStartElementEPKDsPS1_
14:10:04     INFO -  PID 24158 |   18:     0x7f556e946c81 - doContent
14:10:04     INFO -  PID 24158 |   19:     0x7f556e944144 - doProlog
14:10:04     INFO -  PID 24158 |   20:     0x7f556e941287 - prologInitProcessor
14:10:04     INFO -  PID 24158 |   21:     0x7f556e940b9d - MOZ_XML_Parse
14:10:04     INFO -  PID 24158 |   22:     0x7f556c9bca47 - _ZN13nsExpatDriver11ParseBufferEPKDsjbPj
14:10:04     INFO -  PID 24158 |   23:     0x7f556c9bcdeb - _ZN13nsExpatDriver12ConsumeTokenER9nsScannerRb
14:10:04     INFO -  PID 24158 |   24:     0x7f556c9c0102 - _ZN8nsParser8TokenizeEb
14:10:04     INFO -  PID 24158 |   25:     0x7f556c9bf219 - _ZN8nsParser11ResumeParseEbbb
14:10:04     INFO -  PID 24158 |   26:     0x7f556c9c05af - _ZN8nsParser15OnDataAvailableEP10nsIRequestP11nsISupportsP14nsIInputStreammj
14:10:04     INFO -  PID 24158 |   27:     0x7f556c830c23 - _ZN12nsJARChannel15OnDataAvailableEP10nsIRequestP11nsISupportsP14nsIInputStreammj
14:10:04     INFO -  PID 24158 |   28:     0x7f556c1698bc - _ZN17nsInputStreamPump15OnStateTransferEv
14:10:04     INFO -  PID 24158 |   29:     0x7f556c169443 - _ZN17nsInputStreamPump18OnInputStreamReadyEP19nsIAsyncInputStream
14:10:04     INFO -  PID 24158 |   30:     0x7f556c0bcab0 - _ZN23nsInputStreamReadyEvent3RunEv
14:10:04     INFO -  PID 24158 |   31:     0x7f556c0dc80e - _ZN8nsThread16ProcessNextEventEbPb
14:10:04     INFO -  PID 24158 |   32:     0x7f556c0de837 - _Z19NS_ProcessNextEventP9nsIThreadb
14:10:04     INFO -  PID 24158 |   33:     0x7f556c51c4ed - _ZN7mozilla3ipc11MessagePump3RunEPN4base11MessagePump8DelegateE
14:10:04     INFO -  PID 24158 |   34:     0x7f556c4ed55b - _ZN11MessageLoop3RunEv
14:10:04     INFO -  PID 24158 |   35:     0x7f556e185618 - _ZN14nsBaseAppShell3RunEv
14:10:04     INFO -  PID 24158 |   36:     0x7f556f1ad82d - _ZN12nsAppStartup3RunEv
14:10:04     INFO -  PID 24158 |   37:     0x7f556f25c0df - _ZN7XREMain11XRE_mainRunEv
14:10:04     INFO -  PID 24158 |   38:     0x7f556f25c943 - _ZN7XREMain8XRE_mainEiPPcRKN7mozilla15BootstrapConfigE
14:10:04     INFO -  PID 24158 |   39:     0x7f556f25cf28 - _Z8XRE_mainiPPcRKN7mozilla15BootstrapConfigE
14:10:04     INFO -  PID 24158 |   40:     0x55a8fc31bcab - main
14:10:04     INFO -  PID 24158 |   41:     0x7f557b10882f - __libc_start_main

I think this is due to us creating a XUL label in the NAC for a XUL tooltip (bug 1461798). If I switch the XUL label created here https://searchfox.org/mozilla-central/rev/29aea2a2a3bd0f5e25ce0b60a76053fb25ba5149/dom/xul/XULTooltipElement.cpp#30 to XUL description (which won't have a CE attached), that makes the crash go away locally.

I'm thinking we should either stop using xul:label inside of tooltip NAC (possibly using xul:description or html:label instead), or somehow skip whatever Custom Element logic is causing the panic for content in NAC.

Brian Grinstead [:bgrins]

Assignee

Updated

•

Last year

Blocks: war-on-xbl

Emilio Cobos Álvarez (:emilio)

Comment 1

•

Last year

If you have a patch and a test-case I'd be interested in debugging this, but yeah, chances are that we're running the CE reactions after sync-styling the anonymous content.

We should just forbid any kind of JS-running during frame construction fwiw... :(

Emilio Cobos Álvarez (:emilio)

Comment 2

•

Last year

I bet removing this would make the crash go away as well:

  https://hg.mozilla.org/try/rev/f30aa7b51e315ae09231f1e4a333546e332a12d5#l4.182

Brian Grinstead [:bgrins]

Assignee

Comment 3

•

Last year

Attached file Bug 1497601 - Test case for crash with empty Custom Element on label (obsolete) — Details

Run with `./mach talos-test -a tresize`

Brian Grinstead [:bgrins]

Assignee

Comment 4

•

Last year

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)
> I bet removing this would make the crash go away as well:
> 
>  
> https://hg.mozilla.org/try/rev/f30aa7b51e315ae09231f1e4a333546e332a12d5#l4.
> 182

That's what I thought too - but I just uploaded a version that registers an empty CE class and it reproduces in `./mach talos-test -a tresize` (the first time the window opens it succeeds on my computer, and the crash happens on the second time).

Brian Grinstead [:bgrins]

Assignee

Comment 5

•

Last year

(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)
> If you have a patch and a test-case I'd be interested in debugging this, but
> yeah, chances are that we're running the CE reactions after sync-styling the
> anonymous content.
> 
> We should just forbid any kind of JS-running during frame construction
> fwiw... :(

Would there be some way to either ignore CE reactions entirely inside of NAC, or at least assert when they are happening so we don't accidentally introduce CE into NAC?

Olli Pettay [:smaug]

Comment 6

•

Last year

hmm, I thought we explicitly disabled CEs in NAC. Was that check removed at some point.

Emilio Cobos Álvarez (:emilio)

Comment 7

•

Last year

(In reply to Olli Pettay [:smaug] from comment #6)
> hmm, I thought we explicitly disabled CEs in NAC. Was that check removed at
> some point.

How does that work? By the time the element is created, it doesn't know whether it's in a NAC subtree, right?

Olli Pettay [:smaug]

Comment 8

•

Last year

sure, in sync case. But in async case that shouldn't an issue.

Brian Grinstead [:bgrins]

Assignee

Comment 9

•

Last year

Attached file Bug 1497601 - Use a xul:description instead of a xul:label for tooltips;r=smaug — Details

We'd like to be able to implement label features with a Custom Element, and we
don't want to run CE reactions inside of the NAC.

Brian Grinstead [:bgrins]

Assignee

Comment 10

•

Last year

I pushed up a patch to start using xul:description inside of tooltip. It seems to work fine from some manual testing, but are there any cases where we rely on xul:label layout features in tooltips?

Brian Grinstead [:bgrins]

Assignee

Comment 11

•

Last year

If we want to go this route, I guess we should also update all the <label class="tooltip-label"> elements in XUL docs to be <description class="tooltip-label">: https://searchfox.org/mozilla-central/search?q=tooltip-label&path=

Neil Deakin

Updated

•

Last year

Priority: -- → P3

Emilio Cobos Álvarez (:emilio)

Comment 12

•

Last year

I took a look at this. We're creating the tooltip in:

  chrome://global/content/backgroundPageThumbs.xhtml

GetEntryGlobal() returns null in:

  https://searchfox.org/mozilla-central/rev/80ac71c1c54af788b32e851192dfd2de2ec18e18/dom/base/nsContentUtils.cpp#10082

It's not a XUL document, so we don't grab the scope object.

Since there's no global the tooltip construction fails, we bail out, and end up trying to construct frames for the unstyled nodes that we had already appended before... So that's basically bug 1482421.

Emilio Cobos Álvarez (:emilio)

Comment 13

•

Last year

Note that the label we're creating is:

  https://searchfox.org/mozilla-central/rev/65f9687eb192f8317b4e02b0b791932eff6237cc/dom/xul/XULTooltipElement.cpp#37

From the following stack:

#0  0x00007f08c7973e77 in nsContentUtils::NewXULOrHTMLElement(mozilla::dom::Element**, mozilla::dom::NodeInfo*, mozilla::dom::FromParser, nsAtom*, mozilla::dom::CustomElementDefinition*) (aResult=0x7ffdb805d208, aNodeInfo=0x7f089ba83380, aFromParser=mozilla::dom::NOT_FROM_PARSER, aIsAtom=0x0, aDefinition=0x0) at /home/emilio/src/moz/gecko-2/dom/base/nsContentUtils.cpp:10091
#1  0x00007f08ca28666c in NS_NewXULElement(mozilla::dom::Element**, already_AddRefed<mozilla::dom::NodeInfo>&&, mozilla::dom::FromParser, nsAtom*, mozilla::dom::CustomElementDefinition*) (aResult=0x7ffdb805d208, aNodeInfo=..., aFromParser=mozilla::dom::NOT_FROM_PARSER, aIsAtom=0x0, aDefinition=0x0) at /home/emilio/src/moz/gecko-2/dom/xul/nsXULElement.cpp:283
#2  0x00007f08ca286322 in mozilla::dom::XULTooltipElement::Init() (this=0x7f08a4f8c670) at /home/emilio/src/moz/gecko-2/dom/xul/XULTooltipElement.cpp:35
#3  0x00007f08ca286164 in mozilla::dom::NS_NewXULTooltipElement(already_AddRefed<mozilla::dom::NodeInfo>&&) (aNodeInfo=...) at /home/emilio/src/moz/gecko-2/dom/xul/XULTooltipElement.cpp:20
#4  0x00007f08ca28f202 in nsXULElement::Construct(already_AddRefed<mozilla::dom::NodeInfo>&&) (aNodeInfo=...) at /home/emilio/src/moz/gecko-2/dom/xul/nsXULElement.cpp:161
#5  0x00007f08c79744bb in nsContentUtils::NewXULOrHTMLElement(mozilla::dom::Element**, mozilla::dom::NodeInfo*, mozilla::dom::FromParser, nsAtom*, mozilla::dom::CustomElementDefinition*) (aResult=0x7f089ba81170, aNodeInfo=0x7f089ba83300, aFromParser=mozilla::dom::NOT_FROM_PARSER, aIsAtom=0x0, aDefinition=0x0) at /home/emilio/src/moz/gecko-2/dom/base/nsContentUtils.cpp:10166
#6  0x00007f08ca28666c in NS_NewXULElement(mozilla::dom::Element**, already_AddRefed<mozilla::dom::NodeInfo>&&, mozilla::dom::FromParser, nsAtom*, mozilla::dom::CustomElementDefinition*) (aResult=0x7f089ba81170, aNodeInfo=..., aFromParser=mozilla::dom::NOT_FROM_PARSER, aIsAtom=0x0, aDefinition=0x0) at /home/emilio/src/moz/gecko-2/dom/xul/nsXULElement.cpp:283
#7  0x00007f08ca9bb221 in nsCanvasFrame::CreateAnonymousContent(nsTArray<nsIAnonymousContentCreator::ContentInfo>&) (this=0x7f089ba810b0, aElements=nsTArray<nsIAnonymousContentCreator::ContentInfo> & = {...}) at /home/emilio/src/moz/gecko-2/layout/generic/nsCanvasFrame.cpp:178
#8  0x00007f08ca8c0ffa in nsCSSFrameConstructor::GetAnonymousContent(nsIContent*, nsIFrame*, nsTArray<nsIAnonymousContentCreator::ContentInfo>&) (this=0x7f089bd12300, aParent=0x7f089ba24550, aParentFrame=0x7f089ba810b0, aContent=nsTArray<nsIAnonymousContentCreator::ContentInfo> & = {...}) at /home/emilio/src/moz/gecko-2/layout/base/nsCSSFrameConstructor.cpp:4053
#9  0x00007f08ca8c03e8 in nsCSSFrameConstructor::ConstructAnonymousContentForCanvas(nsFrameConstructorState&, nsIFrame*, nsIContent*) (this=0x7f089bd12300, aState=..., aFrame=0x7f089ba810b0, aDocElement=0x7f089ba24550) at /home/emilio/src/moz/gecko-2/layout/base/nsCSSFrameConstructor.cpp:2895
#10 0x00007f08ca8bf312 in nsCSSFrameConstructor::ConstructDocElementFrame(mozilla::dom::Element*, nsILayoutHistoryState*) (this=0x7f089bd12300, aDocElement=0x7f089ba24550, aFrameState=0x0)
    at /home/emilio/src/moz/gecko-2/layout/base/nsCSSFrameConstructor.cpp:2633
#11 0x00007f08ca8cddd4 in nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind) (this=0x7f089bd12300, aStartChild=0x7f089ba24550, aEndChild=0x0, aFrameState=0x0, aInsertionKind=nsCSSFrameConstructor::InsertionKind::Sync) at /home/emilio/src/moz/gecko-2/layout/base/nsCSSFrameConstructor.cpp:7321
#12 0x00007f08ca8cd26b in nsCSSFrameConstructor::ContentInserted(nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind) (this=0x7f089bd12300, aChild=0x7f089ba24550, aFrameState=0x0, aInsertionKind=nsCSSFrameConstructor::InsertionKind::Sync) at /home/emilio/src/moz/gecko-2/layout/base/nsCSSFrameConstructor.cpp:7231
#13 0x00007f08ca85fc39 in mozilla::PresShell::Initialize() (this=0x7f089ba7e000) at /home/emilio/src/moz/gecko-2/layout/base/PresShell.cpp:1808
#14 0x00007f08c7ba775a in nsContentSink::StartLayout(bool) (this=0x7f089ba6d000, aIgnorePendingSheets=false) at /home/emilio/src/moz/gecko-2/dom/base/nsContentSink.cpp:1276
#15 0x00007f08ca1e78af in nsXMLContentSink::MaybeStartLayout(bool) (this=0x7f089ba6d000, aIgnorePendingSheets=false) at /home/emilio/src/moz/gecko-2/dom/xml/nsXMLContentSink.cpp:932

Brian Grinstead [:bgrins]

Assignee

Comment 14

•

Last year

Is there a way to somehow completely ignore CE definitions / reactions from within NAC? We don't actually need any of the Custom Element behavior in this case (or presumably in any case since we don't want them to happen).

Emilio Cobos Álvarez (:emilio)

Comment 15

•

Last year

Hmm, it's somewhat hard without tagging all the callers around or something... By the time we're creating that element we still don't know we're going to be NAC.

Phabricator Automation

Updated

•

Last year

Attachment #9015948 - Attachment description: Bug 1497601 - Use a xul:description instead of a xul:label in tooltip NAC;r=smaug → Bug 1497601 - Use a xul:description instead of a xul:label for tooltips;r=smaug

Brian Grinstead [:bgrins]

Assignee

Comment 16

•

Last year

I didn't see anywhere we used label features, so I pushed up a version that uses description everywhere.

Brian Grinstead [:bgrins]

Assignee

Updated

•

Last year

Assignee: nobody → bgrinstead

Status: NEW → ASSIGNED

Pulsebot

Comment 17

•

Last year

Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f2e35ed6a692
Use a xul:description instead of a xul:label for tooltips;r=smaug

Cosmin Sabou [:CosminS]

Comment 18

•

Last year

bugherder

https://hg.mozilla.org/mozilla-central/rev/f2e35ed6a692

Status: ASSIGNED → RESOLVED

Closed: Last year

status-firefox64: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla64

Andrei Vaida [:avaida] - PTO, returning Oct 29

Updated

•

Last year

Flags: qe-verify-

Phabricator Automation

Updated

•

11 days ago

Attachment #9015632 - Attachment is obsolete: true

Bug 1497601 - Test case for crash with empty Custom Element on label Last year Brian Grinstead [:bgrins] 46 bytes, text/x-phabricator-request		Details \| Review
Bug 1497601 - Use a xul:description instead of a xul:label for tooltips;r=smaug Last year Brian Grinstead [:bgrins] 46 bytes, text/x-phabricator-request		Details \| Review