Changeset 261018 in webkit


Ignore:
Timestamp:
May 1, 2020 1:50:18 PM (4 years ago)
Author:
Jack Lee
Message:

Nullptr crash in EditCommand::EditCommand via CompositeEditCommand::removeNode
​https://bugs.webkit.org/show_bug.cgi?id=207600
Source/WebCore:

<rdar://problem/56969450>

Reviewed by Geoffrey Garen.

Second part of the fix. Remove m_frame in FrameSelection so it will not be
inadvertently used and cause this crash.

No new tests, covered by existing test.

  • editing/AlternativeTextController.cpp:

(WebCore::AlternativeTextController::rootViewRectForRange const):

  • editing/FrameSelection.cpp:

(WebCore::FrameSelection::FrameSelection):
(WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
(WebCore::FrameSelection::modify):
(WebCore::FrameSelection::selectFrameElementInParentIfFullySelected):
(WebCore::FrameSelection::setFocusedElementIfNeeded):
(WebCore::FrameSelection::shouldDeleteSelection const):
(WebCore::FrameSelection::shouldDeleteSelection):
(WebCore::FrameSelection::revealSelection):
(WebCore::FrameSelection:: shouldChangeSelection):
(WebCore::FrameSelection::shouldChangeSelection const):

  • editing/FrameSelection.h:
  • editing/atk/FrameSelectionAtk.cpp:

(WebCore::FrameSelection::notifyAccessibilityForSelectionChange):

  • editing/mac/FrameSelectionMac.mm:

(WebCore::FrameSelection::notifyAccessibilityForSelectionChange):

LayoutTests:

Reviewed by Geoffrey Garen.

Reduce run time for this test case.

  • editing/inserting/insert-list-then-edit-command-crash.html:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r261004 r261018  
     12020-05-01  Jack Lee  <shihchieh_lee@apple.com>
     2
     3        Nullptr crash in EditCommand::EditCommand via CompositeEditCommand::removeNode
     4        https://bugs.webkit.org/show_bug.cgi?id=207600
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Reduce run time for this test case.
     9
     10        * editing/inserting/insert-list-then-edit-command-crash.html:
     11
    1122020-05-01  Eric Carlson  <eric.carlson@apple.com>
    213

  • trunk/LayoutTests/editing/inserting/insert-list-then-edit-command-crash.html

    r260831 r261018  
    1 <body><image></image><form id=form contentEditable=true><object data=? onload=objectOnLoad()></object></form><dialog open="true">a</dialog>
     1<div style="width: 1px; height: 1px;"></div><div contentEditable=true><object data="?" onload=objectOnLoad()></object></div><span>text</span>
    22<script>
    3     document.getSelection().empty();
    4     document.execCommand("selectAll", false);
    53    if (window.testRunner) {
    64        testRunner.dumpAsText();
    … …  
    86    }
    97
     8    document.getSelection().empty();
     9    document.execCommand("selectAll", false);
     10
    1011    function objectOnLoad() {
    1112        document.execCommand("insertUnorderedList", false);
    1213        document.execCommand("italic", false);
    13         requestAnimationFrame(function () {
    14             document.body.innerHTML = "<p> Tests inserting list followed by an edit command. The test passes if WebKit doesn't crash or hit an assertion.</p>";
    15             if (window.testRunner)
    16                 testRunner.notifyDone();
    17         });
     14        document.body.innerHTML = "<p> Tests inserting list followed by an edit command. The test passes if WebKit doesn't crash or hit an assertion.</p>";
     15        testRunner.notifyDone();
    1816    }
    1917</script>

  • trunk/Source/WebCore/ChangeLog

    r261017 r261018  
     12020-05-01  Jack Lee  <shihchieh_lee@apple.com>
     2
     3        Nullptr crash in EditCommand::EditCommand via CompositeEditCommand::removeNode
     4        https://bugs.webkit.org/show_bug.cgi?id=207600
     5        <rdar://problem/56969450>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Second part of the fix. Remove m_frame in FrameSelection so it will not be
     10        inadvertently used and cause this crash.
     11
     12        No new tests, covered by existing test.
     13
     14        * editing/AlternativeTextController.cpp:
     15        (WebCore::AlternativeTextController::rootViewRectForRange const):
     16        * editing/FrameSelection.cpp:
     17        (WebCore::FrameSelection::FrameSelection):
     18        (WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
     19        (WebCore::FrameSelection::modify):
     20        (WebCore::FrameSelection::selectFrameElementInParentIfFullySelected):
     21        (WebCore::FrameSelection::setFocusedElementIfNeeded):
     22        (WebCore::FrameSelection::shouldDeleteSelection const):
     23        (WebCore::FrameSelection::shouldDeleteSelection):
     24        (WebCore::FrameSelection::revealSelection):
     25        (WebCore::FrameSelection:: shouldChangeSelection):
     26        (WebCore::FrameSelection::shouldChangeSelection const):
     27        * editing/FrameSelection.h:
     28        * editing/atk/FrameSelectionAtk.cpp:
     29        (WebCore::FrameSelection::notifyAccessibilityForSelectionChange):
     30        * editing/mac/FrameSelectionMac.mm:
     31        (WebCore::FrameSelection::notifyAccessibilityForSelectionChange):
     32
    1332020-05-01  Darin Adler  <darin@apple.com>
    234

  • trunk/Source/WebCore/editing/AlternativeTextController.cpp

    r260831 r261018  
    343343FloatRect AlternativeTextController::rootViewRectForRange(const SimpleRange& range) const
    344344{
    345     auto* view = m_document.frame()->view();
     345    auto* view = m_document.view();
    346346    if (!view)
    347347        return { };

  • trunk/Source/WebCore/editing/FrameSelection.cpp

    r260855 r261018  
    147147FrameSelection::FrameSelection(Document* document)
    148148    : m_document(document)
    149     , m_frame(document? document->frame() : nullptr)
    150149    , m_xPosForVerticalArrowNavigation(NoXPosForVerticalArrowNavigation())
    151150    , m_granularity(CharacterGranularity)
    … …  
    158157    , m_caretPaint(true)
    159158    , m_isCaretBlinkingSuspended(false)
    160     , m_focused(m_frame && m_frame->page() && m_frame->page()->focusController().focusedFrame() == m_frame)
     159    , m_focused(document && document->frame() && document->page() && document->page()->focusController().focusedFrame() == document->frame())
    161160    , m_shouldShowBlockCursor(false)
    162161    , m_pendingSelectionUpdate(false)
    … …  
    337336        newSelection.setIsDirectional(true);
    338337
    339     if (!m_frame) {
     338    if (!m_document || !m_document->frame()) {
    340339        m_selection = newSelection;
    341340        return false;
    … …  
    343342
    344343    // <http://bugs.webkit.org/show_bug.cgi?id=23464>: Infinite recursion at FrameSelection::setSelection
    345     // if document->frame() == m_frame we can get into an infinite loop
     344    // if document->frame() == m_document->frame() we can get into an infinite loop
    346345    if (Document* newSelectionDocument = newSelection.base().document()) {
    347346        if (RefPtr<Frame> newSelectionFrame = newSelectionDocument->frame()) {
    348             if (newSelectionFrame != m_frame && newSelectionDocument != m_document) {
    349                 newSelectionFrame->selection().setSelection(newSelection, options, AXTextStateChangeIntent(), align, granularity);
     347            if (newSelectionFrame != m_document->frame() && newSelectionDocument != m_document) {
     348                newSelectionDocument->selection().setSelection(newSelection, options, AXTextStateChangeIntent(), align, granularity);
    350349                // It's possible that during the above set selection, this FrameSelection has been modified by
    351350                // selectFrameElementInParentIfFullySelected, but that the selection is no longer valid since
    … …  
    385384        auto* oldFocusedElement = m_document->focusedElement();
    386385        setFocusedElementIfNeeded();
     386        if (!m_document->frame())
     387            return false;
    387388        // FIXME: Should not be needed.
    388389        if (m_document->focusedElement() != oldFocusedElement)
    … …  
    13781379        return false;
    13791380
    1380     if (isSpatialNavigationEnabled(m_frame))
     1381    if (m_document && isSpatialNavigationEnabled(m_document->frame())) {
    13811382        if (!wasRange && alter == AlterationMove && position == originalStartPosition)
    13821383            return false;
     1384    }
    13831385
    13841386    if (m_document && AXObjectCache::accessibilityEnabled()) {
    … …  
    19331935{
    19341936    // Find the parent frame; if there is none, then we have nothing to do.
    1935     Frame* parent = m_frame->tree().parent();
     1937    Frame* parent = m_document->frame()->tree().parent();
    19361938    if (!parent)
    19371939        return;
    1938     Page* page = m_frame->page();
     1940    Page* page = m_document->page();
    19391941    if (!page)
    19401942        return;
    … …  
    19491951
    19501952    // Get to the <iframe> or <frame> (or even <object>) element in the parent frame.
    1951     Element* ownerElement = m_frame->ownerElement();
     1953    Element* ownerElement = m_document->ownerElement();
    19521954    if (!ownerElement)
    19531955        return;
    … …  
    22542256    if (caretBrowsing) {
    22552257        if (Element* anchor = enclosingAnchorElement(m_selection.base())) {
    2256             m_document->page()->focusController().setFocusedElement(anchor, *m_frame);
     2258            m_document->page()->focusController().setFocusedElement(anchor, *m_document->frame());
    22572259            return;
    22582260        }
    … …  
    22662268            // work in the long term, but this is the safest fix at this time.
    22672269            if (target->isMouseFocusable() && !isFrameElement(target)) {
    2268                 m_document->page()->focusController().setFocusedElement(target, *m_frame);
     2270                m_document->page()->focusController().setFocusedElement(target, *m_document->frame());
    22692271                return;
    22702272            }
    … …  
    22752277
    22762278    if (caretBrowsing)
    2277         m_document->page()->focusController().setFocusedElement(nullptr, *m_frame);
     2279        m_document->page()->focusController().setFocusedElement(nullptr, *m_document->frame());
    22782280}
    22792281
    … …  
    23012303{
    23022304#if PLATFORM(IOS_FAMILY)
    2303     if (m_frame->selectionChangeCallbacksDisabled())
     2305    if (m_document->frame() && m_document->frame()->selectionChangeCallbacksDisabled())
    23042306        return true;
    23052307#endif
    … …  
    24122414                updateAppearance();
    24132415                if (m_document->page())
    2414                     m_document->page()->chrome().client().notifyRevealedSelectionByScrollingFrame(*m_frame);
     2416                    m_document->page()->chrome().client().notifyRevealedSelectionByScrollingFrame(*m_document->frame());
    24152417            }
    24162418        }
    … …  
    24462448{
    24472449#if PLATFORM(IOS_FAMILY)
    2448     if (m_frame->selectionChangeCallbacksDisabled())
     2450    if (m_document->frame() && m_document->frame()->selectionChangeCallbacksDisabled())
    24492451        return true;
    24502452#endif

  • trunk/Source/WebCore/editing/FrameSelection.h

    r260855 r261018  
    339339
    340340    Document* m_document;
    341     Frame* m_frame;
    342341
    343342    LayoutUnit m_xPosForVerticalArrowNavigation;

  • trunk/Source/WebCore/editing/atk/FrameSelectionAtk.cpp

    r251798 r261018  
    9898        return;
    9999
    100     AXObjectCache* cache = m_frame->document()->existingAXObjectCache();
     100    AXObjectCache* cache = m_document->existingAXObjectCache();
    101101    if (!cache)
    102102        return;

  • trunk/Source/WebCore/editing/mac/FrameSelectionMac.mm

    r237266 r261018  
    3535void FrameSelection::notifyAccessibilityForSelectionChange(const AXTextStateChangeIntent& intent)
    3636{
    37     Document* document = m_frame->document();
    38 
    3937    if (m_selection.start().isNotNull() && m_selection.end().isNotNull()) {
    40         if (AXObjectCache* cache = document->existingAXObjectCache())
     38        if (AXObjectCache* cache = m_document->existingAXObjectCache())
    4139            cache->postTextStateChangeNotification(m_selection.start(), intent, m_selection);
    4240    }
    … …  
    4745        return;
    4846
    49     RenderView* renderView = document->renderView();
     47    RenderView* renderView = m_document->renderView();
    5048    if (!renderView)
    5149        return;
    52     FrameView* frameView = m_frame->view();
     50    FrameView* frameView = m_document->view();
    5351    if (!frameView)
    5452        return;
Note: See TracChangeset for help on using the changeset viewer.