[webkit-changes] [WebKit/WebKit] ab4981: Crash in RemoteLayerTreeEventDispatcherDisplayLink...

[Vitor Roriz]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: ab4981648cdb3f161ae8fcbd4cee84918ea94e16
      https://github.com/WebKit/WebKit/commit/ab4981648cdb3f161ae8fcbd4cee84918ea94e16
  Author: Vitor Roriz <vitor.roriz at apple.com>
  Date:   2023-06-27 (Tue, 27 Jun 2023)

  Changed paths:
    M Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h
    M Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeDrawingAreaProxyMac.h
    M Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeDrawingAreaProxyMac.mm
    M Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.cpp
    M Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h
    M Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingCoordinatorProxyMac.h
    M Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingCoordinatorProxyMac.mm

  Log Message:
  -----------
  Crash in RemoteLayerTreeEventDispatcherDisplayLinkClient teardown
https://bugs.webkit.org/show_bug.cgi?id=258133
rdar://109463023

Reviewed by Simon Fraser.

RemoteLayerTreeEventDispatcherDisplayLinkClient inherits indirectly from
CanMakeCheckedPtrBase. This means that for deleting a object of this type
we should first delete any CheckRef objects pointing to it.

At RemoteLayerTreeEventDispatcher::invalidate we currently call
stopDisplayLinkObserver() to remove the associated observer of
DisplayLink::Client. If that Client has no more observers, we remove
the CheckRef for this client from DisplayLink's m_client's map (See removeInfoForClientIfUnused()).

The problem is, since we want to delete m_displayClient at the end of
invalidate() we have to make sure that the associated CheckRef gets removed
from the map, independently of how many observers it still has.
Therefore, instead of removing just the single associated observer,
we can remove the reference for the client from DisplayLink.

The problem is: When RemoteLayerTreeDrawingAreaProxyMac::windowsCreenDidChange()
is called, we will end up having 2 DisplayLink objects and
both of them, hold a CheckedRef pointing to the same DisplayLink::Client.
So in teardown, when we invalidate the RemoteLayerTreeEventDispatcher,
even if we remove the DisplayLink::Client from the DisplayLink for that
dispatcher, the other DisplayLink will still have a CheckedRef for the
same client. Therefore, the m_count won't be zero when we are deleting
DisplayLink::Client.

Solution: When, for some reason, a DisplayLink::Client is associated
with another DisplayLink, we should remove its CheckedRef from the
original DisplayLink referencing it. We do that by creating a new function
windowScreenWillChange() to be called before the displayID is updated.
On the dispatcher, this function will remove the DisplayLink::Client reference
from the old DisplayLink object m_client's map.

* Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h:
(WebKit::RemoteScrollingCoordinatorProxy::windowScreenWillChange):
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeDrawingAreaProxyMac.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeDrawingAreaProxyMac.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxyMac::~RemoteLayerTreeDrawingAreaProxyMac):
(WebKit::RemoteLayerTreeDrawingAreaProxyMac::existingDisplayLink):
(WebKit::RemoteLayerTreeDrawingAreaProxyMac::removeObserver):
(WebKit::RemoteLayerTreeDrawingAreaProxyMac::setPreferredFramesPerSecond):
(WebKit::RemoteLayerTreeDrawingAreaProxyMac::windowScreenDidChange):
(WebKit::RemoteLayerTreeDrawingAreaProxyMac::exisingDisplayLink): Deleted.
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.cpp:
(WebKit::RemoteLayerTreeEventDispatcher::drawingAreaMac const):
(WebKit::RemoteLayerTreeEventDispatcher::displayLink const):
(WebKit::RemoteLayerTreeEventDispatcher::existingDisplayLink const):
(WebKit::RemoteLayerTreeEventDispatcher::removeDisplayLinkClient):
(WebKit::RemoteLayerTreeEventDispatcher::windowScreenWillChange):
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingCoordinatorProxyMac.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingCoordinatorProxyMac.mm:
(WebKit::RemoteScrollingCoordinatorProxyMac::windowScreenWillChange):

Canonical link: https://commits.webkit.org/265566@main