Changeset 241848 in webkit


Ignore:
Timestamp:
Feb 20, 2019 4:06:27 PM (5 years ago)
Author:
rniwa@webkit.org
Message:

Crash in DOMWindowExtension::suspendForPageCache
https://bugs.webkit.org/show_bug.cgi?id=194871

Reviewed by Chris Dumez.

This is a speculative fix for a crash in DOMWindowExtension::suspendForPageCache.

We think it's possible for DOMWindowExtension::suspendForPageCache notifying the clients via
dispatchWillDisconnectDOMWindowExtensionFromGlobalObject to remove other DOMWindowExtension's.
Check that each DOMWindowProperty is still in m_properties before invoking suspendForPageCache
to avoid the crash.

  • page/DOMWindow.cpp:

(WebCore::DOMWindow::willDestroyCachedFrame):
(WebCore::DOMWindow::willDestroyDocumentInFrame):
(WebCore::DOMWindow::willDetachDocumentFromFrame):
(WebCore::DOMWindow::suspendForPageCache):
(WebCore::DOMWindow::resumeFromPageCache):

  • page/DOMWindowExtension.cpp:

(WebCore::DOMWindowExtension::suspendForPageCache):

Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r241842 r241848  
     12019-02-20  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Crash in DOMWindowExtension::suspendForPageCache
     4        https://bugs.webkit.org/show_bug.cgi?id=194871
     5
     6        Reviewed by Chris Dumez.
     7
     8        This is a speculative fix for a crash in DOMWindowExtension::suspendForPageCache.
     9
     10        We think it's possible for DOMWindowExtension::suspendForPageCache notifying the clients via
     11        dispatchWillDisconnectDOMWindowExtensionFromGlobalObject to remove other DOMWindowExtension's.
     12        Check that each DOMWindowProperty is still in m_properties before invoking suspendForPageCache
     13        to avoid the crash.
     14
     15        * page/DOMWindow.cpp:
     16        (WebCore::DOMWindow::willDestroyCachedFrame):
     17        (WebCore::DOMWindow::willDestroyDocumentInFrame):
     18        (WebCore::DOMWindow::willDetachDocumentFromFrame):
     19        (WebCore::DOMWindow::suspendForPageCache):
     20        (WebCore::DOMWindow::resumeFromPageCache):
     21        * page/DOMWindowExtension.cpp:
     22        (WebCore::DOMWindowExtension::suspendForPageCache):
     23
    1242019-02-20  Alex Christensen  <achristensen@webkit.org>
    225
  • trunk/Source/WebCore/page/DOMWindow.cpp

    r241432 r241848  
    457457    // It is necessary to copy m_properties to a separate vector because the DOMWindowProperties may
    458458    // unregister themselves from the DOMWindow as a result of the call to willDestroyGlobalObjectInCachedFrame.
    459     for (auto& property : copyToVector(m_properties))
    460         property->willDestroyGlobalObjectInCachedFrame();
     459    for (auto* property : copyToVector(m_properties)) {
     460        if (m_properties.contains(property))
     461            property->willDestroyGlobalObjectInCachedFrame();
     462    }
    461463}
    462464
     
    465467    // It is necessary to copy m_properties to a separate vector because the DOMWindowProperties may
    466468    // unregister themselves from the DOMWindow as a result of the call to willDestroyGlobalObjectInFrame.
    467     for (auto& property : copyToVector(m_properties))
    468         property->willDestroyGlobalObjectInFrame();
     469    for (auto* property : copyToVector(m_properties)) {
     470        if (m_properties.contains(property))
     471            property->willDestroyGlobalObjectInFrame();
     472    }
    469473}
    470474
     
    476480    // It is necessary to copy m_properties to a separate vector because the DOMWindowProperties may
    477481    // unregister themselves from the DOMWindow as a result of the call to willDetachGlobalObjectFromFrame.
    478     for (auto& property : copyToVector(m_properties))
    479         property->willDetachGlobalObjectFromFrame();
     482    for (auto& property : copyToVector(m_properties)) {
     483        if (m_properties.contains(property))
     484            property->willDetachGlobalObjectFromFrame();
     485    }
    480486
    481487    if (m_performance)
     
    521527void DOMWindow::suspendForPageCache()
    522528{
    523     for (auto& property : copyToVector(m_properties))
    524         property->suspendForPageCache();
     529    for (auto* property : copyToVector(m_properties)) {
     530        if (m_properties.contains(property))
     531            property->suspendForPageCache();
     532    }
    525533
    526534    m_suspendedForDocumentSuspension = true;
     
    529537void DOMWindow::resumeFromPageCache()
    530538{
    531     for (auto& property : copyToVector(m_properties))
    532         property->resumeFromPageCache();
     539    for (auto* property : copyToVector(m_properties)) {
     540        if (m_properties.contains(property))
     541            property->resumeFromPageCache();
     542    }
    533543
    534544    m_suspendedForDocumentSuspension = false;
  • trunk/Source/WebCore/page/DOMWindowExtension.cpp

    r237029 r241848  
    4949    // while there is still work to do.
    5050    Ref<DOMWindowExtension> protectedThis(*this);
    51    
    52     Frame* frame = this->frame();
     51
     52    auto frame = makeRef(*this->frame());
    5353    frame->loader().client().dispatchWillDisconnectDOMWindowExtensionFromGlobalObject(this);
    5454
    55     m_disconnectedFrame = frame;
     55    m_disconnectedFrame = WTFMove(frame);
    5656
    5757    DOMWindowProperty::suspendForPageCache();
Note: See TracChangeset for help on using the changeset viewer.