Firefox crashes using `closeTab` on an document with a frame and `readystatechange` listener

RESOLVED WORKSFORME

Status

()

--
critical
RESOLVED WORKSFORME
6 years ago
6 years ago

People

(Reporter: zer0, Assigned: smaug)

Tracking

({crash, reproducible})

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Firefox crashes systematically using the following code:

    const { getActiveTab, getTabContentWindow, closeTab } = require("sdk/tabs/utils")
    const { getMostRecentBrowserWindow } = require("sdk/window/utils");

    const HTML = "<html>\
      <body>\
        <div>foo</div>\
        <div>and</div>\
        <textarea>noodles</textarea>\
      </body>\
    </html>";

    const URL = "data:text/html;charset=utf-8," + encodeURIComponent(HTML);

    const FRAME_HTML = "<iframe src='" + URL + "'><iframe>";
    const FRAME_URL = "data:text/html;charset=utf-8," + encodeURIComponent(FRAME_HTML);

    function close() {
      closeTab(getActiveTab(getMostRecentBrowserWindow()));
    }

    require("sdk/tabs").open({
      url: FRAME_URL,
      onReady: function() {
        let window = getTabContentWindow(getActiveTab(getMostRecentBrowserWindow()));
        let { document } = window.frames[0];

        if (document.readyState === "complete")
          close();

        document.addEventListener("readystatechange", function readystate() {
          if (this.readyState === "complete") {
            document.removeEventListener("readystatechange", readystate);

            close();
          }
        });
      }
    })

The culprit seems to be the `readystatechange`, if I just close the tab without adding that listener it works. Plus, if I close the tab after *any* kind of delay, it seems work too.
(Reporter)

Comment 1

6 years ago
Created attachment 712410 [details]
An add-on that adds a widget that executes the code described in the bug.
(Reporter)

Comment 2

6 years ago
Just to be more precise, as I said if the `close` call is executed with any sort of delay it works. So basically, replacing the `close` call inside the listener with:

    require("sdk/timers").setTimeout(close, 0);

Doesn't crash Firefox.
in nsDocumentViewer::LoadComplete mDocument is null

stack:

 	xul.dll!nsCOMPtr<nsIDocument>::operator->()  Line 783 + 0x23 bytes	C++
>	xul.dll!nsDocumentViewer::LoadComplete(tag_nsresult aStatus=NS_OK)  Line 1032 + 0xb bytes	C++
 	xul.dll!nsDocShell::EndPageLoad(nsIWebProgress * aProgress=0x191dbe0c, nsIChannel * aChannel=0x12bb06b8, tag_nsresult aStatus=NS_OK)  Line 6648	C++
 	xul.dll!nsDocShell::OnStateChange(nsIWebProgress * aProgress=0x191dbe0c, nsIRequest * aRequest=0x12bb06b8, unsigned int aStateFlags=0x00020010, tag_nsresult aStatus=NS_OK)  Line 6476	C++
 	xul.dll!nsDocLoader::DoFireOnStateChange(nsIWebProgress * const aProgress=0x191dbe0c, nsIRequest * const aRequest=0x12bb06b8, int & aStateFlags=0x00020010, tag_nsresult aStatus=NS_OK)  Line 1306	C++
 	xul.dll!nsDocLoader::doStopDocumentLoad(nsIRequest * request=0x12bb06b8, tag_nsresult aStatus=NS_OK)  Line 886	C++
 	xul.dll!nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout=true)  Line 777	C++
 	xul.dll!nsDocLoader::OnStopRequest(nsIRequest * aRequest=0x136f7350, nsISupports * aCtxt=0x00000000, tag_nsresult aStatus=NS_OK)  Line 662	C++
 	xul.dll!nsLoadGroup::RemoveRequest(nsIRequest * request=0x136f7350, nsISupports * ctxt=0x00000000, tag_nsresult aStatus=NS_OK)  Line 676 + 0x31 bytes	C++
 	xul.dll!nsDocument::DoUnblockOnload()  Line 7635	C++
 	xul.dll!nsDocument::UnblockOnload(bool aFireSync=true)  Line 7562	C++
 	xul.dll!nsDocument::DispatchContentLoadedEvents()  Line 4421	C++
 	xul.dll!nsRunnableMethodImpl<void (__thiscall nsDocument::*)(void),1>::Run()  Line 368	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=false, bool * result=0x0017f2ff)  Line 627 + 0x19 bytes	C++
 	xul.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x0042c0c8, bool mayWait=false)  Line 238 + 0x17 bytes	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate=0x0028d430)  Line 82 + 0xe bytes	C++
 	xul.dll!MessageLoop::RunInternal()  Line 216	C++
 	xul.dll!MessageLoop::RunHandler()  Line 209	C++
 	xul.dll!MessageLoop::Run()  Line 183	C++
 	xul.dll!nsBaseAppShell::Run()  Line 165	C++
 	xul.dll!nsAppShell::Run()  Line 154 + 0x9 bytes	C++
 	xul.dll!nsAppStartup::Run()  Line 288 + 0x1c bytes	C++
 	xul.dll!XREMain::XRE_mainRun()  Line 3832 + 0x22 bytes	C++
 	xul.dll!XREMain::XRE_main(int argc=0x00000001, char * * argv=0x002865a0, const nsXREAppData * aAppData=0x0017f914)  Line 3899 + 0x8 bytes	C++
 	xul.dll!XRE_main(int argc=0x00000001, char * * argv=0x002865a0, const nsXREAppData * aAppData=0x0017f914, unsigned int aFlags=0x00000000)  Line 4102 + 0x17 bytes	C++
 	firefox.exe!do_main(int argc=0x00000001, char * * argv=0x002865a0, nsIFile * xreDirectory=0x0028c710)  Line 185 + 0x14 bytes	C++
Keywords: crash
Something is really fishy here, when I walk up on the stack to nsDocShell::EndPageLoad to this call: mContentViewer->LoadComplete(aStatus); according to my debugger mContentViewer is null... So within that call somehow mContentViewer gets nulled out.

Anyway, it looks like we are not protected against the rude case where the document complete callback closes the parent window of the document.
(Assignee)

Comment 5

6 years ago
Nothing fishy there ;) We're closing the window so we need to null check mDocument.
We do have null checks in some cases in the method, but looks like missing it at least right
after flush.
(Assignee)

Updated

6 years ago
Assignee: nobody → bugs

Updated

6 years ago
Severity: normal → critical
Crash Signature: [@ nsCOMPtr<nsIDocument>::operator->()]
Component: General → Layout
Product: Add-on SDK → Core
Keywords: reproducible
(Assignee)

Comment 6

6 years ago
This was fixed in another bug. Reporter, could you verify using Nightly?
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Resolution: FIXED → WORKSFORME
You need to log in before you can comment on or make changes to this bug.