NULL ptr in WebCore::Range::processAncestorsAndTheirSiblings

[Berend-Jan Wever]

Chromium: http://code.google.com/p/chromium/issues/detail?id=112427
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=17443217

Fuzzer: Bj_doc_fuzzer

Crash Type: UNKNOWN
Crash Address: 0x000000000026
Crash State:
  - crash stack -
  WebCore::Range::processAncestorsAndTheirSiblings
  WebCore::Range::processContents
  WebCore::Range::deleteContents
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=108839:108881

Minimized Testcase (0.97 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95ceK8J7bne4cv4iUyHvS6W5Dm-Lg_ZCryRU2Y42ayuWg6nWS3qKv0A1aj2WSdRHboY3FKGUuNs6YNK6wPyxBjL16K7Ni7ew9n6iM_dyvpEhsjBEWjBJsxY6X5xq2-j2qmgBeDqaRfLk17tLBcPV45QJChXEQ

[Ryosuke Niwa]

Created attachment 125099 [details]
reduction

[Yuta Kitamura]

Created attachment 206046 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 206046 [details]
Patch

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

> Source/WebCore/ChangeLog:15
> +        of the range since mutation observers can change the state of the range.

You mean mutation events?  Mutation observes are never called synchronously.

> Source/WebCore/dom/Range.cpp:755
> +    originalStart.clear();
> +    originalEnd.clear();
> +

It doesn't seem right/necessary to clear these ranges here.

> LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml:8
> +var f = function ()

Why don't we just do:
function f()
instead?
Also, please give it a more descriptive name.

> LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml:16
> +var g = function ()
> +{

Ditto.

[Yuta Kitamura]

Created attachment 206293 [details]
Patch v2

[Yuta Kitamura]

Comment on attachment 206046 [details]
Patch

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

>> Source/WebCore/ChangeLog:15
>> +        of the range since mutation observers can change the state of the range.
> 
> You mean mutation events?  Mutation observes are never called synchronously.

You are right. Fixed.

>> Source/WebCore/dom/Range.cpp:755
>> +
> 
> It doesn't seem right/necessary to clear these ranges here.

Removed.

>> LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml:8
>> +var f = function ()
> 
> Why don't we just do:
> function f()
> instead?
> Also, please give it a more descriptive name.

Fixed.

>> LayoutTests/fast/dom/Range/resources/detach-range-during-deletecontents-iframe.xhtml:16
>> +{
> 
> Ditto.

Fixed.

[Ryosuke Niwa]

Could you land a patch for this?

[Ryosuke Niwa]

The patch got landed in https://chromium.googlesource.com/chromium/blink/+/0c1f35bf0bd7df75db2b862efc883b1ae418c453 for Blink.

[Yuta Kitamura]

Sorry for delay, I just cq+'ed.

[WebKit Commit Bot]

Comment on attachment 206293 [details]
Patch v2

Clearing flags on attachment: 206293

Committed r152707: <http://trac.webkit.org/changeset/152707>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.