Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove InjectedBundlePagePolicyClient #5889

Conversation

achristensen07
Copy link
Contributor

@achristensen07 achristensen07 commented Oct 28, 2022

9eeab8d

Remove InjectedBundlePagePolicyClient
https://bugs.webkit.org/show_bug.cgi?id=247169
rdar://101655797

Reviewed by Tim Horton.

There was one layout test, fast/loader/policy-delegate-action-hit-test-zoomed.html,
which printed the dom node path of the clicked link.  This functionality is not available
nor needed in WK2.  The WK1 test still verifies it.  WK2 has _WKHitTestResult with tests
to verify we get the node info we need in this case.

* Source/WebKit/Sources.txt:
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:
* Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
(WKBundlePageSetPolicyClient):
* Source/WebKit/WebProcess/InjectedBundle/InjectedBundlePagePolicyClient.cpp: Removed.
* Source/WebKit/WebProcess/InjectedBundle/InjectedBundlePagePolicyClient.h: Removed.
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
(WebKit::WebFrameLoaderClient::dispatchUnableToImplementPolicy):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::close):
(WebKit::WebPage::initializeInjectedBundlePolicyClient): Deleted.
* Source/WebKit/WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::injectedBundleLoaderClient):
(WebKit::WebPage::injectedBundlePolicyClient): Deleted.

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

bcb2242

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  πŸ§ͺ win
βœ… πŸ›  ios-sim βœ… πŸ›  mac-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk1
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ›  watch-sim βœ… πŸ§ͺ mac-wk2-stress

@achristensen07 achristensen07 self-assigned this Oct 28, 2022
@achristensen07 achristensen07 added WebKit API For issues and bugs in the Web Kit public embedding APIs WebKit Local Build labels Oct 28, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 28, 2022
@achristensen07 achristensen07 force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from 1e51d06 to 6b95a38 Compare October 29, 2022 17:45
@achristensen07 achristensen07 force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from 6b95a38 to ed78d67 Compare October 29, 2022 20:01
@achristensen07 achristensen07 force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from ed78d67 to 9da8a08 Compare October 29, 2022 20:14
@achristensen07 achristensen07 force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from 9da8a08 to be10e5c Compare October 29, 2022 22:30
@achristensen07 achristensen07 force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from be10e5c to 420fcf8 Compare October 29, 2022 22:35
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Oct 29, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 29, 2022
@achristensen07 achristensen07 force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from 420fcf8 to 2efcd95 Compare November 5, 2022 04:13
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Nov 5, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 5, 2022
@achristensen07 achristensen07 force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from 2efcd95 to 6798dca Compare November 5, 2022 18:59
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Nov 5, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 6, 2022
@achristensen07 achristensen07 force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from 6798dca to bcb2242 Compare November 8, 2022 04:28
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Nov 8, 2022
@@ -6,6 +6,7 @@
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.setCustomPolicyDelegate(true);
if (testRunner.skipPolicyDelegateNotifyDone) { testRunner.skipPolicyDelegateNotifyDone() }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the weird style? Should be on the next line with no curly braces, just like in C++

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JS is slightly different than C++ in its handling of if statements with/without {} and I like to do this to be as unambiguous as possible.

@achristensen07 achristensen07 added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 9, 2022
https://bugs.webkit.org/show_bug.cgi?id=247169
rdar://101655797

Reviewed by Tim Horton.

There was one layout test, fast/loader/policy-delegate-action-hit-test-zoomed.html,
which printed the dom node path of the clicked link.  This functionality is not available
nor needed in WK2.  The WK1 test still verifies it.  WK2 has _WKHitTestResult with tests
to verify we get the node info we need in this case.

* Source/WebKit/Sources.txt:
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:
* Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
(WKBundlePageSetPolicyClient):
* Source/WebKit/WebProcess/InjectedBundle/InjectedBundlePagePolicyClient.cpp: Removed.
* Source/WebKit/WebProcess/InjectedBundle/InjectedBundlePagePolicyClient.h: Removed.
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
(WebKit::WebFrameLoaderClient::dispatchUnableToImplementPolicy):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::close):
(WebKit::WebPage::initializeInjectedBundlePolicyClient): Deleted.
* Source/WebKit/WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::injectedBundleLoaderClient):
(WebKit::WebPage::injectedBundlePolicyClient): Deleted.

Canonical link: https://commits.webkit.org/256491@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Remove-InjectedBundlePagePolicyClient branch from bcb2242 to 9eeab8d Compare November 9, 2022 18:44
@webkit-commit-queue
Copy link
Collaborator

Committed 256491@main (9eeab8d): https://commits.webkit.org/256491@main

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

@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 Nov 9, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit 9eeab8d into WebKit:main Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit API For issues and bugs in the Web Kit public embedding APIs
Projects
None yet
5 participants