Skip to content

Commit

Permalink
REGRESSION (275819@main): 5 Tests in TestWebKitAPI.WKWebExtensionAPIW…
Browse files Browse the repository at this point in the history
…ebRequest sometimes timeout (270812).

https://webkit.org/b/271392
rdar://124401551

Reviewed by Brian Weinstein.

Change from using webView:didFinishNavigation: to _webView:navigationDidFinishDocumentLoad: for
tracking when the background page has finished loading. This will properly fire after modules have
executed, or any async script elements in a custom background page.

This allows us to remove the arbitrary delay we had before, which was wholly inadequate across different
machines, and was firing too soon for slow machines. By waiting for the document load, we know any
background content event listeners will be registered and can properly dispatch events.

* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm:
(-[_WKWebExtensionContextDelegate _webView:navigationDidFinishDocumentLoad:]): Added.
(WebKit::WebExtensionContext::performTasksAfterBackgroundContentLoads): Removed dispatch_after().
(WebKit::WebExtensionContext::didFinishDocumentLoad):
(-[_WKWebExtensionContextDelegate webView:didFinishNavigation:]): Deleted.
(WebKit::WebExtensionContext::didFinishNavigation): Deleted.
* Source/WebKit/UIProcess/Extensions/WebExtensionContext.h:

Canonical link: https://commits.webkit.org/276487@main
  • Loading branch information
xeenon committed Mar 21, 2024
1 parent 29b1ce3 commit 3fd7495
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,12 @@ - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigati
decisionHandler(WKNavigationActionPolicyCancel);
}

- (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
- (void)_webView:(WKWebView *)webView navigationDidFinishDocumentLoad:(WKNavigation *)navigation
{
if (!_webExtensionContext)
return;

_webExtensionContext->didFinishNavigation(webView, navigation);
_webExtensionContext->didFinishDocumentLoad(webView, navigation);
}

- (void)webView:(WKWebView *)webView didFailNavigation:(WKNavigation *)navigation withError:(NSError *)error
Expand Down Expand Up @@ -3326,38 +3326,33 @@ static inline bool isNotRunningInTestRunner()

void WebExtensionContext::performTasksAfterBackgroundContentLoads()
{
RELEASE_LOG_DEBUG(Extensions, "Performing tasks soon after background content loads");
if (!isLoaded())
return;

constexpr auto performDelay = 100_ms;
RELEASE_LOG_DEBUG(Extensions, "Background content loaded");

// Delay to give time for addListener messages to register the events, this is needed because modules execute after page load fires.
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, performDelay.nanosecondsAs<int64_t>()), dispatch_get_main_queue(), makeBlockPtr([this, protectedThis = Ref { *this }] {
if (!isLoaded())
return;

if (m_shouldFireStartupEvent) {
fireRuntimeStartupEventIfNeeded();
m_shouldFireStartupEvent = false;
}
if (m_shouldFireStartupEvent) {
fireRuntimeStartupEventIfNeeded();
m_shouldFireStartupEvent = false;
}

if (m_installReason != InstallReason::None) {
fireRuntimeInstalledEventIfNeeded();
if (m_installReason != InstallReason::None) {
fireRuntimeInstalledEventIfNeeded();

m_installReason = InstallReason::None;
m_previousVersion = nullString();
}
m_installReason = InstallReason::None;
m_previousVersion = nullString();
}

RELEASE_LOG_DEBUG(Extensions, "Performing %{public}zu task(s) after background content loaded", m_actionsToPerformAfterBackgroundContentLoads.size());
RELEASE_LOG_DEBUG(Extensions, "Performing %{public}zu task(s) after background content loaded", m_actionsToPerformAfterBackgroundContentLoads.size());

for (auto& action : m_actionsToPerformAfterBackgroundContentLoads)
action();
for (auto& action : m_actionsToPerformAfterBackgroundContentLoads)
action();

m_backgroundContentIsLoaded = true;
m_actionsToPerformAfterBackgroundContentLoads.clear();
m_backgroundContentIsLoaded = true;
m_actionsToPerformAfterBackgroundContentLoads.clear();

saveBackgroundPageListenersToStorage();
scheduleBackgroundContentToUnload();
}).get());
saveBackgroundPageListenersToStorage();
scheduleBackgroundContentToUnload();
}

void WebExtensionContext::wakeUpBackgroundContentIfNecessary(CompletionHandler<void()>&& completionHandler)
Expand Down Expand Up @@ -3422,12 +3417,11 @@ static inline bool isNotRunningInTestRunner()
return false;
}

void WebExtensionContext::didFinishNavigation(WKWebView *webView, WKNavigation *)
void WebExtensionContext::didFinishDocumentLoad(WKWebView *webView, WKNavigation *)
{
if (webView != m_backgroundWebView)
return;

// When didFinishNavigation fires for a service worker, the service worker has not executed yet.
// The service worker will notify the load via a completion handler instead.
if (extension().backgroundContentIsServiceWorker())
return;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/Extensions/WebExtensionContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ class WebExtensionContext : public API::ObjectImpl<API::Object::Type::WebExtensi
bool safeToLoadBackgroundContent() const { return m_safeToLoadBackgroundContent; }

bool decidePolicyForNavigationAction(WKWebView *, WKNavigationAction *);
void didFinishNavigation(WKWebView *, WKNavigation *);
void didFinishDocumentLoad(WKWebView *, WKNavigation *);
void didFailNavigation(WKWebView *, WKNavigation *, NSError *);
void webViewWebContentProcessDidTerminate(WKWebView *);

Expand Down

0 comments on commit 3fd7495

Please sign in to comment.