Skip to content

Commit

Permalink
REGRESSION (275244@main): Crash in MediaCapability::MediaCapability w…
Browse files Browse the repository at this point in the history
…hen loading 'about:blank'

https://bugs.webkit.org/show_bug.cgi?id=270318
rdar://123856265

Reviewed by Jer Noble.

In 275244@main MediaCapability was changed to track a SecurityOrigin rather than a URL. Since
'about:blank' is considered an opaque origin, a nil NSURL is returned by SecurityOrigin::toURL()
(after implicit conversion). A crash occurs when MediaCapability attempts to instantiate a
BEMediaEnvironment with the nil URL since -initWithWebPageURL: requires a nonnull NSURL.

Fixed this by reverting MediaCapability to storing the webpage URL as a WebCore::URL, and instead
using URL::protocolHostAndPort() and protocolHostAndPortAreEqual() to ensure that the
MediaEnvironment is not reset during same-origin navigations.

* Source/WebKit/Platform/cocoa/MediaCapability.h:
* Source/WebKit/Platform/cocoa/MediaCapability.mm:
(WebKit::createMediaEnvironment):
(WebKit::MediaCapability::MediaCapability):
* Source/WebKit/SourcesCocoa.txt:
* Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::WebPageProxy::setMediaCapability):
(WebKit::WebPageProxy::deactivateMediaCapability):
(WebKit::WebPageProxy::resetMediaCapability):
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:

Canonical link: https://commits.webkit.org/275533@main
  • Loading branch information
aestes committed Mar 1, 2024
1 parent c957547 commit 144facc
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 22 deletions.
12 changes: 4 additions & 8 deletions Source/WebKit/Platform/cocoa/MediaCapability.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,9 @@
#include "ExtensionCapability.h"
#include <wtf/Forward.h>
#include <wtf/Noncopyable.h>
#include <wtf/Ref.h>
#include <wtf/URL.h>
#include <wtf/WeakPtr.h>

namespace WebCore {
class SecurityOrigin;
}

OBJC_CLASS BEMediaEnvironment;

namespace WebKit {
Expand All @@ -46,7 +42,7 @@ class ExtensionCapabilityGrant;
class MediaCapability final : public ExtensionCapability, public CanMakeWeakPtr<MediaCapability> {
WTF_MAKE_NONCOPYABLE(MediaCapability);
public:
explicit MediaCapability(Ref<WebCore::SecurityOrigin>&&);
explicit MediaCapability(URL&&);
MediaCapability(MediaCapability&&) = default;
MediaCapability& operator=(MediaCapability&&) = default;

Expand All @@ -61,7 +57,7 @@ class MediaCapability final : public ExtensionCapability, public CanMakeWeakPtr<
void setState(State state) { m_state = state; }
bool isActivatingOrActive() const;

const WebCore::SecurityOrigin& securityOrigin() const { return m_securityOrigin.get(); }
const URL& webPageURL() const { return m_webPageURL; }

// ExtensionCapability
String environmentIdentifier() const final;
Expand All @@ -70,7 +66,7 @@ class MediaCapability final : public ExtensionCapability, public CanMakeWeakPtr<

private:
State m_state { State::Inactive };
Ref<WebCore::SecurityOrigin> m_securityOrigin;
URL m_webPageURL;
RetainPtr<BEMediaEnvironment> m_mediaEnvironment;
};

Expand Down
15 changes: 10 additions & 5 deletions Source/WebKit/Platform/cocoa/MediaCapability.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,18 @@
#import <WebCore/SecurityOrigin.h>
#import <wtf/text/WTFString.h>

#import "ExtensionKitSoftLink.h"

namespace WebKit {

MediaCapability::MediaCapability(Ref<WebCore::SecurityOrigin>&& securityOrigin)
: m_securityOrigin { WTFMove(securityOrigin) }
, m_mediaEnvironment { adoptNS([[BEMediaEnvironment alloc] initWithWebPageURL:m_securityOrigin->toURL()]) }
static RetainPtr<BEMediaEnvironment> createMediaEnvironment(const URL& webPageURL)
{
NSURL *protocolHostAndPortURL = URL { webPageURL.protocolHostAndPort() };
RELEASE_ASSERT(protocolHostAndPortURL);
return adoptNS([[BEMediaEnvironment alloc] initWithWebPageURL:protocolHostAndPortURL]);
}

MediaCapability::MediaCapability(URL&& webPageURL)
: m_webPageURL { WTFMove(webPageURL) }
, m_mediaEnvironment { createMediaEnvironment(m_webPageURL) }
{
setPlatformCapability([BEProcessCapability mediaPlaybackAndCaptureWithEnvironment:m_mediaEnvironment.get()]);
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/SourcesCocoa.txt
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Platform/classifier/ResourceLoadStatisticsClassifier.cpp

Platform/cocoa/AssertionCapability.mm @no-unify
Platform/cocoa/ExtensionCapabilityGrant.mm
Platform/cocoa/MediaCapability.mm @no-unify
Platform/cocoa/MediaCapability.mm
Platform/cocoa/PaymentAuthorizationPresenter.mm
Platform/cocoa/PaymentAuthorizationViewController.mm
Platform/cocoa/WKPaymentAuthorizationDelegate.mm
Expand Down
10 changes: 4 additions & 6 deletions Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1043,13 +1043,13 @@ static bool exceedsRenderTreeSizeSizeThreshold(uint64_t thresholdSize, uint64_t
return;
}

WEBPAGEPROXY_RELEASE_LOG(ProcessCapabilities, "setMediaCapability: creating (envID=%{public}s) for host '%{sensitive}s'", internals().mediaCapability->environmentIdentifier().utf8().data(), internals().mediaCapability->securityOrigin().host().utf8().data());
WEBPAGEPROXY_RELEASE_LOG(ProcessCapabilities, "setMediaCapability: creating (envID=%{public}s) for URL '%{sensitive}s'", internals().mediaCapability->environmentIdentifier().utf8().data(), internals().mediaCapability->webPageURL().string().utf8().data());
send(Messages::WebPage::SetMediaEnvironment(internals().mediaCapability->environmentIdentifier()));
}

void WebPageProxy::deactivateMediaCapability(MediaCapability& capability)
{
WEBPAGEPROXY_RELEASE_LOG(ProcessCapabilities, "deactivateMediaCapability: deactivating (envID=%{public}s) for host '%{sensitive}s'", capability.environmentIdentifier().utf8().data(), capability.securityOrigin().host().utf8().data());
WEBPAGEPROXY_RELEASE_LOG(ProcessCapabilities, "deactivateMediaCapability: deactivating (envID=%{public}s) for URL '%{sensitive}s'", capability.environmentIdentifier().utf8().data(), capability.webPageURL().string().utf8().data());
Ref processPool { protectedProcess()->protectedProcessPool() };
processPool->extensionCapabilityGranter().setMediaCapabilityActive(capability, false);
processPool->extensionCapabilityGranter().revoke(capability);
Expand All @@ -1067,10 +1067,8 @@ static bool exceedsRenderTreeSizeSizeThreshold(uint64_t thresholdSize, uint64_t
return;
}

Ref securityOrigin = SecurityOrigin::create(currentURL);

if (!mediaCapability() || !mediaCapability()->securityOrigin().isSameOriginAs(securityOrigin.get()))
setMediaCapability(MediaCapability { WTFMove(securityOrigin) });
if (!mediaCapability() || !protocolHostAndPortAreEqual(mediaCapability()->webPageURL(), currentURL))
setMediaCapability(MediaCapability { WTFMove(currentURL) });
}

void WebPageProxy::updateMediaCapability()
Expand Down
2 changes: 0 additions & 2 deletions Source/WebKit/WebKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1901,7 +1901,6 @@
A13B3DA2207F39DE0090C58D /* MobileWiFiSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = A13B3DA1207F39DE0090C58D /* MobileWiFiSPI.h */; };
A13DC682207AA6B20066EF72 /* WKApplicationStateTrackingView.h in Headers */ = {isa = PBXBuildFile; fileRef = A13DC680207AA6B20066EF72 /* WKApplicationStateTrackingView.h */; };
A141DF4F2B06ED0800E80B2D /* MediaCapability.h in Headers */ = {isa = PBXBuildFile; fileRef = A141DF4D2B06ECFB00E80B2D /* MediaCapability.h */; };
A141DF502B06ED0F00E80B2D /* MediaCapability.mm in Sources */ = {isa = PBXBuildFile; fileRef = A141DF4E2B06ECFB00E80B2D /* MediaCapability.mm */; };
A141DF522B07054900E80B2D /* ExtensionCapabilityGrant.h in Headers */ = {isa = PBXBuildFile; fileRef = A141DF512B07054900E80B2D /* ExtensionCapabilityGrant.h */; };
A14F9B632B686DD300AD9C56 /* WKSLinearMediaTypes.h in Headers */ = {isa = PBXBuildFile; fileRef = A14F9B612B686DD200AD9C56 /* WKSLinearMediaTypes.h */; settings = {ATTRIBUTES = (Private, ); }; };
A14F9B642B686DD300AD9C56 /* LinearMediaTypes.swift in Sources */ = {isa = PBXBuildFile; fileRef = A14F9B622B686DD200AD9C56 /* LinearMediaTypes.swift */; };
Expand Down Expand Up @@ -19146,7 +19145,6 @@
41A0EB142641714900794471 /* LibWebRTCCodecsProxy.mm in Sources */,
51F060E11654318500F3281C /* LibWebRTCNetworkMessageReceiver.cpp in Sources */,
449D90DA21FDC30B00F677C0 /* LocalAuthenticationSoftLink.mm in Sources */,
A141DF502B06ED0F00E80B2D /* MediaCapability.mm in Sources */,
07E19EFB23D401F10094FFB4 /* MediaPlayerPrivateRemoteMessageReceiver.cpp in Sources */,
510C52D32B241019008EABED /* MediaSourcePrivateRemoteMessageReceiverMessageReceiver.cpp in Sources */,
9B4790912531563200EC11AB /* MessageArgumentDescriptions.cpp in Sources */,
Expand Down

0 comments on commit 144facc

Please sign in to comment.