Skip to content

Commit

Permalink
Safari crashes at WebKit::WebExtensionContext::openInspectors.
Browse files Browse the repository at this point in the history
https://webkit.org/b/273560
rdar://127189700

Reviewed by Brian Weinstein.

Add a null check for _WKInspector befor tryign to get API::Inspector from it.

Also moved extensionHasAccess() check to openWindows() and openTabs(), so the
caller does not need to do it. A extensionHasAccess() check was missing in
the openInspectors() loop of openTabs().

Also skip extra work with early returns for !hasInspectorBackgroundPage().

Finally, adopt WTF::compactMap and WTF::map() more.

* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionContext.mm:
(-[_WKWebExtensionContext openWindows]): Pass IgnoreExtensionAccess::Yes since API should see all.
(-[_WKWebExtensionContext openTabs]): Ditto.
* Source/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPICookiesCocoa.mm:
(WebKit::WebExtensionContext::cookiesGetAllCookieStores):
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm:
(WebKit::WebExtensionContext::openWindows const):
(WebKit::WebExtensionContext::openTabs const):
(WebKit::WebExtensionContext::openInspectors const):
(WebKit::WebExtensionContext::loadedInspectors const):
(WebKit::WebExtensionContext::inspectorExtension const):
(WebKit::WebExtensionContext::inspector const):
(WebKit::WebExtensionContext::processes const):
(WebKit::WebExtensionContext::isInspectorBackgroundPage const):
(WebKit::WebExtensionContext::isDevToolsMessageAllowed): Removed #ifdef since this method
is already guarded by it.
* Source/WebKit/UIProcess/Extensions/WebExtensionContext.h:

Canonical link: https://commits.webkit.org/278239@main
  • Loading branch information
xeenon committed May 2, 2024
1 parent d0941d2 commit 484a4fe
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 33 deletions.
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionContext.mm
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ - (void)clearUserGestureInTab:(id<_WKWebExtensionTab>)tab

- (NSArray<id<_WKWebExtensionWindow>> *)openWindows
{
return toAPI(_webExtensionContext->openWindows());
return toAPI(_webExtensionContext->openWindows(WebKit::WebExtensionContext::IgnoreExtensionAccess::Yes));
}

- (id<_WKWebExtensionWindow>)focusedWindow
Expand All @@ -666,7 +666,7 @@ - (void)clearUserGestureInTab:(id<_WKWebExtensionTab>)tab

- (NSSet<id<_WKWebExtensionTab>> *)openTabs
{
return toAPI(_webExtensionContext->openTabs());
return toAPI(_webExtensionContext->openTabs(WebKit::WebExtensionContext::IgnoreExtensionAccess::Yes));
}

static inline Ref<WebKit::WebExtensionWindow> toImpl(id<_WKWebExtensionWindow> window, WebKit::WebExtensionContext& context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,6 @@ static inline URL toURL(const WebCore::Cookie& cookie)
stores.set(defaultSessionID, Vector<WebExtensionTabIdentifier> { });

for (Ref tab : openTabs()) {
if (!tab->extensionHasAccess())
continue;

for (WKWebView *webView in tab->webViews()) {
auto sessionID = webView.configuration.websiteDataStore->_websiteDataStore.get()->sessionID();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1987,26 +1987,25 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
return tab.isValid() && tab.extensionContext() == this && m_tabMap.get(tab.identifier()) == &tab;
}

WebExtensionContext::WindowVector WebExtensionContext::openWindows() const
WebExtensionContext::WindowVector WebExtensionContext::openWindows(IgnoreExtensionAccess ignoreExtensionAccess) const
{
return WTF::map(m_windowOrderVector, [&](auto& identifier) -> Ref<WebExtensionWindow> {
return WTF::compactMap(m_windowOrderVector, [&](auto& identifier) -> std::optional<Ref<WebExtensionWindow>> {
RefPtr window = m_windowMap.get(identifier);
ASSERT(window && window->isOpen());

if (ignoreExtensionAccess == IgnoreExtensionAccess::No && !window->extensionHasAccess())
return std::nullopt;
return *window;
});
}

WebExtensionContext::TabVector WebExtensionContext::openTabs() const
WebExtensionContext::TabVector WebExtensionContext::openTabs(IgnoreExtensionAccess ignoreExtensionAccess) const
{
TabVector result;
result.reserveInitialCapacity(m_tabMap.size());

for (Ref tab : m_tabMap.values()) {
if (tab->isOpen())
result.append(WTFMove(tab));
}

return result;
return WTF::compactMap(m_tabMap, [&](auto& entry) -> std::optional<Ref<WebExtensionTab>> {
if (ignoreExtensionAccess == IgnoreExtensionAccess::No && !entry.value->extensionHasAccess())
return std::nullopt;
return entry.value;
});
}

RefPtr<WebExtensionWindow> WebExtensionContext::focusedWindow(IgnoreExtensionAccess ignoreExtensionAccess) const
Expand Down Expand Up @@ -3526,14 +3525,20 @@ static inline bool isNotRunningInTestRunner()

WebExtensionContext::InspectorTabVector WebExtensionContext::openInspectors(Function<bool(WebExtensionTab&, WebInspectorUIProxy&)>&& predicate) const
{
ASSERT(isLoaded());

if (!extension().hasInspectorBackgroundPage())
return { };

InspectorTabVector result;

for (Ref tab : openTabs()) {
if (!tab->extensionHasAccess())
continue;

for (WKWebView *webView in tab->webViews()) {
Ref inspector = *webView._inspector->_inspector;
auto *webInspector = webView._inspector;
if (!webInspector)
continue;

Ref inspector = *webInspector->_inspector;
if (inspector->isVisible() && (!predicate || predicate(tab, inspector)))
result.append({ inspector, tab.ptr() });
}
Expand All @@ -3544,14 +3549,24 @@ static inline bool isNotRunningInTestRunner()

WebExtensionContext::InspectorTabVector WebExtensionContext::loadedInspectors() const
{
ASSERT(isLoaded());

if (!extension().hasInspectorBackgroundPage())
return { };

InspectorTabVector result;

for (auto entry : m_inspectorBackgroundPageMap)
result.append({ entry.key, getTab(std::get<WebExtensionTabIdentifier>(entry.value)) });

return result;
}

RefPtr<API::InspectorExtension> WebExtensionContext::inspectorExtension(WebPageProxyIdentifier webPageProxyIdentifier) const
{
ASSERT(isLoaded());
ASSERT(extension().hasInspectorBackgroundPage());

RefPtr<WebInspectorUIProxy> foundInspector;

for (auto entry : m_inspectorBackgroundPageMap) {
Expand All @@ -3575,6 +3590,9 @@ static inline bool isNotRunningInTestRunner()

RefPtr<WebInspectorUIProxy> WebExtensionContext::inspector(const API::InspectorExtension& inspectorExtension) const
{
ASSERT(isLoaded());
ASSERT(extension().hasInspectorBackgroundPage());

for (auto entry : m_inspectorExtensionMap) {
if (entry.value.ptr() == &inspectorExtension)
return &entry.key;
Expand All @@ -3585,10 +3603,10 @@ static inline bool isNotRunningInTestRunner()

HashSet<Ref<WebProcessProxy>> WebExtensionContext::processes(const API::InspectorExtension& inspectorExtension) const
{
HashSet<Ref<WebProcessProxy>> result;
ASSERT(isLoaded());
ASSERT(extension().hasInspectorBackgroundPage());

if (!isLoaded())
return result;
HashSet<Ref<WebProcessProxy>> result;

RefPtr inspectorProxy = inspector(inspectorExtension);
if (!inspectorProxy)
Expand All @@ -3606,6 +3624,11 @@ static inline bool isNotRunningInTestRunner()

bool WebExtensionContext::isInspectorBackgroundPage(WKWebView *webView) const
{
ASSERT(isLoaded());

if (!extension().hasInspectorBackgroundPage())
return false;

for (auto entry : m_inspectorBackgroundPageMap) {
if (webView == std::get<RetainPtr<WKWebView>>(entry.value))
return true;
Expand All @@ -3616,13 +3639,7 @@ static inline bool isNotRunningInTestRunner()

bool WebExtensionContext::isDevToolsMessageAllowed()
{
#if ENABLE(INSPECTOR_EXTENSIONS)
if (!isLoaded())
return false;
return extension().hasInspectorBackgroundPage();
#else
return false;
#endif
return isLoaded() && extension().hasInspectorBackgroundPage();
}

void WebExtensionContext::loadInspectorBackgroundPagesDuringLoad()
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/Extensions/WebExtensionContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,8 @@ class WebExtensionContext : public API::ObjectImpl<API::Object::Type::WebExtensi

void openNewTab(const WebExtensionTabParameters&, CompletionHandler<void(RefPtr<WebExtensionTab>)>&&);

WindowVector openWindows() const;
TabVector openTabs() const;
WindowVector openWindows(IgnoreExtensionAccess = IgnoreExtensionAccess::No) const;
TabVector openTabs(IgnoreExtensionAccess = IgnoreExtensionAccess::No) const;

RefPtr<WebExtensionWindow> focusedWindow(IgnoreExtensionAccess = IgnoreExtensionAccess::No) const;
RefPtr<WebExtensionWindow> frontmostWindow(IgnoreExtensionAccess = IgnoreExtensionAccess::No) const;
Expand Down

0 comments on commit 484a4fe

Please sign in to comment.