Diff
Modified: trunk/Source/WebCore/ChangeLog (249747 => 249748)
--- trunk/Source/WebCore/ChangeLog 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebCore/ChangeLog 2019-09-11 00:54:05 UTC (rev 249748)
@@ -1,3 +1,21 @@
+2019-09-10 Chris Dumez <cdu...@apple.com>
+
+ Nullptr crash in Page::sessionID() via WebKit::WebFrameLoaderClient::detachedFromParent2()
+ https://bugs.webkit.org/show_bug.cgi?id=201625
+
+ Reviewed by Ryosuke Niwa.
+
+ This is based on a patch from Ryosuke Niwa.
+
+ Drop setHasFrameSpecificStorageAccess() in WebCore and call it from the WebKit layer instead.
+
+ * dom/DocumentStorageAccess.cpp:
+ (WebCore::DocumentStorageAccess::requestStorageAccess):
+ (WebCore::DocumentStorageAccess::setHasFrameSpecificStorageAccess): Deleted.
+ * dom/DocumentStorageAccess.h:
+ * loader/EmptyFrameLoaderClient.h:
+ * loader/FrameLoaderClient.h:
+
2019-09-10 Brady Eidson <beid...@apple.com>
Add SPI to save a PDF from the contents of a WKWebView.
Modified: trunk/Source/WebCore/dom/DocumentStorageAccess.cpp (249747 => 249748)
--- trunk/Source/WebCore/dom/DocumentStorageAccess.cpp 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebCore/dom/DocumentStorageAccess.cpp 2019-09-11 00:54:05 UTC (rev 249748)
@@ -181,10 +181,9 @@
}));
}
- if (wasGranted == StorageAccessWasGranted::Yes) {
- setHasFrameSpecificStorageAccess(true);
+ if (wasGranted == StorageAccessWasGranted::Yes)
promise->resolve();
- } else {
+ else {
if (promptWasShown == StorageAccessPromptWasShown::Yes)
setWasExplicitlyDeniedFrameSpecificStorageAccess();
promise->reject();
@@ -215,12 +214,6 @@
return frame && frame->loader().client().hasFrameSpecificStorageAccess();
}
-void DocumentStorageAccess::setHasFrameSpecificStorageAccess(bool value)
-{
- if (auto* frame = m_document.frame())
- frame->loader().client().setHasFrameSpecificStorageAccess(value);
-}
-
} // namespace WebCore
#endif // ENABLE(RESOURCE_LOAD_STATISTICS)
Modified: trunk/Source/WebCore/dom/DocumentStorageAccess.h (249747 => 249748)
--- trunk/Source/WebCore/dom/DocumentStorageAccess.h 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebCore/dom/DocumentStorageAccess.h 2019-09-11 00:54:05 UTC (rev 249748)
@@ -65,7 +65,6 @@
static DocumentStorageAccess* from(Document&);
static const char* supplementName();
bool hasFrameSpecificStorageAccess() const;
- void setHasFrameSpecificStorageAccess(bool);
void setWasExplicitlyDeniedFrameSpecificStorageAccess() { ++m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess; };
bool isAllowedToRequestFrameSpecificStorageAccess() { return m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess < maxNumberOfTimesExplicitlyDeniedFrameSpecificStorageAccess; };
void enableTemporaryTimeUserGesture();
Modified: trunk/Source/WebCore/loader/EmptyFrameLoaderClient.h (249747 => 249748)
--- trunk/Source/WebCore/loader/EmptyFrameLoaderClient.h 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebCore/loader/EmptyFrameLoaderClient.h 2019-09-11 00:54:05 UTC (rev 249748)
@@ -208,7 +208,6 @@
#endif
#if ENABLE(RESOURCE_LOAD_STATISTICS)
bool hasFrameSpecificStorageAccess() final { return false; }
- void setHasFrameSpecificStorageAccess(bool) final { }
#endif
};
Modified: trunk/Source/WebCore/loader/FrameLoaderClient.h (249747 => 249748)
--- trunk/Source/WebCore/loader/FrameLoaderClient.h 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebCore/loader/FrameLoaderClient.h 2019-09-11 00:54:05 UTC (rev 249748)
@@ -379,7 +379,6 @@
#if ENABLE(RESOURCE_LOAD_STATISTICS)
virtual bool hasFrameSpecificStorageAccess() { return false; }
- virtual void setHasFrameSpecificStorageAccess(bool) { }
#endif
};
Modified: trunk/Source/WebKit/ChangeLog (249747 => 249748)
--- trunk/Source/WebKit/ChangeLog 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebKit/ChangeLog 2019-09-11 00:54:05 UTC (rev 249748)
@@ -1,3 +1,31 @@
+2019-09-10 Chris Dumez <cdu...@apple.com>
+
+ Nullptr crash in Page::sessionID() via WebKit::WebFrameLoaderClient::detachedFromParent2()
+ https://bugs.webkit.org/show_bug.cgi?id=201625
+
+ Reviewed by Ryosuke Niwa.
+
+ This is based on a patch from Ryosuke Niwa.
+
+ The crash was caused by WebFrameLoaderClient::sessionID() calling WebPage::sessionID() without
+ checking the nullity of WebPage::m_page which can be null. Added a null check.
+
+ Because passing a wrong session to RemoveStorageAccessForFrame could result in a leak, this patch
+ also replaces m_hasFrameSpecificStorageAccess boolean with an optioanl struct which stores
+ session ID, frame ID, and page ID even after WebCore::Frame or WebCore::Page had been cleared
+ or before WebFrameLoaderClient::m_frame is set.
+
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::sessionID const):
+ (WebKit::WebFrameLoaderClient::setHasFrameSpecificStorageAccess):
+ (WebKit::WebFrameLoaderClient::detachedFromParent2):
+ (WebKit::WebFrameLoaderClient::dispatchWillChangeDocument):
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+ * WebProcess/WebPage/WebFrame.h:
+ (WebKit::WebFrame::frameLoaderClient const):
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::requestStorageAccess):
+
2019-09-10 Brady Eidson <beid...@apple.com>
Add SPI to save a PDF from the contents of a WKWebView.
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (249747 => 249748)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2019-09-11 00:54:05 UTC (rev 249748)
@@ -138,9 +138,23 @@
PAL::SessionID WebFrameLoaderClient::sessionID() const
{
- return m_frame && m_frame->page() ? m_frame->page()->sessionID() : PAL::SessionID::defaultSessionID();
+ WebPage* page = m_frame ? m_frame->page() : nullptr;
+ if (!page || !page->corePage()) {
+ ASSERT_NOT_REACHED();
+ return PAL::SessionID::defaultSessionID();
+ }
+ return page->sessionID();
}
+#if ENABLE(RESOURCE_LOAD_STATISTICS)
+void WebFrameLoaderClient::setHasFrameSpecificStorageAccess(FrameSpecificStorageAccessIdentifier&& frameSpecificStorageAccessIdentifier )
+{
+ ASSERT(!m_frameSpecificStorageAccessIdentifier);
+
+ m_frameSpecificStorageAccessIdentifier = WTFMove(frameSpecificStorageAccessIdentifier);
+}
+#endif
+
void WebFrameLoaderClient::frameLoaderDestroyed()
{
m_frame->invalidate();
@@ -181,9 +195,10 @@
return;
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- if (m_hasFrameSpecificStorageAccess) {
- WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::RemoveStorageAccessForFrame(sessionID(), frameID().value(), pageID().value()), 0);
- m_hasFrameSpecificStorageAccess = false;
+ if (m_frameSpecificStorageAccessIdentifier) {
+ WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::RemoveStorageAccessForFrame(
+ m_frameSpecificStorageAccessIdentifier->sessionID, m_frameSpecificStorageAccessIdentifier->frameID, m_frameSpecificStorageAccessIdentifier->pageID), 0);
+ m_frameSpecificStorageAccessIdentifier = WTF::nullopt;
}
#endif
@@ -408,9 +423,10 @@
if (!webPage)
return;
- if (m_hasFrameSpecificStorageAccess && !WebCore::areRegistrableDomainsEqual(currentUrl, newUrl)) {
- WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::RemoveStorageAccessForFrame(sessionID(), frameID().value(), pageID().value()), 0);
- m_hasFrameSpecificStorageAccess = false;
+ if (m_frameSpecificStorageAccessIdentifier && !WebCore::areRegistrableDomainsEqual(currentUrl, newUrl)) {
+ WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::RemoveStorageAccessForFrame(
+ m_frameSpecificStorageAccessIdentifier->sessionID, m_frameSpecificStorageAccessIdentifier->frameID, m_frameSpecificStorageAccessIdentifier->pageID), 0);
+ m_frameSpecificStorageAccessIdentifier = WTF::nullopt;
}
#endif
}
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h (249747 => 249748)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h 2019-09-11 00:54:05 UTC (rev 249748)
@@ -27,6 +27,7 @@
#include "WebPageProxyIdentifier.h"
#include <WebCore/FrameLoaderClient.h>
+#include <pal/SessionID.h>
namespace PAL {
class SessionID;
@@ -58,8 +59,14 @@
PAL::SessionID sessionID() const final;
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- bool hasFrameSpecificStorageAccess() { return m_hasFrameSpecificStorageAccess; }
- void setHasFrameSpecificStorageAccess(bool value) { m_hasFrameSpecificStorageAccess = value; };
+ bool hasFrameSpecificStorageAccess() final { return !!m_frameSpecificStorageAccessIdentifier; }
+
+ struct FrameSpecificStorageAccessIdentifier {
+ PAL::SessionID sessionID;
+ WebCore::FrameIdentifier frameID;
+ WebCore::PageIdentifier pageID;
+ };
+ void setHasFrameSpecificStorageAccess(FrameSpecificStorageAccessIdentifier&&);
#endif
private:
@@ -284,7 +291,7 @@
bool m_frameCameFromPageCache;
bool m_useIconLoadingClient { false };
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- bool m_hasFrameSpecificStorageAccess { false };
+ Optional<FrameSpecificStorageAccessIdentifier> m_frameSpecificStorageAccessIdentifier;
#endif
};
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h (249747 => 249748)
--- trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h 2019-09-11 00:54:05 UTC (rev 249748)
@@ -172,6 +172,8 @@
void setFirstLayerTreeTransactionIDAfterDidCommitLoad(TransactionID transactionID) { m_firstLayerTreeTransactionIDAfterDidCommitLoad = transactionID; }
#endif
+ WebFrameLoaderClient* frameLoaderClient() const { return m_frameLoaderClient.get(); }
+
private:
static Ref<WebFrame> create(std::unique_ptr<WebFrameLoaderClient>);
explicit WebFrame(std::unique_ptr<WebFrameLoaderClient>);
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (249747 => 249748)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-09-11 00:46:32 UTC (rev 249747)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-09-11 00:54:05 UTC (rev 249748)
@@ -6538,7 +6538,11 @@
void WebPage::requestStorageAccess(RegistrableDomain&& subFrameDomain, RegistrableDomain&& topFrameDomain, WebFrame& frame, CompletionHandler<void(WebCore::StorageAccessWasGranted, WebCore::StorageAccessPromptWasShown)>&& completionHandler)
{
- WebProcess::singleton().ensureNetworkProcessConnection().connection().sendWithAsyncReply(Messages::NetworkConnectionToWebProcess::RequestStorageAccess(sessionID(), WTFMove(subFrameDomain), WTFMove(topFrameDomain), frame.frameID(), m_identifier, m_webPageProxyIdentifier), WTFMove(completionHandler));
+ WebProcess::singleton().ensureNetworkProcessConnection().connection().sendWithAsyncReply(Messages::NetworkConnectionToWebProcess::RequestStorageAccess(sessionID(), WTFMove(subFrameDomain), WTFMove(topFrameDomain), frame.frameID(), m_identifier, m_webPageProxyIdentifier), [completionHandler = WTFMove(completionHandler), frame = makeRef(frame), sessionID = sessionID(), pageID = m_identifier, frameID = frame.frameID()](StorageAccessWasGranted wasGranted, WebCore::StorageAccessPromptWasShown promptWasShown) mutable {
+ if (wasGranted == StorageAccessWasGranted::Yes)
+ frame->frameLoaderClient()->setHasFrameSpecificStorageAccess({ sessionID, frameID, pageID });
+ completionHandler(wasGranted, promptWasShown);
+ });
}
void WebPage::wasLoadedWithDataTransferFromPrevalentResource()
