Bug 177071 – [WK1] Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Per Arne Vollan]

The following layout test is crashing on Windows:

fast/events/beforeunload-dom-manipulation-crash.html

[Per Arne Vollan]

FAULTING_IP: 
WebKit!WebCore::HistoryController::updateForRedirectWithLockedBackForwardList+1a [c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\loader\historycontroller.cpp @ 418]
67318dda 8bb084050000    mov     esi,dword ptr [eax+584h]

EXCEPTION_RECORD:  ffffffffffffffff -- (.exr 0xffffffffffffffff)
.exr 0xffffffffffffffff
ExceptionAddress: 0000000067318dda (WebKit!PAL::SessionID::isEphemeral)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 0000000000000000
   Parameter[1]: 0000000000000584
Attempt to read from address 0000000000000584

STACK_TEXT:  
0017e1fc 66f5b747 08f27f78 00000000 090128d0 WebKit!WebCore::HistoryController::updateForRedirectWithLockedBackForwardList+0x1a
0017e210 66f59a37 00000000 08f51e60 090128d0 WebKit!WebCore::FrameLoader::transitionToCommitted+0x177
0017e58c 66f4f78c 00000003 090128d0 0017e5c0 WebKit!WebCore::FrameLoader::commitProvisionalLoad+0x167
0017e59c 66f508a1 08ffeac8 00000003 00000003 WebKit!WebCore::DocumentLoader::commitLoad+0x3c
0017e5c0 66f502a1 08ffeac8 00000003 0017e60c WebKit!WebCore::DocumentLoader::dataReceived+0x91
0017e5d0 674dfc7a 058c0968 08ffeac8 00000003 WebKit!WebCore::DocumentLoader::dataReceived+0x11
0017e60c 674df6b7 08ffeac8 00000003 0017e684 WebKit!WebCore::CachedRawResource::notifyClientsDataWasReceived+0x4a
0017e640 66f67123 0900e620 08f8a0ac 08ffead8 WebKit!WebCore::CachedRawResource::addDataBuffer+0xa7
0017e658 66f66a48 00000000 00000000 00000000 WebKit!WebCore::SubresourceLoader::didReceiveDataOrBuffer+0x93
0017e67c 66f621b0 0900e620 00000003 00000000 WebKit!WebCore::SubresourceLoader::didReceiveBuffer+0x28
0017e9f8 66f623da 00328c01 0900e558 6af83044 WebKit!<lambda_1b5c0f757fd788f55dc65d669fddc406>::operator()<std::optional<WebCore::DataURLDecoder::Result> >+0x260
0017ea18 674eb879 73fae801 00000000 00000000 WebKit!WTF::Function<void __cdecl(std::optional<WebCore::DataURLDecoder::Result>)>::CallableWrapper<<lambda_1b5c0f757fd788f55dc65d669fddc406> >::call+0x5a
0017ea38 674eb796 0017ea01 00000000 00000000 WebKit!WTF::Function<void __cdecl(std::optional<WebCore::DataURLDecoder::Result>)>::operator()+0x59
0017ea54 6aef93b7 00000000 00000000 6af2a030 WebKit!<lambda_6ed09d88622e374e515957791c47d845>::operator()+0x66
0017ea84 6af2a043 0017eab8 76c462fa 991e090c WTF!WTF::dispatchFunctionsFromMainThread+0xe7
0017ea8c 76c462fa 991e090c 0000c15c 00000000 WTF!WTF::ThreadingWindowWndProc+0x13
WARNING: Stack unwind information not available. Following frames may be wrong.
0017eab8 76c46d3a 6af2a030 991e090c 0000c15c USER32!gapfnScSendMessage+0x332
0017eb30 76c477c4 00000000 6af2a030 991e090c USER32!GetThreadDesktop+0xd7
0017eb90 76c4788a 6af2a030 00000000 0017ecc8 USER32!CharPrevW+0x138
0017eba0 6b69dcee 0017ec2c 00722300 009fa990 USER32!DispatchMessageW+0xf
0017ecc8 6b69997a 0017ece0 012f6df0 00722300 DumpRenderTreeLib!runTest+0x6de
0017f540 6b699b4e 00000002 00722300 0017f818 DumpRenderTreeLib!main+0x43a
0017f550 012e16c9 00000002 00722300 012f6dec DumpRenderTreeLib!dllLauncherEntryPoint+0xe
0017f818 012e32ba 00000002 00722300 00725b10 DumpRenderTree!main+0x469
0017f864 7516336a 7efde000 0017f8b0 772e9902 DumpRenderTree!__scrt_common_main_seh+0xff
0017f870 772e9902 7efde000 532031e6 00000000 KERNEL32!BaseThreadInitThunk+0x12
0017f8b0 772e98d5 012e3337 7efde000 00000000 ntdll_772b0000!RtlInitializeExceptionChain+0x63
0017f8c8 00000000 012e3337 7efde000 00000000 ntdll_772b0000!RtlInitializeExceptionChain+0x36

[Per Arne Vollan]

Created attachment 321101 [details]
Patch

[Brent Fulgham]

Comment on attachment 321101 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321101&action=review

Thank you for fixing this. Please make the two adjustments I requested so we have a complete fix.

> Source/WebCore/ChangeLog:9
> +        accessing the page. 

Thank you! I have an partial patch for this that touches one or two other things. Let me annotate your patch.

Can you please make the same fix in "HistoryController::updateForSameDocumentNavigation". In that method, it checks the page for "usesEphemeralSession" before it checks for a nullptr page. THEN it checks for nullptr!

In the same vein, can you please move the ASSERT(m_frame.page()) in "HistoryController::replaceState" so that it is called before the dereference of page to call 'usesEphemeralSession'?

[Per Arne Vollan]

Created attachment 321103 [details]
Patch

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #3)
> Comment on attachment 321101 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321101&action=review
> 
> Thank you for fixing this. Please make the two adjustments I requested so we
> have a complete fix.
> 
> > Source/WebCore/ChangeLog:9
> > +        accessing the page. 
> 
> Thank you! I have an partial patch for this that touches one or two other
> things. Let me annotate your patch.
> 
> Can you please make the same fix in
> "HistoryController::updateForSameDocumentNavigation". In that method, it
> checks the page for "usesEphemeralSession" before it checks for a nullptr
> page. THEN it checks for nullptr!
> 
> In the same vein, can you please move the ASSERT(m_frame.page()) in
> "HistoryController::replaceState" so that it is called before the
> dereference of page to call 'usesEphemeralSession'?

Thanks for reviewing! I have updated the patch.

[Brent Fulgham]

Comment on attachment 321103 [details]
Patch

Looks great! r=me.

[Brent Fulgham]

*** Bug 177020 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 321103 [details]
Patch

Clearing flags on attachment: 321103

Committed r222163: <http://trac.webkit.org/changeset/222163>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brent Fulgham]

Note: This is also <rdar://problem/34465570>.

[Radar WebKit Bug Importer]

<rdar://problem/34693215>

[Arunprasad]

Still "fast/events/beforeunload-dom-manipulation-crash.html" crashes on few platforms,

See AppleWin test result,

https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/3613/steps/layout-test/logs/stdio