Changeset 227948 in webkit


Ignore:
Timestamp:
Jan 31, 2018 8:18:38 PM (6 years ago)
Author:
pvollan@apple.com
Message:

Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing
https://bugs.webkit.org/show_bug.cgi?id=181204
<rdar://problem/36256274>

Reviewed by Ryosuke Niwa.

Source/WebCore:

When a frame element is moved in the DOM tree during the execution of a beforeunload handler,
the frame will be detached when removed from its previous position in the DOM tree. When being
detached, an attempt will also be made to stop the load by calling FrameLoader::stopAllLoaders().
However, this method will return early when executed in a beforeunload handler, since navigation
is not allowed then. The end result is a detached frame which will continue to load, and hitting
asserts in DocumentLoader::dataReceived(), and DocumentLoader::notifyFinished(). It should be
possible to stop a frame load, even when executing a beforeunload handler.

No new tests. Covered by the existing test fast/events/beforeunload-dom-manipulation-crash.html.

  • history/PageCache.cpp:

(WebCore::PageCache::addIfCacheable): Fix a failing API test by allowing scripts to be executed
under the PageCache::prune method.

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::isStopLoadingAllowed const):
(WebCore::FrameLoader::stopAllLoaders):

  • loader/FrameLoader.h:
  • svg/graphics/SVGImage.cpp:

(WebCore::SVGImage::~SVGImage): Disable scripts disallowed assertions in this scope, since it is
safe in this context.

Tools:

Implement 'testRunner.forceImmediateCompletion()' for WK1.

  • DumpRenderTree/TestRunner.cpp:

(forceImmediateCompletionCallback):
(TestRunner::staticFunctions):

LayoutTests:

  • fast/events/beforeunload-dom-manipulation-crash.html: Make it clear that the

frame element is a child of the 'del' element.

  • fast/events/beforeunload-dom-manipulation-crash-expected.html:
  • platform/mac-wk1/TestExpectations: Unskip test.
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r227947 r227948  
     12018-01-31  Per Arne Vollan  <pvollan@apple.com>
     2
     3        Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing
     4        https://bugs.webkit.org/show_bug.cgi?id=181204
     5        <rdar://problem/36256274>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        * fast/events/beforeunload-dom-manipulation-crash.html: Make it clear that the
     10        frame element is a child of the 'del' element.
     11        * fast/events/beforeunload-dom-manipulation-crash-expected.html:
     12        * platform/mac-wk1/TestExpectations: Unskip test.
     13
    1142018-01-31  Javier Fernandez  <jfernandez@igalia.com>
    215
  • trunk/LayoutTests/fast/events/beforeunload-dom-manipulation-crash-expected.txt

    r227743 r227948  
    11This test passes if it does not crash.
     2
     3
  • trunk/LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html

    r227743 r227948  
    2424    <p>This test passes if it does not crash.</p>
    2525    <del id="del">
    26     <iframe id="iframe"></iframe>
     26        <iframe id="iframe"></iframe>
     27    </del>
    2728</body>
  • trunk/LayoutTests/platform/mac-wk1/TestExpectations

    r227764 r227948  
    454454webkit.org/b/175886 svg/animations/smil-leak-element-instances.svg [ Pass Failure ]
    455455
    456 webkit.org/b/177020 fast/events/beforeunload-dom-manipulation-crash.html [ Skip ]
    457 
    458456# <rdar://problem/29201698> DumpRenderTree crashed in com.apple.CoreGraphics: CGDataProviderCopyData + 377
    459457[ HighSierra+ ] fast/canvas/webgl/tex-image-and-sub-image-2d-with-potentially-subsampled-image.html [ Crash ]
  • trunk/Source/WebCore/ChangeLog

    r227947 r227948  
     12018-01-31  Per Arne Vollan  <pvollan@apple.com>
     2
     3        Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing
     4        https://bugs.webkit.org/show_bug.cgi?id=181204
     5        <rdar://problem/36256274>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        When a frame element is moved in the DOM tree during the execution of a beforeunload handler,
     10        the frame will be detached when removed from its previous position in the DOM tree. When being
     11        detached, an attempt will also be made to stop the load by calling FrameLoader::stopAllLoaders().
     12        However, this method will return early when executed in a beforeunload handler, since navigation
     13        is not allowed then. The end result is a detached frame which will continue to load, and hitting
     14        asserts in DocumentLoader::dataReceived(), and DocumentLoader::notifyFinished(). It should be
     15        possible to stop a frame load, even when executing a beforeunload handler.
     16
     17        No new tests. Covered by the existing test fast/events/beforeunload-dom-manipulation-crash.html.
     18
     19        * history/PageCache.cpp:
     20        (WebCore::PageCache::addIfCacheable): Fix a failing API test by allowing scripts to be executed
     21        under the PageCache::prune method.
     22        * loader/FrameLoader.cpp:
     23        (WebCore::FrameLoader::isStopLoadingAllowed const):
     24        (WebCore::FrameLoader::stopAllLoaders):
     25        * loader/FrameLoader.h:
     26        * svg/graphics/SVGImage.cpp:
     27        (WebCore::SVGImage::~SVGImage): Disable scripts disallowed assertions in this scope, since it is
     28        safe in this context.
     29
    1302018-01-31  Javier Fernandez  <jfernandez@igalia.com>
    231
  • trunk/Source/WebCore/history/PageCache.cpp

    r227280 r227948  
    432432    setPageCacheState(*page, Document::InPageCache);
    433433
    434     // Make sure we no longer fire any JS events past this point.
    435     ScriptDisallowedScope::InMainThread scriptDisallowedScope;
    436 
    437     item.m_cachedPage = std::make_unique<CachedPage>(*page);
    438     item.m_pruningReason = PruningReason::None;
    439     m_items.add(&item);
    440    
     434    {
     435        // Make sure we don't fire any JS events in this scope.
     436        ScriptDisallowedScope::InMainThread scriptDisallowedScope;
     437
     438        item.m_cachedPage = std::make_unique<CachedPage>(*page);
     439        item.m_pruningReason = PruningReason::None;
     440        m_items.add(&item);
     441    }
    441442    prune(PruningReason::ReachedMaxSize);
    442443}
  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r227743 r227948  
    12551255}
    12561256
     1257bool FrameLoader::isStopLoadingAllowed() const
     1258{
     1259    return m_pageDismissalEventBeingDispatched == PageDismissalType::None;
     1260}
     1261
    12571262struct SharedBool : public RefCounted<SharedBool> {
    12581263    bool value { false };
     
    16681673{
    16691674    ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);
    1670     if (!isNavigationAllowed())
     1675    if (!isStopLoadingAllowed())
    16711676        return;
    16721677
     
    16741679    if (m_inStopAllLoaders)
    16751680        return;
    1676    
     1681
     1682    // This method might dispatch events.
     1683    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(ScriptDisallowedScope::InMainThread::isScriptAllowed());
     1684
    16771685    // Calling stopLoading() on the provisional document loader can blow away
    16781686    // the frame from underneath.
  • trunk/Source/WebCore/loader/FrameLoader.h

    r227743 r227948  
    390390
    391391    bool isNavigationAllowed() const;
     392    bool isStopLoadingAllowed() const;
    392393
    393394    Frame& m_frame;
  • trunk/Source/WebCore/svg/graphics/SVGImage.cpp

    r227841 r227948  
    7777{
    7878    if (m_page) {
     79        ScriptDisallowedScope::DisableAssertionsInScope disabledScope;
    7980        // Store m_page in a local variable, clearing m_page, so that SVGImageChromeClient knows we're destructed.
    8081        std::unique_ptr<Page> currentPage = WTFMove(m_page);
  • trunk/Tools/ChangeLog

    r227943 r227948  
     12018-01-31  Per Arne Vollan  <pvollan@apple.com>
     2
     3        Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing
     4        https://bugs.webkit.org/show_bug.cgi?id=181204
     5        <rdar://problem/36256274>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Implement 'testRunner.forceImmediateCompletion()' for WK1.
     10
     11        * DumpRenderTree/TestRunner.cpp:
     12        (forceImmediateCompletionCallback):
     13        (TestRunner::staticFunctions):
     14
    1152018-01-31  Alex Christensen  <achristensen@webkit.org>
    216
  • trunk/Tools/DumpRenderTree/TestRunner.cpp

    r227743 r227948  
    19841984    controller->simulateWebNotificationClick(arguments[0]);
    19851985
     1986    return JSValueMakeUndefined(context);
     1987}
     1988
     1989static JSValueRef forceImmediateCompletionCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
     1990{
     1991    TestRunner* controller = static_cast<TestRunner*>(JSObjectGetPrivate(thisObject));
     1992    controller->forceImmediateCompletion();
    19861993    return JSValueMakeUndefined(context);
    19871994}
     
    22432250        { "setSpellCheckerLoggingEnabled", setSpellCheckerLoggingEnabledCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
    22442251        { "setOpenPanelFiles", setOpenPanelFilesCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
     2252        { "forceImmediateCompletion", forceImmediateCompletionCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
    22452253        { 0, 0, 0 }
    22462254    };
Note: See TracChangeset for help on using the changeset viewer.