Bug 159341 – [GTK] Null WebCore::Range dereference in WebEditorClient::updateGlobalSelection

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Fujii Hironori]

Following tests crash with same callstack:

  editing/input/set-value-on-input-and-delete.html
  editing/selection/selection-in-iframe-removed-crash.html
  imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute.html
  imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/selection-after-content-change.html
  imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/selection-not-application-textarea.html
  imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/selection-not-application.html

Callstack:

> #0  0x00007fbe5f48e11c in WTF::RefPtr<WebCore::Node>::get (this=0x10) at ../../Source/WTF/wtf/RefPtr.h:64
> #1  0x00007fbe5f7f990a in (anonymous namespace)::RangeBoundaryPoint::container (this=0x10)
>     at ../../Source/WebCore/dom/RangeBoundaryPoint.h:83
> #2  0x00007fbe5f7f9928 in (anonymous namespace)::Range::startContainer (this=0x0) at ../../Source/WebCore/dom/Range.h:61
> #3  0x00007fbe6023f956 in (anonymous namespace)::Range::text (this=0x0) at ../../Source/WebCore/dom/Range.cpp:891
> #4  0x00007fbe5f9c0671 in (anonymous namespace)::WebEditorClient::updateGlobalSelection (this=0x5cf840, frame=0x7fbe40da2000)
>     at ../../Source/WebKit2/WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:180
> #5  0x00007fbe5f7cbd8d in (anonymous namespace)::WebEditorClient::respondToChangedSelection (this=0x5cf840, frame=0x7fbe40da2000)
>     at ../../Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp:195
> #6  0x00007fbe602e63a9 in (anonymous namespace)::Editor::respondToChangedSelection (this=0x7fbe40da1000, options=6)
>     at ../../Source/WebCore/editing/Editor.cpp:3320
> #7  0x00007fbe602f7435 in (anonymous namespace)::FrameSelection::setSelectionWithoutUpdatingAppearance (this=0x7fbe40dbf230, 
>     newSelectionPossiblyWithoutDirection=..., options=6, align=(anonymous namespace)::FrameSelection::AlignCursorOnScrollIfNeeded, 
>     granularity=(anonymous namespace)::CharacterGranularity) at ../../Source/WebCore/editing/FrameSelection.cpp:327
> #8  0x00007fbe602f756f in (anonymous namespace)::FrameSelection::setSelection (this=0x7fbe40dbf230, selection=..., options=6, 
>     intent=..., align=(anonymous namespace)::FrameSelection::AlignCursorOnScrollIfNeeded, 
>     granularity=(anonymous namespace)::CharacterGranularity) at ../../Source/WebCore/editing/FrameSelection.cpp:335
> #9  0x00007fbe603003b4 in (anonymous namespace)::FrameSelection::selectFrameElementInParentIfFullySelected (this=0x7fbe40dbfaf0)
>     at ../../Source/WebCore/editing/FrameSelection.cpp:1884
> #10 0x00007fbe602f7407 in (anonymous namespace)::FrameSelection::setSelectionWithoutUpdatingAppearance (this=0x7fbe40dbfaf0, 
>     newSelectionPossiblyWithoutDirection=..., options=6, align=(anonymous namespace)::FrameSelection::AlignCursorOnScrollIfNeeded, 
>     granularity=(anonymous namespace)::CharacterGranularity) at ../../Source/WebCore/editing/FrameSelection.cpp:326
> #11 0x00007fbe602f756f in (anonymous namespace)::FrameSelection::setSelection (this=0x7fbe40dbfaf0, selection=..., options=6, 
>     intent=..., align=(anonymous namespace)::FrameSelection::AlignCursorOnScrollIfNeeded, 
>     granularity=(anonymous namespace)::CharacterGranularity) at ../../Source/WebCore/editing/FrameSelection.cpp:335
> #12 0x00007fbe602f7218 in (anonymous namespace)::FrameSelection::setSelectionWithoutUpdatingAppearance (this=0x7fbe40dbf230, 
>     newSelectionPossiblyWithoutDirection=..., options=6, align=(anonymous namespace)::FrameSelection::AlignCursorOnScrollIfNeeded, 
>     granularity=(anonymous namespace)::CharacterGranularity) at ../../Source/WebCore/editing/FrameSelection.cpp:289
> #13 0x00007fbe602f756f in (anonymous namespace)::FrameSelection::setSelection (this=0x7fbe40dbf230, selection=..., options=6, 
>     intent=..., align=(anonymous namespace)::FrameSelection::AlignCursorOnScrollIfNeeded, 
>     granularity=(anonymous namespace)::CharacterGranularity) at ../../Source/WebCore/editing/FrameSelection.cpp:335
> #14 0x00007fbe602f6123 in (anonymous namespace)::FrameSelection::moveTo (this=0x7fbe40dbf230, range=0x7fbe40d3ec00)
>     at ../../Source/WebCore/editing/FrameSelection.cpp:162
> #15 0x00007fbe607870dd in (anonymous namespace)::DOMSelection::addRange (this=0x7fbe40cd8e60, r=0x7fbe40d3ec00)
>     at ../../Source/WebCore/page/DOMSelection.cpp:383
> #16 0x00007fbe61542db1 in (anonymous namespace)::jsDOMSelectionPrototypeFunctionAddRange (state=0x7ffe9af9e200)
>     at DerivedSources/WebCore/JSDOMSelection.cpp:521
> #17 0x00007fbe00288028 in ?? ()
> #18 0x00007ffe9af9e280 in ?? ()
> #19 0x00007fbe5931e48b in llint_entry () from /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18
> Backtrace stopped: frame did not save the PC

[Fujii Hironori]

  editing/input/set-value-on-input-and-delete.html
  
This test constantly crashs on BuildBot.
But, never on my Linux box.
https://build.webkit.org/TestFailures/ says the same problem.
How to mark such test? Should be marked [ Crash Pass ]?

[Fujii Hironori]

Created attachment 282537 [details]
gardening patch

[Carlos Alberto Lopez Perez]

Comment on attachment 282537 [details]
gardening patch

For gardening patchs there is no review required.

If you are still no committer, then do the following:

Change the "Reviewed by NOBODY (OOPS!)." line with just "Unreviewed." or "Unreviewed gardening." or something that contains the word unreviewed.
Upload the patch again but without asking for review:

Tools/Scripts/webkit-patch upload --request-commit --no-review 

Then any committer can just give cq+ (me for example) and the patch will land.

[Michael Catanzaro]

(In reply to comment #1)
> How to mark such test? Should be marked [ Crash Pass ]?

It should probably be marked [ Crash ], to match the results on the bot.

Unfortunately we haven't figured out what to do when we cannot locally reproduce the results on the bot. In theory, all software that can affect test results should be in the jhbuild environment, so it means some essential library is missing from the jhbuild environment.

[Carlos Alberto Lopez Perez]

(In reply to comment #4)
> (In reply to comment #1)
> > How to mark such test? Should be marked [ Crash Pass ]?
> 
> It should probably be marked [ Crash ], to match the results on the bot.
> 
> Unfortunately we haven't figured out what to do when we cannot locally
> reproduce the results on the bot. In theory, all software that can affect
> test results should be in the jhbuild environment, so it means some
> essential library is missing from the jhbuild environment.

I think some tests give different results depending if you run them alone or if you run the whole test suite.

For example: I can reproduce both the crash and the non-crash on editing/input/set-value-on-input-and-delete.html depending on how I run the tests.

If I run this:

$ Tools/Scripts/run-webkit-tests --release --gtk editing

Then I get a crash on editing/input/set-value-on-input-and-delete.html


However, if I run that test alone:

$ Tools/Scripts/run-webkit-tests --release --gtk editing/input/set-value-on-input-and-delete.html


Then it runs fine.

And (at first sight) it don't seems a race condition because forcing only one worker on the first case don't fixes the issue.

See the output: http://sprunge.us/iEIX 

Can you reproduce that also?

[Fujii Hironori]

Created attachment 282695 [details]
Callstack of editing/input/set-value-on-input-and-delete.html

(In reply to comment #3)
> Tools/Scripts/webkit-patch upload --request-commit --no-review 

I didn't know this switch. Thank you.

(In reply to comment #4)
> It should probably be marked [ Crash ], to match the results on the bot.

OK, thanks.

(In reply to comment #5)
> Can you reproduce that also?

Ah, I can.
Attached the callstack. Looks the same crash.

[Fujii Hironori]

Created attachment 282703 [details]
Patch

This seems a trivial fix. I created a patch instread of updating TestExpectations.

[Carlos Garcia Campos]

Comment on attachment 282703 [details]
Patch

Yes! a fix is always better than updating test expectation. Thanks.

[WebKit Commit Bot]

Comment on attachment 282703 [details]
Patch

Clearing flags on attachment: 282703

Committed r202807: <http://trac.webkit.org/changeset/202807>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.