Skip to content

Conversation

@b-weinstein
Copy link
Contributor

@b-weinstein b-weinstein commented Dec 4, 2023

c26e66f

Implement declarativeNetRequest.setExtensionActionOptions
https://bugs.webkit.org/show_bug.cgi?id=265829
rdar://118476776

Reviewed by Timothy Hatcher.

This API has two use cases:
1) To opt an extension into a behavior where the extension's action shows the number of blocked resources on the current page
2) To manually increment or decrement this badged number

It's a bit unfortunate that these two disparate behaviors are combined into one API, but here we are. This PR implements both of
them and adds tests for them.

* Source/WebKit/UIProcess/Cocoa/NavigationState.mm:
(WebKit::NavigationState::NavigationClient::contentRuleListNotification): Call into the WebExtensionController.
* Source/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPIDeclarativeNetRequestCocoa.mm:
(WebKit::WebExtensionContext::shouldDisplayBlockedResourceCountAsBadgeText): Returns whether or not the blocked resource count should be the badge text.
(WebKit::WebExtensionContext::saveShouldDisplayBlockedResourceCountAsBadgeText): Sets whether or not the blocked resource count should be the badge
text, and saves to disk.
(WebKit::WebExtensionContext::incrementActionCountForTab): Get the action for the tab and increment the blocked resource count.
(WebKit::WebExtensionContext::declarativeNetRequestDisplayActionCountAsBadgeText): Call saveShouldDisplayBlockedResourceCountAsBadgeText with the new value, and
clear any blocked resource counts if the flag is turned off.
(WebKit::WebExtensionContext::declarativeNetRequestIncrementActionCount): Call into incrementActionCountForTab after finding the tab.
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionActionCocoa.mm:
(WebKit::WebExtensionAction::clearCustomizations): Clear the blocked resource count.
(WebKit::WebExtensionAction::clearBlockedResourceCount): Ditto.
(WebKit::WebExtensionAction::badgeText const): If we have a blocked resource count - use it as the badge text.
(WebKit::WebExtensionAction::incrementBlockedResourceCount): Modify the blocked resource count member variable.
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionControllerCocoa.mm:
(WebKit::WebExtensionController::handleContentRuleListNotification): Iterate over all of the actions, find the extension if it exists, and call incrementActionCountForTab.
* Source/WebKit/UIProcess/Extensions/WebExtensionAction.h:
* Source/WebKit/UIProcess/Extensions/WebExtensionContext.h:
* Source/WebKit/UIProcess/Extensions/WebExtensionContext.messages.in:
* Source/WebKit/UIProcess/Extensions/WebExtensionController.h:
* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIDeclarativeNetRequestCocoa.mm:
(WebKit::WebExtensionAPIDeclarativeNetRequest::setExtensionActionOptions): Perform object validation and call into the UI process based on if flavor (1) or (2) was called.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIDeclarativeNetRequest.mm:
(TestWebKitAPI::TEST):

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

dfb2ff0

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

@b-weinstein b-weinstein self-assigned this Dec 4, 2023
@b-weinstein b-weinstein added the WebKit Extensions Bugs related to extension support. label Dec 4, 2023
Comment on lines 433 to 434
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this still take the count to zero? What do other browsers do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should be able to take it to zero - that would clear the badge text.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, if m_blockedResourceCount is 1, and amount is -2, should it just reset m_blockedResourceCount to 0? Right now it will stay at 1 with the return.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will see what Chrome does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What you described is what Chrome does. I will make that change.

Comment on lines +306 to +308
Copy link
Contributor

@xeenon xeenon Dec 4, 2023

Choose a reason for hiding this comment

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

You should move this out of the loops, since it is the same for all results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it? Getting the tab seems to require the context, and what happens if one specific extension doesn't have access to a tab?

If they are the same, then I'm fine moving this out of the loop - how would you recommend getting the tab for one particular extension?

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 5, 2023
@b-weinstein b-weinstein force-pushed the eng/Implement-declarativeNetRequest-setExtensionActionOptions branch from 4791e6b to 5b216e1 Compare December 5, 2023 17:36
@b-weinstein b-weinstein added the merge-queue Applied to send a pull request to merge-queue label Dec 5, 2023
@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 5b216e1. Live statuses available at the PR page, #21305

@b-weinstein b-weinstein removed merging-blocked Applied to prevent a change from being merged merge-queue Applied to send a pull request to merge-queue labels Dec 5, 2023
@b-weinstein b-weinstein force-pushed the eng/Implement-declarativeNetRequest-setExtensionActionOptions branch from 5b216e1 to dfb2ff0 Compare December 5, 2023 18:15
@b-weinstein b-weinstein added the merge-queue Applied to send a pull request to merge-queue label Dec 5, 2023
https://bugs.webkit.org/show_bug.cgi?id=265829
rdar://118476776

Reviewed by Timothy Hatcher.

This API has two use cases:
1) To opt an extension into a behavior where the extension's action shows the number of blocked resources on the current page
2) To manually increment or decrement this badged number

It's a bit unfortunate that these two disparate behaviors are combined into one API, but here we are. This PR implements both of
them and adds tests for them.

* Source/WebKit/UIProcess/Cocoa/NavigationState.mm:
(WebKit::NavigationState::NavigationClient::contentRuleListNotification): Call into the WebExtensionController.
* Source/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPIDeclarativeNetRequestCocoa.mm:
(WebKit::WebExtensionContext::shouldDisplayBlockedResourceCountAsBadgeText): Returns whether or not the blocked resource count should be the badge text.
(WebKit::WebExtensionContext::saveShouldDisplayBlockedResourceCountAsBadgeText): Sets whether or not the blocked resource count should be the badge
text, and saves to disk.
(WebKit::WebExtensionContext::incrementActionCountForTab): Get the action for the tab and increment the blocked resource count.
(WebKit::WebExtensionContext::declarativeNetRequestDisplayActionCountAsBadgeText): Call saveShouldDisplayBlockedResourceCountAsBadgeText with the new value, and
clear any blocked resource counts if the flag is turned off.
(WebKit::WebExtensionContext::declarativeNetRequestIncrementActionCount): Call into incrementActionCountForTab after finding the tab.
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionActionCocoa.mm:
(WebKit::WebExtensionAction::clearCustomizations): Clear the blocked resource count.
(WebKit::WebExtensionAction::clearBlockedResourceCount): Ditto.
(WebKit::WebExtensionAction::badgeText const): If we have a blocked resource count - use it as the badge text.
(WebKit::WebExtensionAction::incrementBlockedResourceCount): Modify the blocked resource count member variable.
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionControllerCocoa.mm:
(WebKit::WebExtensionController::handleContentRuleListNotification): Iterate over all of the actions, find the extension if it exists, and call incrementActionCountForTab.
* Source/WebKit/UIProcess/Extensions/WebExtensionAction.h:
* Source/WebKit/UIProcess/Extensions/WebExtensionContext.h:
* Source/WebKit/UIProcess/Extensions/WebExtensionContext.messages.in:
* Source/WebKit/UIProcess/Extensions/WebExtensionController.h:
* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIDeclarativeNetRequestCocoa.mm:
(WebKit::WebExtensionAPIDeclarativeNetRequest::setExtensionActionOptions): Perform object validation and call into the UI process based on if flavor (1) or (2) was called.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIDeclarativeNetRequest.mm:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/271561@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Implement-declarativeNetRequest-setExtensionActionOptions branch from dfb2ff0 to c26e66f Compare December 5, 2023 19:13
@webkit-commit-queue
Copy link
Collaborator

Committed 271561@main (c26e66f): https://commits.webkit.org/271561@main

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

@webkit-commit-queue webkit-commit-queue merged commit c26e66f into WebKit:main Dec 5, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKit Extensions Bugs related to extension support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants