Bug 156033

Summary: REGRESSION (r195605): ASSERTION FAILED: !NoEventDispatchAssertion::isEventDispatchForbidden() when pressing the back button on a page with a focused subframe
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, cdumez, kling, webkit-bug-importer
Priority: P2 Keywords: InRadar, Regression
Version: WebKit Local Build   
Hardware: All   
OS: All   
Bug Depends on: 153449    
Bug Blocks:    
Attachments:
Description Flags
back-from-page-with-focused-frame.html
none
Patch and Layout Test cdumez: review+

Description Daniel Bates 2016-03-30 12:29:49 PDT
Created attachment 275210 [details]
back-from-page-with-focused-frame.html

Using a debug build of WebKit, open the attached test, back-from-page-with-focused-iframe.html. Then WebKit will crash with an assertion failure:

ASSERTION FAILED: !NoEventDispatchAssertion::isEventDispatchForbidden()
/WebKitDev/OpenSource/Source/WebCore/dom/Document.cpp(4142) : void WebCore::Document::dispatchWindowEvent(WebCore::Event &, WebCore::EventTarget *)
1   0x110e443f0 WTFCrash
2   0x110e44419 WTFCrashWithSecurityImplication
3   0x11365f892 WebCore::Document::dispatchWindowEvent(WebCore::Event&, WebCore::EventTarget*)
4   0x113903dff WebCore::FocusController::setFocusedFrame(WTF::PassRefPtr<WebCore::Frame>)
5   0x113200b7c WebCore::CachedFrame::CachedFrame(WebCore::Frame&)
6   0x11320138d WebCore::CachedFrame::CachedFrame(WebCore::Frame&)
7   0x113202247 std::_Unique_if<WebCore::CachedFrame>::_Single_object std::make_unique<WebCore::CachedFrame, WebCore::Frame&>(WebCore::Frame&&&)
8   0x113200c40 WebCore::CachedFrame::CachedFrame(WebCore::Frame&)
9   0x11320138d WebCore::CachedFrame::CachedFrame(WebCore::Frame&)
10  0x11320c54a std::_Unique_if<WebCore::CachedFrame>::_Single_object std::make_unique<WebCore::CachedFrame, WebCore::MainFrame&>(WebCore::MainFrame&&&)
11  0x11320be73 WebCore::CachedPage::CachedPage(WebCore::Page&)
12  0x11320bebd WebCore::CachedPage::CachedPage(WebCore::Page&)
13  0x11498a927 std::_Unique_if<WebCore::CachedPage>::_Single_object std::make_unique<WebCore::CachedPage, WebCore::Page&>(WebCore::Page&&&)
14  0x114988353 WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*)
15  0x113a247db WebCore::FrameLoader::commitProvisionalLoad()
16  0x113a29072 WebCore::FrameLoader::loadProvisionalItemFromCachedPage()
17  0x113a22c98 WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool, WebCore::AllowNavigationToInvalidURL)
18  0x113a3136e WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>, WebCore::AllowNavigationToInvalidURL)::$_4::operator()(WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool) const
19  0x113a312f0 void std::__1::__invoke_void_return_wrapper<void>::__call<WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>, WebCore::AllowNavigationToInvalidURL)::$_4&, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool>(WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>, WebCore::AllowNavigationToInvalidURL)::$_4&&&, WebCore::ResourceRequest const&&&, WTF::PassRefPtr<WebCore::FormState>&&, bool&&)
20  0x113a311ec std::__1::__function::__func<WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>, WebCore::AllowNavigationToInvalidURL)::$_4, std::__1::allocator<WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>, WebCore::AllowNavigationToInvalidURL)::$_4>, void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>::operator()(WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>&&, bool&&)
21  0x114a152d7 std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>::operator()(WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool) const
22  0x114a148c9 WebCore::PolicyCallback::call(bool)
23  0x114a15f25 WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction)
24  0x114a1951e WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::$_1::operator()(WebCore::PolicyAction) const
25  0x114a194ef void std::__1::__invoke_void_return_wrapper<void>::__call<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::$_1&, WebCore::PolicyAction>(WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::$_1&&&, WebCore::PolicyAction&&)
26  0x114a1946c std::__1::__function::__func<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::$_1, std::__1::allocator<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)::$_1>, void (WebCore::PolicyAction)>::operator()(WebCore::PolicyAction&&)
27  0x10d13da2c std::__1::function<void (WebCore::PolicyAction)>::operator()(WebCore::PolicyAction) const
28  0x10d144201 WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(WebCore::NavigationAction const&, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::PolicyAction)>)
29  0x114a15bcd WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>)
30  0x113a22377 WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>, WebCore::AllowNavigationToInvalidURL)
31  0x113a1e7f5 WebCore::FrameLoader::loadDifferentDocumentItem(WebCore::HistoryItem&, WebCore::FrameLoadType, WebCore::FrameLoader::FormSubmissionCacheLoadPolicy)
Comment 1 Radar WebKit Bug Importer 2016-03-30 12:30:49 PDT
<rdar://problem/25446561>
Comment 2 Daniel Bates 2016-03-30 12:38:03 PDT
Created attachment 275211 [details]
Patch and Layout Test
Comment 3 Chris Dumez 2016-03-30 12:55:28 PDT
Comment on attachment 275211 [details]
Patch and Layout Test

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

r=me, one less PageCache bug, Yay :)

> Source/WebCore/history/PageCache.cpp:396
> +    // Focus the main frame, defocusing a focused subframe.

May be a good idea to add a comment stating that this may fire JS 'focus' / 'blur' events. It may also be nice to explain why we need to remove focus from any subframe (i.e. The frame tree will be deconstructed upon entering PageCache and all Frame objects will be destroyed except for the MainFrame which will be re-used for the navigation).
Comment 4 Daniel Bates 2016-03-30 13:55:46 PDT
(In reply to comment #3)
> > Source/WebCore/history/PageCache.cpp:396
> > +    // Focus the main frame, defocusing a focused subframe.
> 
> May be a good idea to add a comment stating that this may fire JS 'focus' /
> 'blur' events. It may also be nice to explain why we need to remove focus
> from any subframe (i.e. The frame tree will be deconstructed upon entering
> PageCache and all Frame objects will be destroyed except for the MainFrame
> which will be re-used for the navigation).

Maybe something like:

Focus the main frame, defocusing a focused subframe (if we have one). We do this here, before the page enters the page cache, while we still can dispatch DOM blur/focus events.
Comment 5 Chris Dumez 2016-03-30 22:57:36 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > > Source/WebCore/history/PageCache.cpp:396
> > > +    // Focus the main frame, defocusing a focused subframe.
> > 
> > May be a good idea to add a comment stating that this may fire JS 'focus' /
> > 'blur' events. It may also be nice to explain why we need to remove focus
> > from any subframe (i.e. The frame tree will be deconstructed upon entering
> > PageCache and all Frame objects will be destroyed except for the MainFrame
> > which will be re-used for the navigation).
> 
> Maybe something like:
> 
> Focus the main frame, defocusing a focused subframe (if we have one). We do
> this here, before the page enters the page cache, while we still can
> dispatch DOM blur/focus events.

Ok
Comment 6 Chris Dumez 2016-03-30 23:01:21 PDT
(In reply to comment #3)
> Comment on attachment 275211 [details]
> Patch and Layout Test
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=275211&action=review
> 
> r=me, one less PageCache bug, Yay :)
> 
> > Source/WebCore/history/PageCache.cpp:396
> > +    // Focus the main frame, defocusing a focused subframe.
> 
> May be a good idea to add a comment stating that this may fire JS 'focus' /
> 'blur' events. It may also be nice to explain why we need to remove focus
> from any subframe (i.e. The frame tree will be deconstructed upon entering
> PageCache and all Frame objects will be destroyed except for the MainFrame
> which will be re-used for the navigation).

Btw, my comment about the frame objects getting destroyed was likely inaccurate.
Comment 7 Daniel Bates 2016-03-31 15:43:45 PDT
Committed r198924: <http://trac.webkit.org/changeset/198924>