Skip to content

Conversation

@pvollan
Copy link
Contributor

@pvollan pvollan commented Feb 29, 2024

868e634

Move ProcessThrottler to AuxiliaryProcessProxy
https://bugs.webkit.org/show_bug.cgi?id=269136
rdar://122708925

Reviewed by Chris Dumez.

We can simplify the code by moving the ProcessThrottler object from every subclass to the
AuxiliaryProcessProxy base class.

This was previously landed in <https://commits.webkit.org/275195@main> and later reverted
because it introduced a crash. The reason for the crash was that the ProcessThrottler
was being deleted later since it moved to AuxiliaryProcessProxy. That could result in
pure virtual function calls in the AuxiliaryProcessProxy destructor, since the
ProcessThrottler calls virtual methods in AuxiliaryProcessProxy. This patch addresses
this crash by calling didDisconnectFromProcess in AuxiliaryProcessProxy::shutDownProcess.

* Source/WebKit/UIProcess/AuxiliaryProcessProxy.h:
(WebKit::AuxiliaryProcessProxy::sendProcessDidResume):
(WebKit::AuxiliaryProcessProxy::clientName const):
* Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:
(WebKit::AuxiliaryProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/AuxiliaryProcessProxy.h:
(WebKit::AuxiliaryProcessProxy::throttler):
* Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:
(WebKit::GPUProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/GPU/GPUProcessProxy.h:
* Source/WebKit/UIProcess/Model/ModelProcessProxy.cpp:
(WebKit::ModelProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/Model/ModelProcessProxy.h:
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::throttler const): Deleted.

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

9495dcb

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ⏳ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ⏳ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ⏳ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@pvollan pvollan requested a review from cdumez as a code owner February 29, 2024 21:36
@pvollan pvollan force-pushed the eng/Move-ProcessThrottler-to-AuxiliaryProcessProxy branch from b3da4ad to d260384 Compare February 29, 2024 21:36
@pvollan pvollan self-assigned this Feb 29, 2024
@pvollan pvollan added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Feb 29, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Feb 29, 2024

@pvollan pvollan force-pushed the eng/Move-ProcessThrottler-to-AuxiliaryProcessProxy branch from d260384 to 9495dcb Compare February 29, 2024 22:07
@pvollan pvollan added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 1, 2024
https://bugs.webkit.org/show_bug.cgi?id=269136
rdar://122708925

Reviewed by Chris Dumez.

We can simplify the code by moving the ProcessThrottler object from every subclass to the
AuxiliaryProcessProxy base class.

This was previously landed in <https://commits.webkit.org/275195@main> and later reverted
because it introduced a crash. The reason for the crash was that the ProcessThrottler
was being deleted later since it moved to AuxiliaryProcessProxy. That could result in
pure virtual function calls in the AuxiliaryProcessProxy destructor, since the
ProcessThrottler calls virtual methods in AuxiliaryProcessProxy. This patch addresses
this crash by calling didDisconnectFromProcess in AuxiliaryProcessProxy::shutDownProcess.

* Source/WebKit/UIProcess/AuxiliaryProcessProxy.h:
(WebKit::AuxiliaryProcessProxy::sendProcessDidResume):
(WebKit::AuxiliaryProcessProxy::clientName const):
* Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:
(WebKit::AuxiliaryProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/AuxiliaryProcessProxy.h:
(WebKit::AuxiliaryProcessProxy::throttler):
* Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:
(WebKit::GPUProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/GPU/GPUProcessProxy.h:
* Source/WebKit/UIProcess/Model/ModelProcessProxy.cpp:
(WebKit::ModelProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/Model/ModelProcessProxy.h:
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::didFinishLaunching):
* Source/WebKit/UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::throttler const): Deleted.

Canonical link: https://commits.webkit.org/275528@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Move-ProcessThrottler-to-AuxiliaryProcessProxy branch from 9495dcb to 868e634 Compare March 1, 2024 01:27
@webkit-commit-queue
Copy link
Collaborator

Committed 275528@main (868e634): https://commits.webkit.org/275528@main

Reviewed commits have been landed. Closing PR #25315 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 868e634 into WebKit:main Mar 1, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants