1269047 - 3,800 instances of NS_ENSURE_SUCCESS(res, res), NS_ENSURE_SUCCESS(rv, rv), NS_ENSURE_TRUE(aSelection->RangeCount()) emitted during linux64 debug testing

Status: RESOLVED FIXED

(Core :: Editor, defect)

Description

[Eric Rahm (on leave)]

These 3 warnings are most likely chained.

> 1261 WARNING: NS_ENSURE_SUCCESS(res, res) failed with result 0x80004005: file editor/libeditor/nsHTMLEditRules.cpp, line 8116
> 1261 WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file editor/libeditor/nsEditor.cpp, line 3681
> 1261 WARNING: NS_ENSURE_TRUE(aSelection->RangeCount()) failed: file editor/libeditor/nsEditor.cpp, line 3702

These warnings [1,2,3] shows up in the following test suites:

>    278 - [TC] Linux64 crashtest C
>    211 - [TC] Linux64 crashtest e10s C
>    171 - [TC] Linux64 mochitest-8 8
>    171 - [TC] Linux64 mochitest-plain-e10s-8 8
>     57 - [TC] Linux64 mochitest-jetpack JP
>     46 - [TC] Linux64 reftest-8 R8
>     46 - [TC] Linux64 reftest-e10s-5 R-e10s5
>     34 - [TC] Linux64 web-platform-tests-2 2
>     32 - [TC] Linux64 web-platform-tests-e10s-2 2
>     23 - [TC] Linux64 mochitest-plain-e10s-3 3
>     22 - [TC] Linux64 mochitest-a11y a11y
>     22 - [TC] Linux64 reftest-e10s-2 R-e10s2
>     22 - [TC] Linux64 mochitest-3 3
>     19 - [TC] Linux64 reftest-3 R3
>     17 - [TC] Linux64 mochitest-chrome-2 c2
>     13 - [TC] Linux64 mochitest-chrome-3 c3
>     12 - [TC] Linux64 mochitest-2 2
>      7 - [TC] Linux64 mochitest-9 9
>      7 - [TC] Linux64 mochitest-plain-e10s-9 9
>      6 - [TC] Linux64 mochitest-plain-e10s-2 2
>      4 - [TC] Linux64 mochitest-browser-chrome-e10s-6 bc6
>      4 - [TC] Linux64 reftest-4 R4
>      4 - [TC] Linux64 mochitest-browser-chrome-2 bc2
>      4 - [TC] Linux64 mochitest-4 4
>      4 - [TC] Linux64 reftest-e10s-3 R-e10s3
>      3 - [TC] Linux64 reftest-2 R2
>      2 - [TC] Linux64 web-platform-tests-4 4
>      2 - [TC] Linux64 mochitest-browser-chrome-e10s-1 bc1
>      2 - [TC] Linux64 mochitest-browser-chrome-1 bc1
>      2 - [TC] Linux64 mochitest-plain-e10s-6 6
>      2 - [TC] Linux64 web-platform-tests-e10s-4 4
>      2 - [TC] Linux64 mochitest-browser-chrome-e10s-7 bc7
>      2 - [TC] Linux64 mochitest-6 6
>      2 - [TC] Linux64 mochitest-browser-chrome-7 bc7
>      2 - [TC] Linux64 mochitest-chrome-1 c1
>      1 - [TC] Linux64 mochitest-1 1
>      1 - [TC] Linux64 web-platform-tests-e10s-3 3
>      1 - [TC] Linux64 web-platform-tests-3 3
>      1 - [TC] Linux64 mochitest-plain-e10s-1 1

They shows up in 706 tests. A few of the most prevalent:

>     98 -        file:///home/worker/workspace/build/tests/reftest/tests/dom/base/crashtests/561981-2.html
>     66 -        layout/base/tests/test_reftests_with_caret.html
>     66 - [e10s] layout/base/tests/test_reftests_with_caret.html
>     52 - [e10s] file:///home/worker/workspace/build/tests/reftest/tests/dom/base/crashtests/561981-2.html
>     47 -        file:///home/worker/workspace/build/tests/reftest/tests/editor/libeditor/crashtests/759748.html
>     47 - [e10s] file:///home/worker/workspace/build/tests/reftest/tests/editor/libeditor/crashtests/759748.html
>     39 -        file:///home/worker/workspace/build/tests/reftest/tests/dom/base/crashtests/561981-1.html
>     18 - [e10s] file:///home/worker/workspace/build/tests/reftest/tests/dom/base/crashtests/561981-1.html
>      9 - [e10s] editor/libeditor/tests/test_bug611182.html
>      9 -        editor/libeditor/tests/test_bug611182.html

[1] https://hg.mozilla.org/mozilla-central/annotate/8c3fd523d75b/editor/libeditor/nsHTMLEditRules.cpp#l8116
[2] https://hg.mozilla.org/mozilla-central/annotate/8c3fd523d75b/editor/libeditor/nsEditor.cpp#l3681
[3] https://hg.mozilla.org/mozilla-central/annotate/8c3fd523d75b/editor/libeditor/nsEditor.cpp#l3702

Comment 1

[Eric Rahm (on leave)]

Definitely chained:

> |nsHTMLEditRules::ConfirmSelectionInBody| ->
> |nsEditor::GetStartNodeAndOffset(Selection* aSelection, nsIDOMNode **outStartNode, int32_t *outStartOffset)| ->
> ||nsEditor::GetStartNodeAndOffset(Selection* aSelection, nsINode** aStartNode, int32_t* aStartOffset)

Comment 2

[Eric Rahm (on leave)]

Attachment: Remove chained ENSURE calls from GetStartNodeAndOffset

Ehsan, this is another rather spammy chain in editor. I'm not sure if it's indicating an unexpected situation or if we were just using NS_ENSURE as a quick exit. This patch just converts the NS_ENSURE calls to ifs.

Comment 3

[Eric Rahm (on leave)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2d39735ca01cdf4522fd184f0b7ac13ac2111b8
Bug 1269047 - Remove chained ENSURE calls from GetStartNodeAndOffset. r=ehsan

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e2d39735ca01