Skip to content

Commit

Permalink
Fix flaky WKWebExtensionAPIWebRequest tests
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=270824
rdar://124401551

Reviewed by Timothy Hatcher.

There were a few cascading issues here.

1) For webRequest, we were trying to create a persistent background page with manifest v3. This is not a supported
configuration, so it was being forced into being non-persistent.

This meant that the background page could sometimes unload, leading to the flakiness described in the bug.

2) After making this change, I discovered that we were attempting to load the background page before it was safe (before
moveLocalStorageIfNeeded was called).

To fix this, add a new variable tracking if it's safe to load the background view. This is set to true in loadBackgroundWebViewDuringLoad,
and checked in loadBackgroundWebViewIfNeeded before attempting to load the background web view.

* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm:
(WebKit::WebExtensionContext::loadBackgroundWebViewDuringLoad):
(WebKit::WebExtensionContext::loadBackgroundWebViewIfNeeded):
(WebKit::WebExtensionContext::loadBackgroundWebView):
* Source/WebKit/UIProcess/Extensions/WebExtensionContext.h:
(WebKit::WebExtensionContext::safeToLoadBackgroundContent const):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIWebRequest.mm:

Canonical link: https://commits.webkit.org/275955@main
  • Loading branch information
b-weinstein authored and xeenon committed Mar 12, 2024
1 parent 1468afa commit 5e785e8
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3013,6 +3013,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
if (!extension().hasBackgroundContent())
return;

m_safeToLoadBackgroundContent = true;
m_shouldFireStartupEvent = extensionController()->isFreshlyCreated();

queueStartupAndInstallEventsForExtensionIfNecessary();
Expand Down Expand Up @@ -3041,7 +3042,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
{
ASSERT(isLoaded());

if (!extension().hasBackgroundContent() || m_backgroundWebView)
if (!extension().hasBackgroundContent() || m_backgroundWebView || !safeToLoadBackgroundContent())
return;

loadBackgroundWebView();
Expand All @@ -3056,6 +3057,8 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)

RELEASE_LOG_DEBUG(Extensions, "Loading background content");

ASSERT(safeToLoadBackgroundContent());

ASSERT(!m_backgroundContentIsLoaded);
m_backgroundContentIsLoaded = false;

Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/UIProcess/Extensions/WebExtensionContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ class WebExtensionContext : public API::ObjectImpl<API::Object::Type::WebExtensi

URL backgroundContentURL();
WKWebView *backgroundWebView() const { return m_backgroundWebView.get(); }
bool safeToLoadBackgroundContent() const { return m_safeToLoadBackgroundContent; }

bool decidePolicyForNavigationAction(WKWebView *, WKNavigationAction *);
void didFinishNavigation(WKWebView *, WKNavigation *);
Expand Down Expand Up @@ -878,6 +879,7 @@ class WebExtensionContext : public API::ObjectImpl<API::Object::Type::WebExtensi
std::unique_ptr<WebCore::Timer> m_unloadBackgroundWebViewTimer;
MonotonicTime m_lastBackgroundPortActivityTime;
bool m_backgroundContentIsLoaded { false };
bool m_safeToLoadBackgroundContent { false };

#if ENABLE(INSPECTOR_EXTENSIONS)
WeakHashMap<WebInspectorUIProxy, TabIdentifierWebViewPair> m_inspectorBackgroundPageMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

#if PLATFORM(MAC)

static auto *webRequestManifest = @{ @"manifest_version": @3, @"permissions": @[ @"webRequest" ], @"background": @{ @"scripts": @[ @"background.js" ], @"type": @"module", @"persistent": @YES } };
static auto *webRequestManifest = @{ @"manifest_version": @2, @"permissions": @[ @"webRequest" ], @"background": @{ @"scripts": @[ @"background.js" ], @"type": @"module", @"persistent": @YES } };

TEST(WKWebExtensionAPIWebRequest, EventListenerTest)
{
Expand Down

0 comments on commit 5e785e8

Please sign in to comment.