[IsDeprecatedWeakRefSmartPointerException] Make PageLoadState and related classes RefCounted or equivalent by basuke · Pull Request #34183 · WebKit/WebKit

basuke · 2024-09-24T20:19:56Z

`0eadc69`

[IsDeprecatedWeakRefSmartPointerException] Make PageLoadState and related classes RefCounted or equivalent
https://bugs.webkit.org/show_bug.cgi?id=280282

Reviewed by Chris Dumez.

Those classes are CanMakeWeakPtr so that need to be lifecycle manageable.

* Source/WebKit/UIProcess/API/C/WKPage.cpp:
(WKPageSetPageStateClient):
* Source/WebKit/UIProcess/API/C/mac/WKPagePrivateMac.mm:
(-[WKObservablePageState initWithPage:]):
* Source/WebKit/UIProcess/API/Cocoa/PageLoadStateObserver.h:
* Source/WebKit/UIProcess/API/Cocoa/WKBrowsingContextController.mm:
(-[WKBrowsingContextController _initWithPageRef:]):
(WebKit::PageLoadStateObserver::ref const):
(WebKit::PageLoadStateObserver::deref const):
* Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _initializeWithConfiguration:]):
* Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:
(webkitWebViewConstructed):
(webkitWebViewDispose):
* Source/WebKit/UIProcess/Cocoa/NavigationState.h:
* Source/WebKit/UIProcess/Cocoa/NavigationState.mm:
(WebKit::NavigationState::ref const):
(WebKit::NavigationState::deref const):
* Source/WebKit/UIProcess/PageLoadState.cpp:
(WebKit::PageLoadState::callObserverCallback):
* Source/WebKit/UIProcess/PageLoadState.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::setPageLoadStateObserver):
* Source/WebKit/UIProcess/WebPageProxy.h:

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

e3e7f0a

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-09-24T20:20:01Z

EWS run on previous version of this PR (hash 06dfe46)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	~~🧪 win-tests~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~		~~🛠 wpe-cairo~~
	⏳ 🧪 api-ios	~~🧪 mac-wk2~~	~~🛠 gtk~~
	~~🛠 vision~~	⏳ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	~~🛠 vision-sim~~	~~🧪 mac-wk2-stress~~	~~🧪 api-gtk~~
	⏳ 🧪 vision-wk2	⏳ 🧪 mac-intel-wk2
	~~🛠 tv~~
	~~🛠 tv-sim~~
	~~🛠 watch~~
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-09-24T20:40:25Z

EWS run on previous version of this PR (hash a49b98e)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	~~🧪 api-wpe~~
	✅ 🧪 ios-wk2-wpt		❌ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-09-24T23:24:20Z

EWS run on previous version of this PR (hash f50f369)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		❌ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-09-25T16:39:33Z

EWS run on previous version of this PR (hash b899496)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		❌ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	❌ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-09-25T18:38:25Z

EWS run on previous version of this PR (hash 1e21ec5)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-09-25T23:30:49Z

EWS run on previous version of this PR (hash 1052aee)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	⏳ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-09-26T20:49:40Z

EWS run on previous version of this PR (hash 82a9f72)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~	~~🛠 win~~
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	~~🧪 win-tests~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🧪 api-mac	~~🧪 api-wpe~~
	✅ 🧪 ios-wk2-wpt		~~🛠 wpe-cairo~~
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	⏳ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	⏳ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-09-26T21:03:44Z

EWS run on previous version of this PR (hash e554161)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

cdumez

LGTM with changes.

cdumez · 2024-09-26T23:25:46Z

Should return a Ref<> since it cannot return nullptr.

cdumez · 2024-09-26T23:25:54Z

return adoptRef(*new StateClient(client));

cdumez · 2024-09-26T23:27:26Z

Should return a Ref<>

cdumez · 2024-09-26T23:28:13Z

The previous style was correct, since this is an Objective C type.

webkit-early-warning-system · 2024-09-27T05:26:36Z

EWS run on current version of this PR (hash e3e7f0a)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

…ated classes RefCounted or equivalent https://bugs.webkit.org/show_bug.cgi?id=280282 Reviewed by Chris Dumez. Those classes are CanMakeWeakPtr so that need to be lifecycle manageable. * Source/WebKit/UIProcess/API/C/WKPage.cpp: (WKPageSetPageStateClient): * Source/WebKit/UIProcess/API/C/mac/WKPagePrivateMac.mm: (-[WKObservablePageState initWithPage:]): * Source/WebKit/UIProcess/API/Cocoa/PageLoadStateObserver.h: * Source/WebKit/UIProcess/API/Cocoa/WKBrowsingContextController.mm: (-[WKBrowsingContextController _initWithPageRef:]): (WebKit::PageLoadStateObserver::ref const): (WebKit::PageLoadStateObserver::deref const): * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm: (-[WKWebView _initializeWithConfiguration:]): * Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp: (webkitWebViewConstructed): (webkitWebViewDispose): * Source/WebKit/UIProcess/Cocoa/NavigationState.h: * Source/WebKit/UIProcess/Cocoa/NavigationState.mm: (WebKit::NavigationState::ref const): (WebKit::NavigationState::deref const): * Source/WebKit/UIProcess/PageLoadState.cpp: (WebKit::PageLoadState::callObserverCallback): * Source/WebKit/UIProcess/PageLoadState.h: * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::setPageLoadStateObserver): * Source/WebKit/UIProcess/WebPageProxy.h: Canonical link: https://commits.webkit.org/284380@main

webkit-commit-queue · 2024-09-27T19:02:12Z

Committed 284380@main (0eadc69): https://commits.webkit.org/284380@main

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

basuke self-assigned this Sep 24, 2024

basuke added the WebKit2 Bugs relating to the WebKit2 API layer label Sep 24, 2024

basuke force-pushed the eng/280282 branch from 06dfe46 to a49b98e Compare September 24, 2024 20:40

basuke marked this pull request as ready for review September 24, 2024 20:46

basuke requested review from a team and cdumez as code owners September 24, 2024 20:46

basuke requested review from geoffreygaren and rniwa September 24, 2024 20:46

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 24, 2024

basuke removed the merging-blocked Applied to prevent a change from being merged label Sep 24, 2024

basuke force-pushed the eng/280282 branch from a49b98e to f50f369 Compare September 24, 2024 23:24

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 25, 2024

basuke marked this pull request as draft September 25, 2024 16:35

basuke removed the merging-blocked Applied to prevent a change from being merged label Sep 25, 2024

basuke force-pushed the eng/280282 branch from f50f369 to b899496 Compare September 25, 2024 16:39

cdumez reviewed Sep 25, 2024

View reviewed changes

Comment thread Source/WebKit/UIProcess/Cocoa/NavigationState.h Outdated

cdumez reviewed Sep 25, 2024

View reviewed changes

Comment thread Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp Outdated

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 25, 2024

basuke removed the merging-blocked Applied to prevent a change from being merged label Sep 25, 2024

basuke force-pushed the eng/280282 branch from b899496 to 1e21ec5 Compare September 25, 2024 18:38

basuke marked this pull request as ready for review September 25, 2024 23:06

basuke requested a review from cdumez September 25, 2024 23:06

basuke force-pushed the eng/280282 branch from 1e21ec5 to 1052aee Compare September 25, 2024 23:30

cdumez requested changes Sep 26, 2024

View reviewed changes

Comment thread Source/WebKit/UIProcess/API/Cocoa/PageLoadStateObserver.h Outdated

Comment thread Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm Outdated

cdumez reviewed Sep 26, 2024

View reviewed changes

Comment thread Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp Outdated

basuke marked this pull request as draft September 26, 2024 20:49

basuke force-pushed the eng/280282 branch from 1052aee to 82a9f72 Compare September 26, 2024 20:49

basuke force-pushed the eng/280282 branch from 82a9f72 to e554161 Compare September 26, 2024 21:03

basuke marked this pull request as ready for review September 26, 2024 21:03

basuke requested a review from cdumez September 26, 2024 21:04

cdumez approved these changes Sep 26, 2024

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 27, 2024

basuke removed the merging-blocked Applied to prevent a change from being merged label Sep 27, 2024

basuke force-pushed the eng/280282 branch from e554161 to e3e7f0a Compare September 27, 2024 05:26

basuke added the merge-queue Applied to send a pull request to merge-queue label Sep 27, 2024

webkit-commit-queue force-pushed the eng/280282 branch from e3e7f0a to 0eadc69 Compare September 27, 2024 19:02

webkit-commit-queue merged commit 0eadc69 into WebKit:main Sep 27, 2024

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 27, 2024

basuke deleted the eng/280282 branch January 16, 2025 19:04

Conversation

basuke commented Sep 24, 2024 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

0eadc69

Uh oh!

webkit-early-warning-system commented Sep 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Sep 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Sep 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

webkit-early-warning-system commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

webkit-early-warning-system commented Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Sep 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cdumez left a comment

Choose a reason for hiding this comment

Uh oh!

cdumez Sep 26, 2024

Choose a reason for hiding this comment

Uh oh!

cdumez Sep 26, 2024

Choose a reason for hiding this comment

Uh oh!

cdumez Sep 26, 2024

Choose a reason for hiding this comment

Uh oh!

cdumez Sep 26, 2024

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Sep 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-commit-queue commented Sep 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

basuke commented Sep 24, 2024 •

edited by webkit-early-warning-system

Loading

`0eadc69`

webkit-early-warning-system commented Sep 24, 2024 •

edited

Loading

webkit-early-warning-system commented Sep 24, 2024 •

edited

Loading

webkit-early-warning-system commented Sep 24, 2024 •

edited

Loading

webkit-early-warning-system commented Sep 25, 2024 •

edited

Loading

webkit-early-warning-system commented Sep 25, 2024 •

edited

Loading

webkit-early-warning-system commented Sep 25, 2024 •

edited

Loading

webkit-early-warning-system commented Sep 26, 2024 •

edited

Loading

webkit-early-warning-system commented Sep 26, 2024 •

edited

Loading

webkit-early-warning-system commented Sep 27, 2024 •

edited

Loading