[webkit-changes] [WebKit/WebKit] 74dab6: REGRESSION (270354 at main?): [ macOS Release x86_64 ...

[Jean-Yves Avenard]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 74dab6364fd5764cab0b524f0a4377967de2ddee
      https://github.com/WebKit/WebKit/commit/74dab6364fd5764cab0b524f0a4377967de2ddee
  Author: Jean-Yves Avenard <jya at apple.com>
  Date:   2023-12-01 (Fri, 01 Dec 2023)

  Changed paths:
    M LayoutTests/platform/wk2/TestExpectations
    M Source/WebCore/platform/graphics/MediaSourcePrivate.cpp
    M Source/WebCore/platform/graphics/SourceBufferPrivate.cpp
    M Source/WebCore/platform/graphics/SourceBufferPrivate.h
    M Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h
    M Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm

  Log Message:
  -----------
  REGRESSION (270354 at main?): [ macOS Release x86_64 wk2 ] media/media-source/mock-managedmse-bufferedchange.html is a flaky crash
https://bugs.webkit.org/show_bug.cgi?id=265549
rdar://118951399

Reviewed by Youenn Fablet.

In 270354 at main, an incorrect assumption was made that the duplicated code found in the
SourceBufferPrivateAVFObjC destructor and SourceBufferPrivateAVFObjC::removedFromMediaSource
could be removed.
However, destructing the AVSampleBufferDisplayLayer in the SourceBuffer destructor is causing
a deadlock in the AVSampleBufferDisplayLayer as its waiting for all the CoreMedia threads to
shutdown; most of those threads waiting on a sync dispatch to the main thread.
As a result, the GPU process was getting killed after a few seconds. This explained the like
of backtrace or crashlog despite the process being shown as "crashed".

Fly-by:
- the SourceBufferPrivate::m_mediaSource doesn't need to be cleared from the
MediaSourcePrivate destructor. It is a weak pointer and it will be set to zero automatically.
- Correct the SourceBufferPrivate::removedFromMediaSource comment. The SourceBufferClient owns
the last reference to the SourceBufferPrivate, as such we don't need to take a strong ref. Which
would be problematic anyway should it be called from the destructor.

This code is in need of a refactoring to avoid the back and forth re-entrancy between all the related objects.

Covered by existing tests.

* LayoutTests/platform/wk2/TestExpectations:
* Source/WebCore/platform/graphics/MediaSourcePrivate.cpp:
(WebCore::MediaSourcePrivate::~MediaSourcePrivate): Deleted.
* Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:
(WebCore::SourceBufferPrivate::removedFromMediaSource):
* Source/WebCore/platform/graphics/SourceBufferPrivate.h:
(WebCore::SourceBufferPrivate::clearMediaSource): Deleted.
* Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::removedFromMediaSource):

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