1392891 - nsContentUtils::GenerateStateKey() creates two nsContentLists for an input element added to a document unless if it has autocomplete=off

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[(no longer active)]

Here we create these two objects:

https://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/dom/base/nsContentUtils.cpp#3039

Because of bug 1377560, we keep these mutation observers alive forever.  :-(

Comment 1

[(no longer active)]

Callstack:

#0  0x00007f0902d99477 in nsContentList::nsContentList(nsINode*, int, nsIAtom*, nsIAtom*, bool) (this=0x7f08dc440b00, aRootNode=0x7f08dc417000, aMatchNameSpaceId=3, aHTMLMatchAtom=0x7f08fbeb0e80, aXMLMatchAtom=0x7f08fbeb0e80, aDeep=<error reading variable: access outside bounds of object referenced via synthetic pointer>) at /home/ehsan/moz/src.1347035/dom/base/nsContentList.cpp:426
#1  0x00007f0903a252b7 in non-virtual thunk to nsHTMLDocument::GetForms() ()
    at /home/ehsan/moz/src.1347035/dom/html/nsHTMLDocument.cpp:2351
#2  0x00007f0902ca3960 in nsContentUtils::GenerateStateKey(nsIContent*, nsIDocument const*, nsACString&) (aContent=0x7f08db2bc240, aDocument=<optimized out>, aKey="0")
    at /home/ehsan/moz/src.1347035/dom/base/nsContentUtils.cpp:3039
#3  0x00007f0903a1729f in nsGenericHTMLFormElementWithState::GenerateStateKey() (this=0x7f08db2bc240) at /home/ehsan/moz/src.1347035/dom/html/nsGenericHTMLElement.cpp:2839
#4  0x00007f09039a2cc5 in mozilla::dom::HTMLInputElement::DoneCreatingElement() (this=
    0x7f08db2bc240) at /home/ehsan/moz/src.1347035/dom/html/HTMLInputElement.cpp:6532
#5  0x00007f09028ec415 in nsHtml5TreeOperation::DoneCreatingElement(nsIContent*) (aNode=0x7f08dc440b00) at /home/ehsan/moz/src.1347035/parser/html/nsHtml5TreeOperation.cpp:727
#6  0x00007f09028ec415 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*) (this=0x7f08f1132788, aBuilder=
    0x7f08dc612000, aScriptElement=0x7ffc90066710, aInterrupted=0x7ffc90066727)
    at /home/ehsan/moz/src.1347035/parser/html/nsHtml5TreeOperation.cpp:939
#7  0x00007f09028ebbc1 in nsHtml5TreeOpExecutor::RunFlushLoop() (this=0x7f08dc612000)
    at /home/ehsan/moz/src.1347035/parser/html/nsHtml5TreeOpExecutor.cpp:465
#8  0x00007f09028fcead in nsHtml5ExecutorFlusher::Run() (this=<optimized out>)
    at /home/ehsan/moz/src.1347035/parser/html/nsHtml5StreamParser.cpp:128
#9  0x00007f0901d9f8f5 in mozilla::SchedulerGroup::Runnable::Run() (this=0x7f08dc608340)
    at /home/ehsan/moz/src.1347035/xpcom/threads/SchedulerGroup.cpp:387
#10 0x00007f0901dad3a8 in nsThread::ProcessNextEvent(bool, bool*) (this=0x7f08fbf1e8a0, aMayWait=<optimized out>, aResult=<optimized out>)
    at /home/ehsan/moz/src.1347035/xpcom/threads/nsThread.cpp:1040
#11 0x00007f0901dae73f in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x7f08dc440b00, aMayWait=<error reading variable: access outside bounds of object referenced via synthetic pointer>)
    at /home/ehsan/moz/src.1347035/xpcom/threads/nsThreadUtils.cpp:521
#12 0x00007f0902298ddd in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (this=0x7f09114d3c90, aDelegate=0x7ffc90066e60) at /home/ehsan/moz/src.1347035/ipc/glue/MessagePump.cpp:125
#13 0x00007f0902236c5b in MessageLoop::RunInternal() (this=0x7f08dc440b00)
    at /home/ehsan/moz/src.1347035/ipc/chromium/src/base/message_loop.cc:326
#14 0x00007f0902236c5b in MessageLoop::RunHandler() (this=<optimized out>)
    at /home/ehsan/moz/src.1347035/ipc/chromium/src/base/message_loop.cc:319
#15 0x00007f0902236c5b in MessageLoop::Run() (this=0x7f08dc440b00)
    at /home/ehsan/moz/src.1347035/ipc/chromium/src/base/message_loop.cc:299
#16 0x00007f09040d26d9 in nsBaseAppShell::Run() (this=0x7f08f60d99e0)
    at /home/ehsan/moz/src.1347035/widget/nsBaseAppShell.cpp:158
#17 0x00007f09057bbb84 in XRE_RunAppShell() ()
    at /home/ehsan/moz/src.1347035/toolkit/xre/nsEmbedFunctions.cpp:865
#18 0x00007f0902236c5b in MessageLoop::RunInternal() (this=0x7f08dc440b00)
    at /home/ehsan/moz/src.1347035/ipc/chromium/src/base/message_loop.cc:326
#19 0x00007f0902236c5b in MessageLoop::RunHandler() (this=<optimized out>)
    at /home/ehsan/moz/src.1347035/ipc/chromium/src/base/message_loop.cc:319
#20 0x00007f0902236c5b in MessageLoop::Run() (this=0x7f08dc440b00)
    at /home/ehsan/moz/src.1347035/ipc/chromium/src/base/message_loop.cc:299

Comment 2

[(no longer active)]

Attachment: Part 1: Make it possible to create nsContentList objects that don't observe DOM mutations

Comment 3

[(no longer active)]

Attachment: Part 2: Add an API to return nsHTMLDocument::mForms/mFormControls without creating a new nsContentList

Comment 4

[(no longer active)]

Attachment: Part 3: Avoid creating live nsContentList objects in nsContentUtils::GenerateStateKey()

Comment 5

[(no longer active)]

Attachment: Part 4: Remove nsIHTMLDocument::GetFormControls()

Comment 6

[(no longer active)]

Attachment: Part 5: Devirtualize nsHTMLDocument::GetForms()

Comment 7

[Olli Pettay [:smaug]]

Comment on attachment 8901530 [details] [diff] [review]
Part 2: Add an API to return nsHTMLDocument::mForms/mFormControls without creating a new nsContentList

Could you make the methods non-virtual and just do a static_cast to nsHTMLDocument when needed. With that, r+.

Comment 8

[Olli Pettay [:smaug]]

Comment on attachment 8901531 [details] [diff] [review]
Part 3: Avoid creating live nsContentList objects in nsContentUtils::GenerateStateKey()

> nsHTMLDocument::GetForms()
> {
>   if (!mForms) {
>+    // Please keep this in sync with nsContentUtils::GenerateStateKey().
Add similar comment for mFormControls case too.

Comment 9

[Olli Pettay [:smaug]]

Comment on attachment 8901532 [details] [diff] [review]
Part 4: Remove nsIHTMLDocument::GetFormControls()

oh, this is the reason you didn't add the comment. ok good.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/46b74d58dc6f
Part 1: Make it possible to create nsContentList objects that don't observe DOM mutations; r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b0603208430
Part 2: Add an API to return nsHTMLDocument::mForms/mFormControls without creating a new nsContentList; r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/81d0287a9a8c
Part 3: Avoid creating live nsContentList objects in nsContentUtils::GenerateStateKey(); r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0512200fe86
Part 4: Remove nsIHTMLDocument::GetFormControls(); r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac41abda3dd9
Part 5: Devirtualize nsHTMLDocument::GetForms(); r=smaug

Comment 11

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/46b74d58dc6f
https://hg.mozilla.org/mozilla-central/rev/9b0603208430
https://hg.mozilla.org/mozilla-central/rev/81d0287a9a8c
https://hg.mozilla.org/mozilla-central/rev/e0512200fe86
https://hg.mozilla.org/mozilla-central/rev/ac41abda3dd9