robhogan@, could you please take a look or suggest another owner?

We need a solution to the layout object getting nuked while we still have a pointer to it but I notice WeakPtr isn't used at all in layout/ and I'm guessing there's a performance reason for that? Will Oilpan solve this sort of thing in future? addChild() is such a dangerous footgun that it seems like there must be a way of alerting callers to the fact that it can make the parent and even some of its siblings go away, so be wary of keeping pointers to them.

The only thing I'm aware of is  bug 417556 , which is about addChild() being too complicated. No activity there, though.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/741853f64fe49635e5b02b641b3799bdeacec37f

commit 741853f64fe49635e5b02b641b3799bdeacec37f
Author: robhogan <robhogan@gmail.com>
Date: Tue Apr 12 22:06:22 2016

parent->addChild() can delete parent and any of its siblings

Once you call parent->addChild() your pointers to parent and any floating or
out-of-flow siblings it has are no longer safe.

BUG= 602271 

Review URL: https://codereview.chromium.org/1879553003

Cr-Commit-Position: refs/heads/master@{#386827}

[modify] https://crrev.com/741853f64fe49635e5b02b641b3799bdeacec37f/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/741853f64fe49635e5b02b641b3799bdeacec37f/third_party/WebKit/LayoutTests/fast/lists/remove-listmarker-and-make-anonblock-empty-2-expected.txt
[add] https://crrev.com/741853f64fe49635e5b02b641b3799bdeacec37f/third_party/WebKit/LayoutTests/fast/lists/remove-listmarker-and-make-anonblock-empty-2.html
[modify] https://crrev.com/741853f64fe49635e5b02b641b3799bdeacec37f/third_party/WebKit/LayoutTests/platform/linux/editing/execCommand/create-list-with-hr-expected.png
[modify] https://crrev.com/741853f64fe49635e5b02b641b3799bdeacec37f/third_party/WebKit/LayoutTests/platform/linux/editing/execCommand/create-list-with-hr-expected.txt
[modify] https://crrev.com/741853f64fe49635e5b02b641b3799bdeacec37f/third_party/WebKit/Source/core/layout/LayoutListItem.cpp

ClusterFuzz has detected this issue as fixed in range 386714:386876.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5979771996995584

Fuzzer: ochang_domfuzzer
Job Type: linux_asan_chrome_v8_arm
Platform Id: linux

Crash Type: Heap-use-after-free READ 4
Crash Address: 0xa6611d80
Crash State:
  blink::LayoutListItem::updateMarkerLocation
  blink::LayoutListItem::subtreeDidChange
  blink::LayoutObject::handleSubtreeModifications
  
Recommended Security Severity: High

Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_v8_arm&range=384665:385240
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_v8_arm&range=386714:386876

Minimized Testcase (0.44 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97X98n_PtyjJsg85d-L6HPL3SZNLgYr6RrSBjpFNDo_SUDMM49p0pN4Ohv9SOhoPTzzvl-YdtrohkCL9jaxeaqs8bn5hy0TTqz857SedFulaTtSKnywk_lgrkYEGeOBU8rbMvUznPXQPVEc6EUC2ziiwL1FPA

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges.

- Your friendly ClusterFuzz

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bdaff5b6a0ca169b95cedd9d3ba0b4e5f971ff4a

commit bdaff5b6a0ca169b95cedd9d3ba0b4e5f971ff4a
Author: Robert Hogan <robhogan@gmail.com>
Date: Wed Apr 13 18:31:10 2016

parent->addChild() can delete parent and any of its siblings

Once you call parent->addChild() your pointers to parent and any floating or
out-of-flow siblings it has are no longer safe.

BUG= 602271 

Review URL: https://codereview.chromium.org/1879553003

Cr-Commit-Position: refs/heads/master@{#386827}
(cherry picked from commit 741853f64fe49635e5b02b641b3799bdeacec37f)

Review URL: https://codereview.chromium.org/1886763003 .

Cr-Commit-Position: refs/branch-heads/2704@{#30}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/bdaff5b6a0ca169b95cedd9d3ba0b4e5f971ff4a/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/bdaff5b6a0ca169b95cedd9d3ba0b4e5f971ff4a/third_party/WebKit/LayoutTests/fast/lists/remove-listmarker-and-make-anonblock-empty-2-expected.txt
[add] https://crrev.com/bdaff5b6a0ca169b95cedd9d3ba0b4e5f971ff4a/third_party/WebKit/LayoutTests/fast/lists/remove-listmarker-and-make-anonblock-empty-2.html
[modify] https://crrev.com/bdaff5b6a0ca169b95cedd9d3ba0b4e5f971ff4a/third_party/WebKit/LayoutTests/platform/linux/editing/execCommand/create-list-with-hr-expected.png
[modify] https://crrev.com/bdaff5b6a0ca169b95cedd9d3ba0b4e5f971ff4a/third_party/WebKit/LayoutTests/platform/linux/editing/execCommand/create-list-with-hr-expected.txt
[modify] https://crrev.com/bdaff5b6a0ca169b95cedd9d3ba0b4e5f971ff4a/third_party/WebKit/Source/core/layout/LayoutListItem.cpp

Your change meets the bar and is auto-approved for M51 (branch: 2704)

Seems like this was already merged to M51 branch 2704 @ comment #10. So removing "Merge-Approved-51". Next time, please wait for approval.

Will do. Thanks.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot