[webkit-reviews] review granted: [Bug 195845] UI-process hit-testing needs to know about containing block relationships : [Attachment 365540] patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Antti Koivisto
<koivisto at iki.fi>'s request for review:
Bug 195845: UI-process hit-testing needs to know about containing block
relationships
https://bugs.webkit.org/show_bug.cgi?id=195845

Attachment 365540: patch

https://bugs.webkit.org/attachment.cgi?id=365540&action=review




--- Comment #14 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 365540
  --> https://bugs.webkit.org/attachment.cgi?id=365540
patch

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

> Source/WebKit/ChangeLog:11
> +	   When an overflow scroller contains a positioned element it may not
be a descendant layer of the scroller,

The "it" is ambiguous.

>
Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:83
> -    std::unique_ptr<ScrollingStateTree>
stateTree(const_cast<RemoteScrollingCoordinatorTransaction&>(transaction).scrol
lingStateTree().release());
> +    auto stateTree =
WTFMove(const_cast<RemoteScrollingCoordinatorTransaction&>(transaction).scrolli
ngStateTree());

Haha

>
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.
mm:133
> +    // FIXME: This doesn't contain ScrollPositioningBehavior::Stationary
nodes. They may need to be handled too.

File bug for this, reference it here.

>
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.
mm:139
> +	       auto* positionedLayerNode =
RemoteLayerTreeNode::forCALayer(positionedNode->layer());

Maybe null-check positionedNode, since we've had other bugs where stale nodeIDs
get left around.