Bug 63908 - chrome.dll!WebCore::firstPositionInNode ReadAV@NULL (b1c286808dfae0c5027c71627f71b962)
: chrome.dll!WebCore::firstPositionInNode ReadAV@NULL (b1c286808dfae0c5027c7162...
Status: RESOLVED FIXED
: WebKit
HTML DOM
: 528+ (Nightly build)
: PC Windows Vista
: P1 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-07-04 07:50 PST by
Modified: 2011-07-20 13:30 PST (History)


Attachments
Repro (202 bytes, text/html)
2011-07-04 07:50 PST, Berend-Jan Wever
no flags Details
Proposed patch. (3.39 KB, patch)
2011-07-20 12:01 PST, Kulanthaivel Palanichamy
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-07-04 07:50:28 PST
Created an attachment (id=99631) [details]
Repro

Chromium: https://code.google.com/p/chromium/issues/detail?id=88389

Repro:
<script>
  var oSelection = window.getSelection();
  document.removeChild(document.documentElement);
  oSelection.collapse(document);
  oSelection.modify("move","backward","documentboundary");
</script>

id:             chrome.dll!WebCore::firstPositionInNode ReadAV@NULL (b1c286808dfae0c5027c71627f71b962)
description:    Attempt to read from unallocated NULL pointer+0x24 in chrome.dll!WebCore::firstPositionInNode
stack:          chrome.dll!WebCore::firstPositionInNode
                chrome.dll!WebCore::startOfDocument
                chrome.dll!WebCore::startOfDocument
                chrome.dll!WebCore::FrameSelection::modifyMovingBackward
                chrome.dll!WebCore::FrameSelection::modify
                chrome.dll!WebCore::DOMSelection::modify
                chrome.dll!WebCore::DOMSelectionInternal::modifyCallback
                ...

// 1) In the repro, the HTML element has been removed from the document.
// 2) Next, the entire document is selected and moved, causing the code to try to determine the start of the document:
// 3) startOfDocument assumes a document ALWAYS has a documentElement in webkit\source\webcore\editing\visible_units.cpp:989:
VisiblePosition startOfDocument(const Node* node)
{
    if (!node)
        return VisiblePosition();

    return VisiblePosition(firstPositionInNode(node->document()->documentElement()), DOWNSTREAM);
}
// 4) anchorNode is NULL in webkit\source\webcore\dom\position.h:270, leading to the crash:
inline Position firstPositionInNode(Node* anchorNode)
{
    if (anchorNode->isTextNode())
        return Position(anchorNode, 0, Position::PositionIsOffsetInAnchor);
    return Position(anchorNode, Position::PositionIsBeforeChildren);
}
------- Comment #1 From 2011-07-20 12:01:30 PST -------
Created an attachment (id=101491) [details]
Proposed patch.
------- Comment #2 From 2011-07-20 12:23:35 PST -------
(From update of attachment 101491 [details])
I wonder if the !documentElement() part of the check is necessary.  Does firstPositionInNode() handle NULL?
------- Comment #3 From 2011-07-20 12:28:16 PST -------
(In reply to comment #2)
> (From update of attachment 101491 [details] [details])
> I wonder if the !documentElement() part of the check is necessary.

Yes. because

> Does firstPositionInNode() handle NULL?

No.
------- Comment #4 From 2011-07-20 13:30:44 PST -------
(From update of attachment 101491 [details])
Clearing flags on attachment: 101491

Committed r91395: <http://trac.webkit.org/changeset/91395>
------- Comment #5 From 2011-07-20 13:30:48 PST -------
All reviewed patches have been landed.  Closing bug.