Bug 197621 – Add assertions to CachedFrame to help figure out crash in CachedFrame constructor

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Chris Dumez]

Add assertions to CachedFrame to help figure out crash in CachedFrame constructor :
Thread[0] EXC_BREAKPOINT (SIGTRAP) (0x0000000000000002, 0x0000000000000000)
[  0] 0x00000007d81eddc3 WebCore`WTFCrashWithInfo(int, char const*, char const*, int) + 19

     0x00000007d81eddb7:     movq %rsi, -0x18(%rbp)
     0x00000007d81eddbb:     movq %rdx, -0x10(%rbp)
     0x00000007d81eddbf:     movl %ecx, -0x4(%rbp)
     0x00000007d81eddc2:     int3 
 ->  0x00000007d81eddc3:      ud2 
     0x00000007d81eddc5:     nopw %cs:(%rax,%rax)

[  1] 0x00000007d904a204 WebCore`WebCore::CachedFrame::CachedFrame(WebCore::Frame&) + 724 at CachedFrame.cpp:150:5
       146 	    // Create the CachedFrames for all Frames in the FrameTree.
       147 	    for (Frame* child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
       148 	        m_childFrames.append(std::make_unique<CachedFrame>(*child));
       149 	
    -> 150 	    RELEASE_ASSERT(m_document->domWindow()->frame());
       151 	
       152 	    // Active DOM objects must be suspended before we cache the frame script data.
       153 	    m_document->suspend(ReasonForSuspension::PageCache);
       154 	
    
[  2] 0x00000007d904bf9b WebCore`WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*) [inlined] WebCore::CachedFrame::CachedFrame(WebCore::Frame&) + 10 at CachedFrame.cpp:137:1
       133 	}
       134 	
       135 	CachedFrame::CachedFrame(Frame& frame)
       136 	    : CachedFrameBase(frame)
    -> 137 	{
       138 	#ifndef NDEBUG
       139 	    cachedFrameCounter.increment();
       140 	#endif
       141 	    ASSERT(m_document);
    
[  2] 0x00000007d904bf91 WebCore`WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*) [inlined] std::__1::__unique_if<WebCore::CachedFrame>::__unique_single std::__1::make_unique<WebCore::CachedFrame, WebCore::Frame&>(WebCore::Frame&) + 13 at memory:3078
       3074	
       3075	template <class _T1, class _D1>
       3076	inline _LIBCPP_INLINE_VISIBILITY
       3077	bool
    -> 3078	operator>=(const unique_ptr<_T1, _D1>& __x, nullptr_t)
       3079	{
       3080	    return !(__x < nullptr);
       3081	}
       3082	
    
[  2] 0x00000007d904bf84 WebCore`WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*) [inlined] WebCore::CachedPage::CachedPage(WebCore::Page&) + 34 at CachedPage.cpp:59
       55  	
       56  	CachedPage::CachedPage(Page& page)
       57  	    : m_page(page)
       58  	    , m_expirationTime(MonotonicTime::now() + Seconds(page.settings().backForwardCacheExpirationInterval()))
    -> 59  	    , m_cachedMainFrame(std::make_unique<CachedFrame>(page.mainFrame()))
       60  	{
       61  	#ifndef NDEBUG
       62  	    cachedPageCounter.increment();
       63  	#endif
    
[  2] 0x00000007d904bf62 WebCore`WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*) [inlined] WebCore::CachedPage::CachedPage(WebCore::Page&) at CachedPage.cpp:60
       56  	CachedPage::CachedPage(Page& page)
       57  	    : m_page(page)
       58  	    , m_expirationTime(MonotonicTime::now() + Seconds(page.settings().backForwardCacheExpirationInterval()))
       59  	    , m_cachedMainFrame(std::make_unique<CachedFrame>(page.mainFrame()))
    -> 60  	{
       61  	#ifndef NDEBUG
       62  	    cachedPageCounter.increment();
       63  	#endif
       64  	}
    
[  2] 0x00000007d904bf62 WebCore`WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*) [inlined] std::__1::__unique_if<WebCore::CachedPage>::__unique_single std::__1::make_unique<WebCore::CachedPage, WebCore::Page&>(WebCore::Page&) + 13 at memory:3078
       3074	
       3075	template <class _T1, class _D1>
       3076	inline _LIBCPP_INLINE_VISIBILITY
       3077	bool
    -> 3078	operator>=(const unique_ptr<_T1, _D1>& __x, nullptr_t)
       3079	{
       3080	    return !(__x < nullptr);
       3081	}
       3082	
    
[  2] 0x00000007d904bf55 WebCore`WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*) + 437 at PageCache.cpp:463
       459 	    {
       460 	        // Make sure we don't fire any JS events in this scope.
       461 	        ScriptDisallowedScope::InMainThread scriptDisallowedScope;
       462 	
    -> 463 	        item.m_cachedPage = std::make_unique<CachedPage>(*page);
       464 	        item.m_pruningReason = PruningReason::None;
       465 	        m_items.add(&item);
       466 	    }
       467 	    prune(PruningReason::ReachedMaxSize);
    
[  3] 0x00000007d801a246 WebCore`WebCore::FrameLoader::commitProvisionalLoad() + 262 at FrameLoader.cpp:1999:32
       1995	
       1996	    if (!m_frame.tree().parent() && history().currentItem() && history().currentItem() != history().provisionalItem()) {
       1997	        // Check to see if we need to cache the page we are navigating away from into the back/forward cache.
       1998	        // We are doing this here because we know for sure that a new page is about to be loaded.
    -> 1999	        PageCache::singleton().addIfCacheable(*history().currentItem(), m_frame.page());
       2000	        
       2001	        WebCore::jettisonExpensiveObjectsOnTopLevelNavigation();
       2002	    }
       2003	
    
[  4] 0x00000007d9243ec2 WebCore`WebCore::DocumentLoader::finishedLoading() [inlined] WebCore::DocumentLoader::commitIfReady() + 35 at DocumentLoader.cpp:366:24
       362 	void DocumentLoader::commitIfReady()
       363 	{
       364 	    if (!m_committed) {
       365 	        m_committed = true;
    -> 366 	        frameLoader()->commitProvisionalLoad();
       367 	    }
       368 	}
       369 	
       370 	bool DocumentLoader::isLoading() const
    
[  4] 0x00000007d9243e9f WebCore`WebCore::DocumentLoader::finishedLoading() + 351 at DocumentLoader.cpp:433
       429 	
       430 	    MonotonicTime responseEndTime = m_timeOfLastDataReceived ? m_timeOfLastDataReceived : MonotonicTime::now();
       431 	    timing().setResponseEnd(responseEndTime);
       432 	
    -> 433 	    commitIfReady();
       434 	    if (!frameLoader())
       435 	        return;
       436 	
       437 	    if (!maybeCreateArchive()) {
    
[  5] 0x00000007d8019c15 WebCore`WebCore::DocumentLoader::maybeLoadEmpty() + 741 at DocumentLoader.cpp:1710:5
       1706	    }
       1707	
       1708	    String mimeType = shouldLoadEmpty ? "text/html" : frameLoader()->client().generatedMIMETypeForURLScheme(m_request.url().protocol().toStringWithoutCopying());
       1709	    m_response = ResourceResponse(m_request.url(), mimeType, 0, String());
    -> 1710	    finishedLoading();
       1711	    return true;
       1712	}
       1713	
       1714	void DocumentLoader::startLoadingMainResource()
    
[  6] 0x00000007d92462c9 WebCore`WebCore::DocumentLoader::loadMainResource(WebCore::ResourceRequest&&) + 2009 at DocumentLoader.cpp:1884:9
       1880	        // If the load was aborted by clearing m_request, it's possible the ApplicationCacheHost
       1881	        // is now in a state where starting an empty load will be inconsistent. Replace it with
       1882	        // a new ApplicationCacheHost.
       1883	        m_applicationCacheHost = std::make_unique<ApplicationCacheHost>(*this);
    -> 1884	        maybeLoadEmpty();
       1885	        return;
       1886	    }
       1887	
       1888	    ASSERT(m_frame);

See <rdar://problem/49877867>.

[Chris Dumez]

Created attachment 369155 [details]
Patch

[Geoffrey Garen]

Comment on attachment 369155 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 369155 [details]
Patch

Clearing flags on attachment: 369155

Committed r244971: <https://trac.webkit.org/changeset/244971>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50510471>

[Darin Adler]

Should we remove these assertions now? Or should we update the comments and keep them permanently?