[webkit-reviews] review denied: [Bug 207600] Nullptr crash in EditCommand::EditCommand via CompositeEditCommand::removeNode : [Attachment 398072] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Jack <shihchieh_lee at apple.com>'s
request for review:
Bug 207600: Nullptr crash in EditCommand::EditCommand via
CompositeEditCommand::removeNode
https://bugs.webkit.org/show_bug.cgi?id=207600

Attachment 398072: Patch

https://bugs.webkit.org/attachment.cgi?id=398072&action=review




--- Comment #72 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 398072
  --> https://bugs.webkit.org/attachment.cgi?id=398072
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398072&action=review

Looks almost ready to land.

> Source/WebCore/ChangeLog:60
> -	   (WebCore::Document::prepareForDestruction):
> +	   (WebCore::Document::willBeRemovedFromFrame):
>	   (WebCore::m_undoManager): Deleted.
> +	   (WebCore::Document::prepareForDestruction): Deleted.
>	   * dom/Document.h:
>	   (WebCore::Document::editor):
>	   (WebCore::Document::editor const):
>	   (WebCore::Document::selection):
>	   (WebCore::Document::selection const):
> -	   * dom/PositionIterator.cpp:
> -	   (WebCore::PositionIterator::isCandidate const):
>	   * editing/AlternativeTextController.cpp:
>	   (WebCore::AlternativeTextController::AlternativeTextController):
>	   (WebCore::AlternativeTextController::stopPendingCorrection):

I think you still need to revert this section, which edits a previous ChangeLog
entry.

> Source/WebCore/ChangeLog:193
>	   (WebCore::Editor::findString):
>	   (WebCore::Editor::countMatchesForText):
>	   (WebCore::Editor::respondToChangedSelection):
> -	   (WebCore::Editor::shouldDetectTelephoneNumbers):
> +	   (WebCore::Editor::shouldDetectTelephoneNumbers const):
>	   (WebCore::Editor::scanSelectionForTelephoneNumbers):
>	   (WebCore::Editor::editorUIUpdateTimerFired):
>	   (WebCore::Editor::selectionStartHasMarkerFor const):
> -	   (WebCore::candidateRangeForSelection):
>	   (WebCore::Editor::stringForCandidateRequest const):
>	   (WebCore::Editor::contextRangeForCandidateRequest const):
>	   (WebCore::Editor::fontAttributesAtSelectionStart const):

Ditto

> Source/WebCore/ChangeLog:229
>	   (WebCore::FrameSelection::modifyMovingRight):
>	   (WebCore::FrameSelection::modifyMovingLeft):
>	   (WebCore::FrameSelection::modify):
> -	   (WebCore::FrameSelection::prepareForDestruction):
> +	   (WebCore::FrameSelection::willBeRemovedFromFrame):
>	   (WebCore::FrameSelection::absoluteCaretBounds):
>	   (WebCore::FrameSelection::recomputeCaretRect):
>	   (WebCore::FrameSelection::contains const):

Ditto

> Source/WebCore/ChangeLog:252
>	   (WebCore::FrameSelection::updateAppearanceAfterLayoutOrStyleChange):
>	   (WebCore::FrameSelection::selectRangeOnElement):
>	   (WebCore::FrameSelection::setCaretBlinks):
> +	   (WebCore::FrameSelection::prepareForDestruction): Deleted.
>	   * editing/FrameSelection.h:
>	   * editing/InsertIntoTextNodeCommand.cpp:
>	   (WebCore::InsertIntoTextNodeCommand::doApply):

Ditto

> Source/WebCore/ChangeLog:300
>	   * editing/TypingCommand.h:
>	   * editing/cocoa/EditorCocoa.mm:
>	   (WebCore::Editor::selectionInHTMLFormat):
> +	   (WebCore::selectionAsAttributedString):
>	   (WebCore::Editor::writeSelectionToPasteboard):
>	   (WebCore::Editor::writeSelection):
>	   (WebCore::Editor::selectionInWebArchiveFormat):

Ditto

> Source/WebCore/ChangeLog:330
>	   * editing/win/EditorWin.cpp:
>	   (WebCore::Editor::pasteWithPasteboard):
>	   (WebCore::Editor::webContentFromPasteboard):
> +	   * history/CachedFrame.cpp:
> +	   (WebCore::CachedFrame::destroy):
>	   * loader/FrameLoader.cpp:
>	   (WebCore::FrameLoader::willTransitionToCommitted):
>	   (WebCore::FrameLoader::closeURL):

Ditto

> Source/WebCore/ChangeLog:339
>	   (WebCore::FrameLoader::clear):
>	   * page/Frame.cpp:
>	   (WebCore::Frame::Frame):
> +	   (WebCore::Frame::setView):
> +	   (WebCore::Frame::setDocument):
>	   (WebCore::Frame::requestDOMPasteAccess):
>	   (WebCore::Frame::setPageAndTextZoomFactors):
>	   * page/Frame.h:

Ditto