commit 31a914fc4b1196f0ac0f59b925165bfa018d752c
author Takashi Sakamoto <tasak@google.com> Wed May 08 02:21:36 2019
committer Commit Bot <commit-bot@chromium.org> Wed May 08 02:21:36 2019
tree a6a482a164d7ae1e456505434ac4a4b7b8757538
parent 426d8d1eaf92d4de74fdad475275119677dfb5a8

Revert "Correctly handle scroll-snap-type changes to 'none'"

This reverts commit 712c3cf3ed8201420acf23f760eaa34be20781cd.

Reason for revert: This patch causes webkit-layout-tests failure on WebKit_Linux_Trusty_ASAN bot:
https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20Trusty%20ASAN/25720

Unexpected Failures:
* external/wpt/css/css-scroll-snap/scroll-snap-type.html
* virtual/threaded/external/wpt/css/css-scroll-snap/scroll-snap-type.html

STDERR: ==1==ERROR: AddressSanitizer: heap-use-after-free on address 0x61200023f8d8 at pc 0x5620c924e56d bp 0x7ffde3c56830 sp 0x7ffde3c56828
STDERR: READ of size 8 at 0x61200023f8d8 thread T0 (content_shell)
STDERR:     #0 0x5620c924e56c in get ./../../base/memory/scoped_refptr.h:212:27
STDERR:     #1 0x5620c924e56c in Style ./../../third_party/blink/renderer/core/layout/layout_object.h:1615:0
STDERR:     #2 0x5620c924e56c in GetPhysicalSnapType ./../../third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc:88:0
STDERR:     #3 0x5620c924e56c in blink::SnapCoordinator::UpdateSnapContainerData(blink::LayoutBox&) ./../../third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc:107:0
STDERR:     #4 0x5620c924e74b in blink::SnapCoordinator::UpdateAllSnapContainerData() ./../../third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc:76:5



Original change's description:
> Correctly handle scroll-snap-type changes to 'none'
> 
> 
> Previously when a scroll container's snap type is changed to 'none' its
> data was discarded including all of its snap areas. However this is
> incorrect. Because while the snap type is 'none', the element is still
> a scroll container which per spec [1] means  that is should continue to
> captures the snap areas in its subtree for whom it is the nearest
> ancestor scroll container . The only difference is that it no longer
> snaps.
> 
> The fix is that we no longer remove the snap container data just
> because is has a 'none' snap type and instead keep it and its snap
> areas. But we check the snap type before performing any snap.
> 
> To ensure this does not introduce any performance regression, this CL
> also includes an optimization where we avoid re-calculating
> snap_container_data when the snap type is 'none'. So keeping these snap
> data should not be cheap.
> 
> Note that there is another problem where if the current snap container
> is no longer a scroll container (e.g., overflow: scroll => overflow:
> visible) we release its snap areas and they become "orphan". But if we
> are to do this correctly, we should re-assign these areas to the next
> stroller in the chain. Similarly when an element becomes a scroll
> container, it can potentially take over snap areas from its parent snap
> container.
> 
> 
> This patch does not address that situation yet but fixes the easier
> problem.
> 
> [1] https://drafts.csswg.org/css-scroll-snap/#overview
> 
> Bug: 953575
> Test:
>  - wpt/css/css-scroll-snap/scroll-snap-type-change.html => Changing snap-type should work correctly
>  - wpt/css/css-scroll-snap/scroll-snap-type.html => Add a specific test for type 'none' to ensure it does not snap
> 
> Change-Id: Ie493ad68ecba818ed41c0ee103ccf44725ff6e3f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1589899
> Reviewed-by: Majid Valipour <majidvp@chromium.org>
> Reviewed-by: David Bokan <bokan@chromium.org>
> Commit-Queue: Majid Valipour <majidvp@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#657460}

TBR=bokan@chromium.org,majidvp@chromium.org

Change-Id: I3a327f6e342e95d045194d24ceaf49de52b2b921
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 953575
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1600437
Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#657571}