[webkit-changes] [WebKit/WebKit] 9e6f0c: [CoreIPC] -[NSButtonCell isKindOfClass:]: message ...

[Aditya Keerthi]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 9e6f0cd6c86a4292c888a02a7eb701bb30877609
      https://github.com/WebKit/WebKit/commit/9e6f0cd6c86a4292c888a02a7eb701bb30877609
  Author: Aditya Keerthi <akeerthi at apple.com>
  Date:   2024-08-08 (Thu, 08 Aug 2024)

  Changed paths:
    M Source/WebCore/Sources.txt
    M Source/WebCore/WebCore.xcodeproj/project.pbxproj
    M Source/WebCore/platform/graphics/GraphicsContext.cpp
    M Source/WebCore/platform/graphics/GraphicsContext.h
    M Source/WebCore/platform/graphics/controls/ControlFactory.cpp
    M Source/WebCore/platform/graphics/controls/ControlFactory.h
    M Source/WebCore/platform/graphics/controls/ControlPart.cpp
    M Source/WebCore/platform/graphics/controls/ControlPart.h
    A Source/WebCore/platform/graphics/controls/EmptyControlFactory.cpp
    A Source/WebCore/platform/graphics/controls/EmptyControlFactory.h
    M Source/WebCore/platform/graphics/displaylists/DisplayListItem.cpp
    M Source/WebCore/platform/graphics/displaylists/DisplayListItem.h
    M Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp
    M Source/WebCore/platform/graphics/displaylists/DisplayListItems.h
    M Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp
    M Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h
    M Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp
    M Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.h
    M Source/WebCore/platform/graphics/ios/controls/ControlFactoryIOS.mm
    M Source/WebCore/platform/graphics/mac/controls/ControlFactoryMac.h
    M Source/WebCore/platform/graphics/mac/controls/ControlFactoryMac.mm
    M Source/WebCore/platform/graphics/mac/controls/ImageControlsButtonMac.mm
    M Source/WebCore/rendering/TextPainter.cpp
    M Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp
    M Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h
    M Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp

  Log Message:
  -----------
  [CoreIPC] -[NSButtonCell isKindOfClass:]: message sent to deallocated instance in WebCore::ControlMac::drawCellInternal
https://bugs.webkit.org/show_bug.cgi?id=273788
rdar://126071623

Reviewed by Said Abou-Hallawa.

`ControlFactory` is not a thread-safe object, and the shared factory should
only ever be used on the main thread. The shared factory is used by
`ControlPart` if one is not already assigned.

Currently, an attempt at ensuring thread-safety is made by avoiding use of
the shared factory on `RemoteRenderingBackend` work queues, by creating and
assigning a thread-specific `ControlFactory` to a `ControlPart` in
`RemoteDisplayListRecorder::drawControlPart`. However, this logic does not
account for the fact that the `DrawControlPart` display list item can also be
applied as a result of applying `DrawDisplayListItems`. In this scenario, the
`ControlPart` will have a null `ControlFactory`, and will simply fall back to
using the shared factory.

Fix by ensuring the creation of a `ControlFactory` in
`RemoteDisplayListRecorder::drawDisplayListItems`, and adding the necessary
plumbing to ensure `ControlPart`s drawn as a result of applying
`DrawDisplayListItems` use a thread-specific factory.

* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::drawDisplayListItems):
* Source/WebCore/platform/graphics/GraphicsContext.h:
* Source/WebCore/platform/graphics/controls/ControlFactory.cpp:
(WebCore::ControlFactory::create):

Remove the release assert for other platforms, as `ControlFactory` is created
when drawing display list items. However, other platforms will continue to not
actually draw `ControlPart`s, using the empty implementation.

(WebCore::ControlFactory::shared):

Use `MainThreadNeverDestroyed`, as the shared factory is not thread-safe.

(WebCore::ControlFactory::createControlFactory): Deleted.
(WebCore::ControlFactory::sharedControlFactory): Deleted.
* Source/WebCore/platform/graphics/controls/ControlFactory.h:

Make `ControlFactory` ref-counted to avoid raw pointer usage in member variables.

Rename static methods to match WebKit convention.

* Source/WebCore/platform/graphics/controls/ControlPart.cpp:
(WebCore::ControlPart::controlFactory const):
* Source/WebCore/platform/graphics/controls/ControlPart.h:
(WebCore::ControlPart::setControlFactory):
(): Deleted.
* Source/WebCore/platform/graphics/controls/EmptyControlFactory.cpp: Added.
(WebCore::EmptyControlFactory::createPlatformApplePayButton):
(WebCore::EmptyControlFactory::createPlatformButton):
(WebCore::EmptyControlFactory::createPlatformColorWell):
(WebCore::EmptyControlFactory::createPlatformImageControlsButton):
(WebCore::EmptyControlFactory::createPlatformInnerSpinButton):
(WebCore::EmptyControlFactory::createPlatformMenuList):
(WebCore::EmptyControlFactory::createPlatformMenuListButton):
(WebCore::EmptyControlFactory::createPlatformMeter):
(WebCore::EmptyControlFactory::createPlatformProgressBar):
(WebCore::EmptyControlFactory::createPlatformSearchField):
(WebCore::EmptyControlFactory::createPlatformSearchFieldCancelButton):
(WebCore::EmptyControlFactory::createPlatformSearchFieldResults):
(WebCore::EmptyControlFactory::createPlatformSliderThumb):
(WebCore::EmptyControlFactory::createPlatformSliderTrack):
(WebCore::EmptyControlFactory::createPlatformSwitchThumb):
(WebCore::EmptyControlFactory::createPlatformSwitchTrack):
(WebCore::EmptyControlFactory::createPlatformTextArea):
(WebCore::EmptyControlFactory::createPlatformTextField):
(WebCore::EmptyControlFactory::createPlatformToggleButton):
* Source/WebCore/platform/graphics/controls/EmptyControlFactory.h: Added.
* Source/WebCore/platform/graphics/displaylists/DisplayListItem.cpp:
(WebCore::DisplayList::applyItem):
* Source/WebCore/platform/graphics/displaylists/DisplayListItem.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::DrawDisplayListItems::apply const):
(WebCore::DisplayList::DrawControlPart::apply const):
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:
(WebCore::DisplayList::Recorder::drawDisplayListItems):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp:
(WebCore::DisplayList::Replayer::Replayer):
(WebCore::DisplayList::Replayer::replay):
* Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.h:
* Source/WebCore/platform/graphics/ios/controls/ControlFactoryIOS.mm:
(WebCore::ControlFactory::create):
(WebCore::ControlFactory::createControlFactory): Deleted.
* Source/WebCore/platform/graphics/mac/controls/ControlFactoryMac.h:
* Source/WebCore/platform/graphics/mac/controls/ControlFactoryMac.mm:
(WebCore::ControlFactory::create):
(WebCore::ControlFactoryMac::shared):
(WebCore::ControlFactory::createControlFactory): Deleted.
(WebCore::ControlFactoryMac::sharedControlFactory): Deleted.
* Source/WebCore/platform/graphics/mac/controls/ImageControlsButtonMac.mm:
(WebCore::ImageControlsButtonMac::servicesRolloverButtonCellSize):
* Source/WebCore/rendering/TextPainter.cpp:
(WebCore::TextPainter::paintTextOrEmphasisMarks):
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
(WebKit::RemoteDisplayListRecorder::controlFactory):
(WebKit::RemoteDisplayListRecorder::drawDisplayListItems):

This is the important part of the fix. A thread-specific `ControlFactory` must
be specified for `DrawDisplayListItems`, so that contained `DrawControlPart`
items do not use the shared factory.

(WebKit::RemoteDisplayListRecorder::drawControlPart):
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:
* Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:
(TestWebKitAPI::TEST):

Originally-landed-as: 272448.998 at safari-7618-branch (dac0ebcb77d8). rdar://132958419
Canonical link: https://commits.webkit.org/282012@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications