Bug 131982

Summary:

REGRESSION (r167652): Broke fast/regions/cssom/region-range-for-box-crash.html in debug mode

Product:

WebKit

Reporter:

Manuel Rego Casasnovas <rego>

Component:

CSS

Assignee:

Manuel Rego Casasnovas <rego>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abucur, buildbot, commit-queue, enrica, esprehn+autocc, glenn, kondapallykalyan, mihnea, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion	none

Manuel Rego Casasnovas

Reported 2014-04-22 04:55:51 PDT

Patch for bug #131511 which landed in r167652 broke fast/regions/cssom/region-range-for-box-crash.html in debug mode. 04:21:37.557 30890 ASSERTION FAILED: !m_code || m_code == defaultExceptionCode 04:21:37.557 30890 /Volumes/Data/slave/mavericks-debug/build/Source/WebCore/dom/Range.h(81) : 04:21:37.557 30890 1 0x1015fd0a0 WTFCrash 04:21:37.557 30890 2 0x1034f8393 WebCore::NoExceptionAssertionChecker::~NoExceptionAssertionChecker() 04:21:37.557 30890 3 0x1034f8335 WebCore::NoExceptionAssertionChecker::~NoExceptionAssertionChecker() 04:21:37.557 30890 4 0x1044a6d5b WebCore::Range::Range(WebCore::Document&, WTF::PassRefPtr<WebCore::Node>, int, WTF::PassRefPtr<WebCore::Node>, int) 04:21:37.557 30890 5 0x1044a541b WebCore::Range::Range(WebCore::Document&, WTF::PassRefPtr<WebCore::Node>, int, WTF::PassRefPtr<WebCore::Node>, int) 04:21:37.557 30890 6 0x10449c6f3 WebCore::Range::create(WebCore::Document&, WTF::PassRefPtr<WebCore::Node>, int, WTF::PassRefPtr<WebCore::Node>, int) 04:21:37.557 30890 7 0x1047d64a8 WebCore::RenderView::splitSelectionBetweenSubtrees(WebCore::RenderObject*, int, WebCore::RenderObject*, int, WebCore::RenderView::SelectionRepaintMode) 04:21:37.557 30890 8 0x1047d4ce1 WebCore::RenderView::setSelection(WebCore::RenderObject*, int, WebCore::RenderObject*, int, WebCore::RenderView::SelectionRepaintMode) 04:21:37.557 30890 9 0x103675bb4 WebCore::FrameSelection::updateAppearance() 04:21:37.557 30890 10 0x103675437 WebCore::FrameSelection::updateAndRevealSelection() 04:21:37.557 30890 11 0x103673aa4 WebCore::FrameSelection::setSelection(WebCore::VisibleSelection const&, unsigned int, WebCore::FrameSelection::CursorAlignOnScroll, WebCore::TextGranularity) 04:21:37.557 30890 12 0x10367cf4c WebCore::FrameSelection::selectAll() 04:21:37.557 30890 13 0x10348ae78 WebCore::executeSelectAll(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) 04:21:37.557 30890 14 0x103486cf5 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 04:21:37.558 30890 15 0x10331ea3e WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) 04:21:37.558 30890 16 0x103c2cd0a WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::ExecState*) 04:21:37.558 30890 17 0x52068c001034 04:21:37.558 30890 18 0x10140b648 llint_entry 04:21:37.558 30890 19 0x101404da4 callToJavaScript 04:21:37.558 30890 20 0x10129efbd JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 04:21:37.558 30890 21 0x101283929 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 04:21:37.558 30890 22 0x100f5820e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 04:21:37.558 30890 23 0x100f58273 JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, JSC::JSValue*) 04:21:37.558 30890 24 0x103b8ae7b WebCore::JSMainThreadExecState::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, JSC::JSValue*) 04:21:37.558 30890 25 0x103d249a8 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) 04:21:37.558 30890 26 0x1034f61df WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul, WTF::CrashOnOverflow>&) 04:21:37.558 30890 27 0x1034f5aae WebCore::EventTarget::fireEventListeners(WebCore::Event*) 04:21:37.558 30890 28 0x103422d0b WebCore::DOMWindow::dispatchEvent(WTF::PassRefPtr<WebCore::Event>, WTF::PassRefPtr<WebCore::EventTarget>) 04:21:37.558 30890 29 0x103429db8 WebCore::DOMWindow::dispatchLoadEvent() 04:21:37.558 30890 30 0x1033166ad WebCore::Document::dispatchWindowLoadEvent() 04:21:37.558 30890 31 0x103313ad3 WebCore::Document::implicitClose()

Attachments
Patch (3.14 KB, patch) 2014-04-22 09:30 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch (3.16 KB, patch) 2014-05-07 03:05 PDT, Manuel Rego Casasnovas	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (537.79 KB, application/zip) 2014-05-07 04:05 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (554.35 KB, application/zip) 2014-05-07 04:27 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (581.94 KB, application/zip) 2014-05-07 05:35 PDT, Build Bot	no flags	Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Manuel Rego Casasnovas

Comment 1 2014-04-22 05:01:33 PDT

Updated test expectations in r167655.

Manuel Rego Casasnovas

Comment 2 2014-04-22 09:30:16 PDT

After debugging this for a while the problem is in RenderView::splitSelectionBetweenSubtrees() when we try to create the |intialRange|: RefPtr<Range> initialRange = Range::create(document(), start->node(), startPos, end->node(), endPos); Basically, Range::create(), calls Range::setEnd() which ends up calling Range::checkNodeWOffset() which returns an error because of the element has not children: case Node::ELEMENT_NODE: case Node::ENTITY_REFERENCE_NODE: case Node::XPATH_NAMESPACE_NODE: { if (!offset) return 0; Node* childBefore = n->childNode(offset - 1); if (!childBefore) ec = INDEX_SIZE_ERR; return childBefore; } This is because of in FrameSelection we have: view->setSelection(startRenderer, startPos.deprecatedEditingOffset(), endRenderer, endPos.deprecatedEditingOffset()); And Position::deprecatedEditingOffset() is calling lastOffsetForEditing() which is returning 1 for the <input> as explained in the following comment: // This method is used to create positions in the DOM. It returns the maximum valid offset // in a node. It returns 1 for some elements even though they do not have children, which // creates technically invalid DOM Positions. Be sure to call parentAnchoredEquivalent // on a Position before using it to create a DOM Range, or an exception will be thrown. int lastOffsetForEditing(const Node* node) As we're only interested in the nodes for the |initialRange| we can avoid the positions and the issue seems to be fixed.

Manuel Rego Casasnovas

Comment 3 2014-04-22 09:30:50 PDT

Created attachment 229888 [details] Patch

Dave Hyatt

Comment 4 2014-04-22 12:41:22 PDT

Comment on attachment 229888 [details] Patch r=me

Manuel Rego Casasnovas

Comment 5 2014-04-22 12:48:59 PDT

Comment on attachment 229888 [details] Patch Clearing flags on attachment: 229888 Committed r167675: <http://trac.webkit.org/changeset/167675>

Manuel Rego Casasnovas

Comment 6 2014-04-22 12:49:06 PDT

All reviewed patches have been landed. Closing bug.

Manuel Rego Casasnovas

Comment 7 2014-05-07 02:45:59 PDT

Reopen bug, as the fix was not right. It was just hiding the issue due to the wrong !renderer->canBeSelectionLeaf() a few lines below, which has been fixed in bug #132493. We need to find a proper solution for this issue. The problem here is that we're not able to create valid DOM ranges from the positions we receive in RenderView::setSelection(). As explained in comment #2, this is because of we're getting 1 as |endPos| for elements that don't have children. We have to be able to create a DOM range from the positions we receive in RenderView::setSelection() from FrameSelection::updateAppearance().

Manuel Rego Casasnovas

Comment 8 2014-05-07 03:05:40 PDT

Created attachment 230982 [details] Patch This could be a possible solution but it's introducing regressions in other tests in debug mode. Maybe we should try to create a valid Position just before creating the DOM range, but I'm still not sure if that would fix or not the problem.

Build Bot

Comment 9 2014-05-07 04:05:44 PDT

Comment on attachment 230982 [details] Patch Attachment 230982 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5698576785080320 New failing tests: svg/text/non-bmp-positioning-lists.svg fast/repaint/selection-gap-transformed-fixed-child.html editing/selection/contains-node-crash.html editing/execCommand/format-block-at-root.html fast/repaint/selection-gap-flipped-absolute-child.html fast/repaint/selection-gap-absolute-child.html fast/repaint/selection-gap-transformed-absolute-child.html

Build Bot

Comment 10 2014-05-07 04:05:47 PDT

Created attachment 230987 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Build Bot

Comment 11 2014-05-07 04:27:33 PDT

Comment on attachment 230982 [details] Patch Attachment 230982 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5400720803102720 New failing tests: svg/text/non-bmp-positioning-lists.svg fast/repaint/selection-gap-transformed-fixed-child.html editing/selection/contains-node-crash.html editing/execCommand/format-block-at-root.html fast/repaint/selection-gap-flipped-absolute-child.html fast/repaint/selection-gap-flipped-fixed-child.html fast/repaint/selection-gap-fixed-child.html fast/repaint/selection-gap-absolute-child.html fast/repaint/selection-gap-transformed-absolute-child.html

Build Bot

Comment 12 2014-05-07 04:27:37 PDT

Created attachment 230988 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Manuel Rego Casasnovas

Comment 13 2014-05-07 04:35:04 PDT

Just as a wild idea, we could probably try to avoid create DOM ranges in RenderView. Instead of some kind of structure with the data we need for the selection: { RenderObject* start; int startPos; RenderObject* end; int endPost; } This would probably fix the kind of problems we're having here.

Build Bot

Comment 14 2014-05-07 05:35:28 PDT

Comment on attachment 230982 [details] Patch Attachment 230982 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6208198277070848 New failing tests: fast/repaint/selection-gap-transformed-fixed-child.html editing/selection/contains-node-crash.html editing/execCommand/format-block-at-root.html fast/repaint/selection-gap-flipped-absolute-child.html fast/repaint/selection-gap-flipped-fixed-child.html fast/repaint/selection-gap-fixed-child.html fast/repaint/selection-gap-absolute-child.html fast/repaint/selection-gap-transformed-absolute-child.html

Build Bot

Comment 15 2014-05-07 05:35:31 PDT

Created attachment 230995 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Manuel Rego Casasnovas

Comment 16 2014-05-20 04:03:23 PDT

This has been fixed in r169105 (bug #132720).

Alexey Proskuryakov

Comment 17 2014-05-20 10:11:45 PDT

The test still crashes, filed bug 133124.

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