Skip to content

Commit

Permalink
DNR content rule list notifications aren't being delivered to the cor…
Browse files Browse the repository at this point in the history 
…rect extension

https://bugs.webkit.org/show_bug.cgi?id=266099
<rdar://problem/119397749>

Reviewed by Timothy Hatcher.

When notifying an extension that a DNR rule was matched (for getMatchedRules or setExtensionActionOptions), we need a way
to match the content rule list notification to the extension. After the change of the name of the content rule list to be the
same across all extensions, there was no way to do this anymore.

It's a bit silly to require the unique name of the extension in an already unique directory, but ContentRuleListResults only
has the identifier of the content rule list that performed the action, not the store it came from.

While I'm here, a few more changes were made:
1) Clear out some DNR ivars when an extension context is unloaded
2) Enhance the DynamicRules test to unload and reload the extension context, and verify that the rules are still loaded and applied
upon unload/reload of the context.

* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm:
(WebKit::WebExtensionContext::unload):
(WebKit::WebExtensionContext::addDeclarativeNetRequestRulesToPrivateUserContentControllers):
(WebKit::WebExtensionContext::compileDeclarativeNetRequestRules):
(WebKit::WebExtensionContext::loadDeclarativeNetRequestRules):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIDeclarativeNetRequest.mm:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/271762@main
  • Loading branch information
@b-weinstein
b-weinstein committed Dec 8, 2023
1 parent 7b6ea01 commit b094502
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 18 deletions.
27 changes: 19 additions & 8 deletions Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
static NSString * const lastSeenBaseURLStateKey = @"LastSeenBaseURL";
static NSString * const lastSeenVersionStateKey = @"LastSeenVersion";
static NSString * const lastLoadedDeclarativeNetRequestHashStateKey = @"LastLoadedDeclarativeNetRequestHash";
static NSString * const declarativeNetRequestContentRuleListIdentifier = @"DeclarativeNetRequest.data";

// Update this value when any changes are made to the WebExtensionEventListenerType enum.
static constexpr NSInteger currentBackgroundContentListenerStateVersion = 3;
Expand Down Expand Up @@ -267,15 +266,13 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)

unloadBackgroundWebView();
removeInjectedContent();
removeDeclarativeNetRequestRules();

invalidateStorage();
unloadDeclarativeNetRequestState();

m_extensionController = nil;
m_contentScriptWorld = nullptr;

m_matchedRules.clear();

return true;
}

Expand Down Expand Up @@ -514,6 +511,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
}

addedPermissions.add(entry.key);
addedPermissions.add(entry.key);
}

if (addedPermissions.isEmpty() && removedPermissions.isEmpty())
Expand Down Expand Up @@ -2943,6 +2941,19 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
}
}

void WebExtensionContext::unloadDeclarativeNetRequestState()
{
removeDeclarativeNetRequestRules();

m_sessionRulesIDs.clear();
m_dynamicRulesIDs.clear();
m_matchedRules.clear();

m_declarativeNetRequestDynamicRulesStore = nullptr;
m_declarativeNetRequestSessionRulesStore = nullptr;
m_declarativeNetRequestRuleStore = nullptr;
}

WKContentRuleListStore *WebExtensionContext::declarativeNetRequestRuleStore()
{
if (m_declarativeNetRequestRuleStore)
Expand All @@ -2969,7 +2980,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)

void WebExtensionContext::addDeclarativeNetRequestRulesToPrivateUserContentControllers()
{
[declarativeNetRequestRuleStore() lookUpContentRuleListForIdentifier:declarativeNetRequestContentRuleListIdentifier completionHandler:^(WKContentRuleList *ruleList, NSError *error) {
[declarativeNetRequestRuleStore() lookUpContentRuleListForIdentifier:uniqueIdentifier() completionHandler:^(WKContentRuleList *ruleList, NSError *error) {
if (!ruleList)
return;

Expand Down Expand Up @@ -3008,7 +3019,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
auto *previouslyLoadedHash = objectForKey<NSString>(m_state, lastLoadedDeclarativeNetRequestHashStateKey);
auto *hashOfWebKitRules = computeStringHashForContentBlockerRules(webKitRules);

[declarativeNetRequestRuleStore() lookUpContentRuleListForIdentifier:declarativeNetRequestContentRuleListIdentifier completionHandler:makeBlockPtr([this, protectedThis = Ref { *this }, completionHandler = WTFMove(completionHandler), previouslyLoadedHash = String { previouslyLoadedHash }, hashOfWebKitRules = String { hashOfWebKitRules }, webKitRules = String { webKitRules }](WKContentRuleList *foundRuleList, NSError *) mutable {
[declarativeNetRequestRuleStore() lookUpContentRuleListForIdentifier:uniqueIdentifier() completionHandler:makeBlockPtr([this, protectedThis = Ref { *this }, completionHandler = WTFMove(completionHandler), previouslyLoadedHash = String { previouslyLoadedHash }, hashOfWebKitRules = String { hashOfWebKitRules }, webKitRules = String { webKitRules }](WKContentRuleList *foundRuleList, NSError *) mutable {
if (foundRuleList) {
if ([previouslyLoadedHash isEqualToString:hashOfWebKitRules]) {
auto userContentControllers = hasAccessInPrivateBrowsing() ? extensionController()->allUserContentControllers() : extensionController()->allNonPrivateUserContentControllers();
Expand All @@ -3020,7 +3031,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
}
}

[declarativeNetRequestRuleStore() compileContentRuleListForIdentifier:declarativeNetRequestContentRuleListIdentifier encodedContentRuleList:webKitRules completionHandler:makeBlockPtr([this, protectedThis = Ref { *this }, completionHandler = WTFMove(completionHandler), hashOfWebKitRules = String { hashOfWebKitRules }](WKContentRuleList *ruleList, NSError *error) mutable {
[declarativeNetRequestRuleStore() compileContentRuleListForIdentifier:uniqueIdentifier() encodedContentRuleList:webKitRules completionHandler:makeBlockPtr([this, protectedThis = Ref { *this }, completionHandler = WTFMove(completionHandler), hashOfWebKitRules = String { hashOfWebKitRules }](WKContentRuleList *ruleList, NSError *error) mutable {
if (error) {
RELEASE_LOG_ERROR(Extensions, "Error compiling declarativeNetRequest rules: %{public}@", privacyPreservingDescription(error));
completionHandler(false);
Expand Down Expand Up @@ -3051,7 +3062,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
auto applyDeclarativeNetRequestRules = [this, protectedThis = Ref { *this }, completionHandler = WTFMove(completionHandler), allJSONData = RetainPtr { allJSONData }] () mutable {
if (!allJSONData.get().count) {
removeDeclarativeNetRequestRules();
[declarativeNetRequestRuleStore() removeContentRuleListForIdentifier:declarativeNetRequestContentRuleListIdentifier completionHandler:^(NSError *) { }];
[declarativeNetRequestRuleStore() removeContentRuleListForIdentifier:uniqueIdentifier() completionHandler:^(NSError *) { }];
completionHandler(true);
return;
}
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/Extensions/WebExtensionContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ class WebExtensionContext : public API::ObjectImpl<API::Object::Type::WebExtensi
// Loading/unloading static rules
void loadDeclarativeNetRequestRules(CompletionHandler<void(bool)>&&);
void compileDeclarativeNetRequestRules(NSArray *, CompletionHandler<void(bool)>&&);
void unloadDeclarativeNetRequestState();
WKContentRuleListStore *declarativeNetRequestRuleStore();

// Updating user content controllers with new rules.
Expand Down
31 changes: 21 additions & 10 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIDeclarativeNetRequest.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -552,14 +552,17 @@
}, TestWebKitAPI::HTTPServer::Protocol::Http);

auto *backgroundScript = Util::constructScript(@[
@"let sessionRules = await browser.declarativeNetRequest.getDynamicRules()",
@"browser.test.assertEq(sessionRules.length, 0)",
@"await browser.declarativeNetRequest.updateDynamicRules({ addRules: [{ id: 1, priority: 1, action: {type: 'block'}, condition: { urlFilter: 'frame' } }] })",
@"sessionRules = await browser.declarativeNetRequest.getDynamicRules()",
@"browser.test.assertEq(sessionRules.length, 1)",

// Yield after the background page has loaded so we can load a tab.
@"browser.test.yield('Load Tab')"
@"let dynamicRules = await browser.declarativeNetRequest.getDynamicRules()",
@"if (dynamicRules.length == 0) {",
@" await browser.declarativeNetRequest.updateDynamicRules({ addRules: [{ id: 1, priority: 1, action: {type: 'block'}, condition: { urlFilter: 'frame' } }] })",
@" dynamicRules = await browser.declarativeNetRequest.getDynamicRules()",
@" browser.test.assertEq(dynamicRules.length, 1)",
// Yield after updating the dynamic rules so we can unload and re-load the extension.
@" browser.test.yield('Unload extension')",
@"} else {",
@" browser.test.assertEq(dynamicRules.length, 1)",
@" browser.test.yield('Load Tab')",
@"}"
]);

auto *declarativeNetRequestManifest = @{
Expand All @@ -569,14 +572,22 @@
};

auto extension = adoptNS([[_WKWebExtension alloc] _initWithManifestDictionary:declarativeNetRequestManifest resources:@{ @"background.js": backgroundScript }]);
auto manager = adoptNS([[TestWebExtensionManager alloc] initForExtension:extension.get()]);

auto manager = adoptNS([[TestWebExtensionManager alloc] initForExtension:extension.get() extensionControllerConfiguration:_WKWebExtensionControllerConfiguration._temporaryConfiguration]);

// Give the extension a unique identifier so it opts into saving data in the temporary configuration.
manager.get().context.uniqueIdentifier = @"org.webkit.test.extension (76C788B8)";

// Grant the declarativeNetRequest permission.
[manager.get().context setPermissionStatus:_WKWebExtensionContextPermissionStatusGrantedExplicitly forPermission:_WKWebExtensionPermissionDeclarativeNetRequest];

[manager loadAndRun];

// FIXME: We should attempt to unload and reload the extension context to verify that the dynamic rules are persisted, but this won't work since tests are run with non-persistent storage.
EXPECT_NS_EQUAL(manager.get().yieldMessage, @"Unload extension");

[manager.get().controller unloadExtensionContext:manager.get().context error:nullptr];

[manager loadAndRun];

EXPECT_NS_EQUAL(manager.get().yieldMessage, @"Load Tab");

Expand Down

0 comments on commit b094502

Please sign in to comment.