WebKit Bugzilla
Attachment 368286 Details for
Bug 197300
: ASSERT(scriptExecutionContext()) in Performance::resourceTimingBufferFullTimerFired()
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-197300-20190425163911.patch (text/plain), 4.96 KB, created by
Chris Dumez
on 2019-04-25 16:39:12 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Chris Dumez
Created:
2019-04-25 16:39:12 PDT
Size:
4.96 KB
patch
obsolete
>Subversion Revision: 244669 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 392a5b91bb7cffa7a5f1952d2080cb02fa243f76..27f13086c03593f146f90c2d48c6b7cde9240e04 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,43 @@ >+2019-04-25 Chris Dumez <cdumez@apple.com> >+ >+ ASSERT(scriptExecutionContext()) in Performance::resourceTimingBufferFullTimerFired() >+ https://bugs.webkit.org/show_bug.cgi?id=197300 >+ <rdar://problem/49965990> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ We crash because the scriptExecutionContext has been destroyed by the time the m_resourceTimingBufferFullTimer >+ timer fires. However, r241598 already makes sure that we stop the timer when the script execution context >+ is destroyed. This makes me think that somebody restarts the timer *after* the script execution context has >+ been destroyed. The thing is that we only start the timer in Performance::addResourceTiming() and there are >+ only 2 call sites for this method. Both call sites get the Performance object from the Window object, which >+ they get from the Document object. As a result, I would believe that the Window's document is alive, even >+ though the Performance object's scriptExecutionContext is not. This could indicate that the Performance >+ object's scriptExecutionContext gets out of sync with its Window's document. I have found one place where >+ it could happen in theory (DOMWindow::didSecureTransitionTo()). I have not been able to write a test >+ confirming my theory though so this is a speculative fix. I have also added a few assertions to help us >+ track down the issue if my speculative fix turns out to be ineffective. >+ >+ No new tests, we do not know how to reproduce. >+ >+ * page/DOMWindow.cpp: >+ (WebCore::DOMWindow::didSecureTransitionTo): >+ This is a speculative fix for the crash. When a DOMWindow transitions from one document to >+ another, reset its data members which store the DOMWindow's document to make sure that they >+ do not get out of sync. >+ >+ (WebCore::DOMWindow::crypto const): >+ (WebCore::DOMWindow::navigator): >+ (WebCore::DOMWindow::performance const): >+ Add assertions to to make sure that the member's scriptExecutionContext is in sync with >+ the window's. >+ >+ * page/Performance.cpp: >+ (WebCore::Performance::addResourceTiming): >+ Add assertion to make sure that the scriptExecutionContext() is non-null when calling this >+ as this may start the m_resourceTimingBufferFullTimer timer. If my speculative fix above >+ does not work, we should hit this and this should tell us which call site is causing this. >+ > 2019-04-25 Timothy Hatcher <timothy@apple.com> > > Disable ContentChangeObserver on iOSMac. >diff --git a/Source/WebCore/page/DOMWindow.cpp b/Source/WebCore/page/DOMWindow.cpp >index 7341ec739749e49520e09bde78ae94f8c0ec3e79..9583062ade9c46b478a8b3abc95d983d1d2599b2 100644 >--- a/Source/WebCore/page/DOMWindow.cpp >+++ b/Source/WebCore/page/DOMWindow.cpp >@@ -415,6 +415,12 @@ DOMWindow::DOMWindow(Document& document) > void DOMWindow::didSecureTransitionTo(Document& document) > { > observeContext(&document); >+ >+ // The Window is being transferred from one document to another so we need to reset data >+ // members that store the window's document (rather than the window itself). >+ m_crypto = nullptr; >+ m_navigator = nullptr; >+ m_performance = nullptr; > } > > DOMWindow::~DOMWindow() >@@ -648,6 +654,7 @@ Crypto& DOMWindow::crypto() const > { > if (!m_crypto) > m_crypto = Crypto::create(document()); >+ ASSERT(m_crypto->scriptExecutionContext() == document()); > return *m_crypto; > } > >@@ -713,6 +720,7 @@ Navigator& DOMWindow::navigator() > { > if (!m_navigator) > m_navigator = Navigator::create(scriptExecutionContext(), *this); >+ ASSERT(m_navigator->scriptExecutionContext() == document()); > > return *m_navigator; > } >@@ -723,6 +731,7 @@ Performance& DOMWindow::performance() const > MonotonicTime timeOrigin = document() && document()->loader() ? document()->loader()->timing().referenceMonotonicTime() : MonotonicTime::now(); > m_performance = Performance::create(document(), timeOrigin); > } >+ ASSERT(m_performance->scriptExecutionContext() == document()); > return *m_performance; > } > >diff --git a/Source/WebCore/page/Performance.cpp b/Source/WebCore/page/Performance.cpp >index a83d0c0a84678f49ffb8a421998babaf4b346f30..57a2a391386386d2c440af2adeff95bced820079 100644 >--- a/Source/WebCore/page/Performance.cpp >+++ b/Source/WebCore/page/Performance.cpp >@@ -181,6 +181,8 @@ void Performance::setResourceTimingBufferSize(unsigned size) > > void Performance::addResourceTiming(ResourceTiming&& resourceTiming) > { >+ ASSERT(scriptExecutionContext()); >+ > auto entry = PerformanceResourceTiming::create(m_timeOrigin, WTFMove(resourceTiming)); > > if (m_waitingForBackupBufferToBeProcessed) {
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 197300
:
368286
|
368288