541050 - [HTML5][Patch] docshell/test/test_bug404548.html fails due to pagehide event handler not executing

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P1)

Description

[Henri Sivonen (:hsivonen)]

docshell/test/test_bug404548.html fails. On the face of things, time timeout probably doesn't allow for enough event ping-ponging between the main thread and the parser thread.

Comment 1

[Henri Sivonen (:hsivonen)]

That was a bad guess. It seems the onpagehide handlers don't run at all.

Comment 2

[Henri Sivonen (:hsivonen)]

It seems that the event dispatching machinery is run for pagehide. Still, the code in the onpagehide attribute doesn't appear to run.

Comment 3

[Henri Sivonen (:hsivonen)]

http://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20html%3E%0A%3Ciframe%20src%3D%22data%3Atext%2Fhtml%2C%3Cbody%20onpagehide%3D%2522parent.w%28%27hide%27%29%2522%3E%3Ca%20href%3D%27http%3A%2F%2Fwww.example.com%2F%27%3ELink%3C%2Fa%3E%22%3E%3C%2Fiframe%3E Shows that in general, the pagehide handler works with the HTML5 parser.

Comment 4

[Henri Sivonen (:hsivonen)]

With the old parser, the stack to the pagehide handler JS boundary is
#32	0x0007c4db in JS_CallFunctionValue at jsapi.cpp:5120
#33	0x1376cc78 in nsJSContext::CallEventHandler at nsJSEnvironment.cpp:2168
#34	0x137ef0c7 in nsJSEventListener::HandleEvent at nsJSEventListener.cpp:228

#35	0x13570d4b in nsEventListenerManager::HandleEventSubType at nsEventListenerManager.cpp:1056
#36	0x13571216 in nsEventListenerManager::HandleEvent at nsEventListenerManager.cpp:1172
#37	0x135a0295 in nsEventTargetChainItem::HandleEvent at nsEventDispatcher.cpp:200

#38	0x1359ebcc in nsEventTargetChainItem::HandleEventTargetChain at nsEventDispatcher.cpp:325

#39	0x1359f7e7 in nsEventDispatcher::Dispatch at nsEventDispatcher.cpp:600
#40	0x1359fb61 in nsEventDispatcher::DispatchDOMEvent at nsEventDispatcher.cpp:663
#41	0x134a2ee1 in nsDocument::DispatchPageTransition at nsDocument.cpp:7281
#42	0x134a3106 in nsDocument::OnPageHide at nsDocument.cpp:7389
#43	0x131a0e34 in DocumentViewerImpl::PageHide at nsDocumentViewer.cpp:1267
#44	0x14ca56dc in nsDocShell::FirePageHideNotification at nsDocShell.cpp:1449
#45	0x14ca57b6 in nsDocShell::FirePageHideNotification at nsDocShell.cpp:1461
#46	0x14c97424 in nsDocShell::CreateContentViewer at nsDocShell.cpp:6956
#47	0x14cc65ad in nsDSURIContentListener::DoContent at nsDSURIContentListener.cpp:138
#48	0x14cd0a80 in nsDocumentOpenInfo::TryContentListener at nsURILoader.cpp:736
#49	0x14cd152b in nsDocumentOpenInfo::DispatchContent at nsURILoader.cpp:434
#50	0x14cd23d0 in nsDocumentOpenInfo::OnStartRequest at nsURILoader.cpp:280
#51	0x12141532 in nsBaseChannel::OnStartRequest at nsBaseChannel.cpp:665
#52	0x1215439c in nsInputStreamPump::OnStateStart at nsInputStreamPump.cpp:439
#53	0x1215513c in nsInputStreamPump::OnInputStreamReady at nsInputStreamPump.cpp:395
#54	0x003930d4 in nsInputStreamReadyEvent::Run at nsStreamUtils.cpp:112
#55	0x003c7fe2 in nsThread::ProcessNextEvent at nsThread.cpp:527
#56	0x0034b0c7 in NS_ProcessPendingEvents_P at nsThreadUtils.cpp:200
#57	0x12fdc529 in nsBaseAppShell::NativeEventCallback at nsBaseAppShell.cpp:125
#58	0x12f8c00f in nsAppShell::ProcessGeckoEvents at nsAppShell.mm:510
#59	0x956e38cb in __CFRunLoopDoSources0
#60	0x956e138f in __CFRunLoopRun
#61	0x956e0864 in CFRunLoopRunSpecific
#62	0x956e0691 in CFRunLoopRunInMode
#63	0x990f2f0c in RunCurrentEventLoopInMode
#64	0x990f2cc3 in ReceiveNextEventCommon
#65	0x990f2b48 in BlockUntilNextEventMatchingListInMode
#66	0x9146aac5 in _DPSNextEvent
#67	0x9146a306 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
#68	0x9142c49f in -[NSApplication run]
#69	0x12f8a671 in nsAppShell::Run at nsAppShell.mm:863
#70	0x1527125a in nsAppStartup::Run at nsAppStartup.cpp:182
#71	0x00011cf4 in XRE_main at nsAppRunner.cpp:3476
#72	0x00002629 in main at nsBrowserApp.cpp:158

The stack frame #38	0x1359ebcc in nsEventTargetChainItem::HandleEventTargetChain at nsEventDispatcher.cpp:325 is the target phase call to HandleEvent().

With the HTML5 parser, the target phase call to HandleEvent() never comes through to #35	0x13570d4b in nsEventListenerManager::HandleEventSubType at nsEventListenerManager.cpp:1056.

The reason seems to be that mTarget->GetListenerManager(PR_FALSE); returns null in #37	0x135a0295 in nsEventTargetChainItem::HandleEvent at nsEventDispatcher.cpp:200.

smaug, any guidance on what might be the cause and how to fix on Mochitest, when pagehide does work on the Web (comment #3).

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Does html5 parser somehow cause changing the inner window before pagehide?
That new window wouldn't necessarily have event listener manager.

Comment 6

[Henri Sivonen (:hsivonen)]

I checked the number and order of
 * inner window reuses
 * inner window creations
 * pagehide fires
 * inner window nulling from dtor

With and without the HTML5 parser, the sequence is the same except one cycle collection-initiated dtor runs later with the HTML5 parser.

At least so far, it seems that the inner window isn't the problem. (Or if it is, it is in a particularly subtle way.)

Comment 7

[Henri Sivonen (:hsivonen)]

I added a bunch of printfs around. I was hoping to find the same number of ELM retrievals from the global window in both cases but with at least one more null return in the HTML5 case.

I found that the total number of ELM retrievals from the window was greater with the HTML5 parser, so I guess the exact numbers isn't interesting. This is due to mouse events poking the ELM getter on global window, right?

A more interesting printf finding was that the onpagehide listener comes through to nsEventListenerManager::AddScriptEventListener with the old parser but not with the HTML5 parser.

Comment 8

[Henri Sivonen (:hsivonen)]

Attachment: Add greater-than signs to body tags in data: URLs

So it turns out that I was debugging totally the wrong thing. What happened here was that body tags in the data URLs were being dropped as a new (even if questionable) HTML5 security measure (on the principle of not running code made malformed by a premature EOF) and the body that got through was the implicit body.

It turns out that in a data: URL inside another tag, it's not obvious that the > character at the end of the data: URL is missing unless you are alertly looking for that kind of problem...

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 424766 [details] [diff] [review]
Add greater-than signs to body tags in data: URLs

r=me, but drop the extra '>' at the end of the second <iframe>?  It looks like it just mistakenly had "> where it meant >"

Comment 10

[Henri Sivonen (:hsivonen)]

http://hg.mozilla.org/mozilla-central/rev/472fdc09a0aa

Landed with s/>>/>/.