Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/10a29c5ef01177b72535c855713df141a9ef9ddc
Cr-Commit-Position: refs/heads/master@{#422209}

[bokan, 2016-09-25 14:49:16]

Description was changed from

==========
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
==========

to

==========
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[bokan, 2016-09-25 15:56:36]

Description was changed from

==========
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[bokan, 2016-09-25 15:56:36]

bokan@chromium.org changed reviewers:
+ dtapuska@chromium.org

[bokan, 2016-09-25 15:56:44]

https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h (right):

https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h:50: #include
"core/page/scrolling/StickyPositionScrollingConstraints.h"
Unrelated but I noticed this is needed to include this file in new places. It's
current uses just happen to have it via other includes.

[bokan, 2016-09-25 16:01:36]

The CQ bit was checked by bokan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-25 16:01:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-25 17:16:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-25 17:16:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dtapuska, 2016-09-26 13:51:55]

On 2016/09/25 15:56:44, bokan (OOO Sept 26-28) wrote:
>
https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h (right):
> 
>
https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h:50: #include
> "core/page/scrolling/StickyPositionScrollingConstraints.h"
> Unrelated but I noticed this is needed to include this file in new places.
It's
> current uses just happen to have it via other includes.

seems reasonable. But there seems to be a use after free on the asan bot with a
layout object.

[bokan, 2016-09-26 15:05:06]

On 2016/09/26 13:51:55, dtapuska wrote:
> On 2016/09/25 15:56:44, bokan (OOO Sept 26-28) wrote:
> >
>
https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
(right):
> > 
> >
>
https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h:50: #include
> > "core/page/scrolling/StickyPositionScrollingConstraints.h"
> > Unrelated but I noticed this is needed to include this file in new places.
> It's
> > current uses just happen to have it via other includes.
> 
> seems reasonable. But there seems to be a use after free on the asan bot with
a
> layout object.

Yeah, I'll fix that first

[bokan, 2016-09-29 15:17:02]

On 2016/09/26 15:05:06, bokan (OOO Sept 26-28) wrote:
> On 2016/09/26 13:51:55, dtapuska wrote:
> > On 2016/09/25 15:56:44, bokan (OOO Sept 26-28) wrote:
> > >
> >
>
https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
> (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h:50:
#include
> > > "core/page/scrolling/StickyPositionScrollingConstraints.h"
> > > Unrelated but I noticed this is needed to include this file in new places.
> > It's
> > > current uses just happen to have it via other includes.
> > 
> > seems reasonable. But there seems to be a use after free on the asan bot
with
> a
> > layout object.
> 
> Yeah, I'll fix that first

The issue was that we don't do all the normal tear-down in the case that the
document is being destroyed. Specifically, this meant that we didn't do the
ScrollAnchor removal so the ScrollAnchor kept it's pointer to a deleted
LayoutObject. Recomputing the root scroller means we reassign ScrollableAreas to
the ScrollAnchor which causes it to touch its dead pointer. I've avoided
recomputing the root scroller if we're destroying the document. ptal.

[bokan, 2016-09-30 16:26:11]

ping

[dtapuska, 2016-09-30 16:29:20]

lgtm

[bokan, 2016-09-30 16:29:41]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2016-09-30 16:30:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 19:05:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 19:05:44]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[bokan, 2016-09-30 19:06:14]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2016-09-30 19:06:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 20:35:33]

Description was changed from

==========
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2016-09-30 20:35:34]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-30 20:40:28]

Description was changed from

==========
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.

With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).

This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.

BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/10a29c5ef01177b72535c855713df141a9ef9ddc
Cr-Commit-Position: refs/heads/master@{#422209}
==========

[commit-bot: I haz the power, 2016-09-30 20:40:29]

Patchset 4 (id:??) landed as
https://crrev.com/10a29c5ef01177b72535c855713df141a9ef9ddc
Cr-Commit-Position: refs/heads/master@{#422209}