Changeset 207068 in webkit
- Timestamp:
- Oct 11, 2016 1:48:26 AM (8 years ago)
- Location:
- releases/WebKitGTK/webkit-2.14
- Files:
-
- 2 added
- 9 edited
Legend:
- Unmodified
- Added
- Removed
-
releases/WebKitGTK/webkit-2.14/LayoutTests/ChangeLog
r207056 r207068 1 2016-09-10 Chris Dumez <cdumez@apple.com> 2 3 It is possible for Document::m_frame pointer to become stale 4 https://bugs.webkit.org/show_bug.cgi?id=161812 5 <rdar://problem/27745023> 6 7 Reviewed by Ryosuke Niwa. 8 9 Add layout test that crashes on both Mac and iOS due to using a stale 10 Document::m_frame pointer. 11 12 * fast/history/pagehide-remove-iframe-crash-expected.txt: Added. 13 * fast/history/pagehide-remove-iframe-crash.html: Added. 14 1 15 2016-09-08 Simon Fraser <simon.fraser@apple.com> 2 16 -
releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog
r207063 r207068 1 2016-09-10 Chris Dumez <cdumez@apple.com> 2 3 It is possible for Document::m_frame pointer to become stale 4 https://bugs.webkit.org/show_bug.cgi?id=161812 5 <rdar://problem/27745023> 6 7 Reviewed by Ryosuke Niwa. 8 9 Document::m_frame is supposed to get cleared by Document::prepareForDestruction(). 10 The Frame destructor calls Frame::setView(nullptr) which is supposed to call the 11 prepareForDestruction() on the Frame's associated document. However, 12 Frame::setView(nullptr) was calling prepareForDestruction() only if 13 Document::inPageCache() returned true. This is because, we allow Documents to 14 stay alive in the PageCache even though they don't have a frame. 15 16 The issue is that Document::m_inPageCache flag was set to true right before 17 firing the pagehide event, so technically before really entering PageCache. 18 Therefore, we can run into problems if a Frame gets destroyed by a pagehide 19 EventHandler because ~Frame() will not call Document::prepareForDestruction() 20 due to Document::m_inPageCache being true. After the frame is destroyed, 21 Document::m_frame becomes stale and any action on the document will likely 22 lead to crashes (such as the one in the layout test and the radar which 23 happens when trying to unregister event listeners from the document). 24 25 The solution adopted in this patch is to replace the m_inPageCache boolean 26 with a m_pageCacheState enumeration that has 3 states: 27 - NotInPageCache 28 - AboutToEnterPageCache 29 - InPageCache 30 31 Frame::setView() / Frame::setDocument() were then updated to call 32 Document::prepareForDestruction() on the associated document whenever 33 the document's pageCacheState is not InPageCache. This means that we 34 will now call Document::prepareForDestruction() when the document is 35 being detached from its frame while firing the pagehide event. 36 37 Note that I tried to keep this patch minimal. Therefore, I kept 38 the Document::inPageCache() getter for now. I plan to switch all its 39 calls sites to the new Document::pageCacheState() getter in a follow-up 40 patch so that we can finally drop the confusing Document::inPageCache(). 41 42 Test: fast/history/pagehide-remove-iframe-crash.html 43 44 * dom/Document.cpp: 45 (WebCore::Document::Document): 46 (WebCore::Document::~Document): 47 (WebCore::Document::createRenderTree): 48 (WebCore::Document::destroyRenderTree): 49 (WebCore::Document::setFocusedElement): 50 (WebCore::Document::setPageCacheState): 51 (WebCore::Document::topDocument): 52 * dom/Document.h: 53 (WebCore::Document::pageCacheState): 54 (WebCore::Document::inPageCache): 55 * history/CachedFrame.cpp: 56 (WebCore::CachedFrame::destroy): 57 * history/PageCache.cpp: 58 (WebCore::setPageCacheState): 59 (WebCore::PageCache::addIfCacheable): 60 * loader/FrameLoader.cpp: 61 (WebCore::FrameLoader::stopAllLoaders): 62 (WebCore::FrameLoader::open): 63 * loader/HistoryController.cpp: 64 (WebCore::HistoryController::invalidateCurrentItemCachedPage): 65 * page/Frame.cpp: 66 (WebCore::Frame::setView): 67 1 68 2016-09-09 Antti Koivisto <antti@apple.com> 2 69 -
releases/WebKitGTK/webkit-2.14/Source/WebCore/dom/Document.cpp
r205644 r207068 494 494 #endif 495 495 , m_createRenderers(true) 496 , m_inPageCache(false)497 496 , m_accessKeyMapValid(false) 498 497 , m_documentClasses(documentClasses) … … 603 602 604 603 ASSERT(!renderView()); 605 ASSERT( !m_inPageCache);604 ASSERT(m_pageCacheState != InPageCache); 606 605 ASSERT(m_ranges.isEmpty()); 607 606 ASSERT(!m_parentTreeScope); … … 2249 2248 { 2250 2249 ASSERT(!renderView()); 2251 ASSERT( !m_inPageCache);2250 ASSERT(m_pageCacheState != InPageCache); 2252 2251 ASSERT(!m_axObjectCache || this != &topDocument()); 2253 2252 … … 2313 2312 { 2314 2313 ASSERT(hasLivingRenderTree()); 2315 ASSERT( !m_inPageCache);2314 ASSERT(m_pageCacheState != InPageCache); 2316 2315 2317 2316 TemporaryChange<bool> change(m_renderTreeBeingDestroyed, true); … … 3799 3798 return true; 3800 3799 3801 if ( m_inPageCache)3800 if (inPageCache()) 3802 3801 return false; 3803 3802 … … 4727 4726 } 4728 4727 4729 void Document::set InPageCache(bool flag)4730 { 4731 if (m_ inPageCache == flag)4732 return; 4733 4734 m_ inPageCache = flag;4728 void Document::setPageCacheState(PageCacheState state) 4729 { 4730 if (m_pageCacheState == state) 4731 return; 4732 4733 m_pageCacheState = state; 4735 4734 4736 4735 FrameView* v = view(); 4737 4736 Page* page = this->page(); 4738 4737 4739 if (flag) { 4738 switch (state) { 4739 case InPageCache: 4740 4740 if (v) { 4741 4741 // FIXME: There is some scrolling related work that needs to happen whenever a page goes into the … … 4757 4757 4758 4758 clearSharedObjectPool(); 4759 } else { 4759 break; 4760 case NotInPageCache: 4760 4761 if (childNeedsStyleRecalc()) 4761 4762 scheduleStyleRecalc(); 4763 break; 4764 case AboutToEnterPageCache: 4765 break; 4762 4766 } 4763 4767 } … … 5078 5082 // FIXME: This special-casing avoids incorrectly determined top documents during the process 5079 5083 // of AXObjectCache teardown or notification posting for cached or being-destroyed documents. 5080 if (! m_inPageCache&& !m_renderTreeBeingDestroyed) {5084 if (!inPageCache() && !m_renderTreeBeingDestroyed) { 5081 5085 if (!m_frame) 5082 5086 return const_cast<Document&>(*this); -
releases/WebKitGTK/webkit-2.14/Source/WebCore/dom/Document.h
r205613 r207068 1002 1002 void finishedParsing(); 1003 1003 1004 bool inPageCache() const { return m_inPageCache; } 1005 void setInPageCache(bool flag); 1004 enum PageCacheState { NotInPageCache, AboutToEnterPageCache, InPageCache }; 1005 1006 PageCacheState pageCacheState() const { return m_pageCacheState; } 1007 void setPageCacheState(PageCacheState); 1008 1009 // FIXME: Update callers to use pageCacheState() instead. 1010 bool inPageCache() const { return m_pageCacheState != NotInPageCache; } 1006 1011 1007 1012 // Elements can register themselves for the "suspend()" and … … 1594 1599 1595 1600 bool m_createRenderers; 1596 bool m_inPageCache;1601 PageCacheState m_pageCacheState { NotInPageCache }; 1597 1602 1598 1603 HashSet<Element*> m_documentSuspensionCallbackElements; -
releases/WebKitGTK/webkit-2.14/Source/WebCore/history/CachedFrame.cpp
r201310 r207068 265 265 m_document->removeAllEventListeners(); 266 266 267 m_document->set InPageCache(false);267 m_document->setPageCacheState(Document::NotInPageCache); 268 268 m_document->prepareForDestruction(); 269 269 -
releases/WebKitGTK/webkit-2.14/Source/WebCore/history/PageCache.cpp
r201655 r207068 374 374 } 375 375 376 static void set InPageCache(Page& page, bool isInPageCache)376 static void setPageCacheState(Page& page, Document::PageCacheState pageCacheState) 377 377 { 378 378 for (Frame* frame = &page.mainFrame(); frame; frame = frame->tree().traverseNext()) { 379 379 if (auto* document = frame->document()) 380 document->set InPageCache(isInPageCache);380 document->setPageCacheState(pageCacheState); 381 381 } 382 382 } … … 408 408 return; 409 409 410 // Make sure all the documents know they are being added to the PageCache. 411 setInPageCache(*page, true); 410 setPageCacheState(*page, Document::AboutToEnterPageCache); 412 411 413 412 // Focus the main frame, defocusing a focused subframe (if we have one). We do this here, … … 422 421 // could have altered the page in a way that could prevent caching. 423 422 if (!canCache(*page)) { 424 setInPageCache(*page, false); 425 return; 426 } 423 setPageCacheState(*page, Document::NotInPageCache); 424 return; 425 } 426 427 setPageCacheState(*page, Document::InPageCache); 427 428 428 429 // Make sure we no longer fire any JS events past this point. -
releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/FrameLoader.cpp
r205644 r207068 1598 1598 void FrameLoader::stopAllLoaders(ClearProvisionalItemPolicy clearProvisionalItemPolicy) 1599 1599 { 1600 ASSERT(!m_frame.document() || !m_frame.document()->inPageCache());1600 ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache); 1601 1601 if (m_pageDismissalEventBeingDispatched != PageDismissalType::None) 1602 1602 return; … … 2096 2096 clear(document, true, true, cachedFrame.isMainFrame()); 2097 2097 2098 document->set InPageCache(false);2098 document->setPageCacheState(Document::NotInPageCache); 2099 2099 2100 2100 m_needsClear = true; -
releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/HistoryController.cpp
r200770 r207068 271 271 ASSERT(cachedPage->document() == m_frame.document()); 272 272 if (cachedPage->document() == m_frame.document()) { 273 cachedPage->document()->set InPageCache(false);273 cachedPage->document()->setPageCacheState(Document::NotInPageCache); 274 274 cachedPage->clear(); 275 275 } -
releases/WebKitGTK/webkit-2.14/Source/WebCore/page/Frame.cpp
r205717 r207068 247 247 // notified. If we wait until the view is destroyed, then things won't be hooked up enough for 248 248 // these calls to work. 249 if (!view && m_doc && !m_doc->inPageCache())249 if (!view && m_doc && m_doc->pageCacheState() != Document::InPageCache) 250 250 m_doc->prepareForDestruction(); 251 251 … … 269 269 ASSERT(!newDocument || newDocument->frame() == this); 270 270 271 if (m_doc && !m_doc->inPageCache())271 if (m_doc && m_doc->pageCacheState() != Document::InPageCache) 272 272 m_doc->prepareForDestruction(); 273 273
Note: See TracChangeset
for help on using the changeset viewer.