Diff
Modified: branches/safari-608.2.11.1-branch/Source/WebCore/ChangeLog (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebCore/ChangeLog 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebCore/ChangeLog 2019-09-11 20:12:07 UTC (rev 249775)
@@ -1,3 +1,25 @@
+2019-09-11 Alan Coon <alanc...@apple.com>
+
+ Apply patch. rdar://problem/55240886
+
+ 2019-09-11 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-05 Kocsen Chung <kocsen_ch...@apple.com>
Cherry-pick r249534. rdar://problem/55084674
Modified: branches/safari-608.2.11.1-branch/Source/WebCore/dom/DocumentStorageAccess.cpp (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebCore/dom/DocumentStorageAccess.cpp 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebCore/dom/DocumentStorageAccess.cpp 2019-09-11 20:12:07 UTC (rev 249775)
@@ -189,10 +189,9 @@
}));
}
- if (wasGranted == StorageAccessWasGranted::Yes) {
- document->setHasFrameSpecificStorageAccess(true);
+ if (wasGranted == StorageAccessWasGranted::Yes)
promise->resolve();
- } else {
+ else {
if (promptWasShown == StorageAccessPromptWasShown::Yes)
document->setWasExplicitlyDeniedFrameSpecificStorageAccess();
promise->reject();
@@ -223,12 +222,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: branches/safari-608.2.11.1-branch/Source/WebCore/dom/DocumentStorageAccess.h (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebCore/dom/DocumentStorageAccess.h 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebCore/dom/DocumentStorageAccess.h 2019-09-11 20:12:07 UTC (rev 249775)
@@ -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: branches/safari-608.2.11.1-branch/Source/WebCore/loader/EmptyFrameLoaderClient.h (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebCore/loader/EmptyFrameLoaderClient.h 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebCore/loader/EmptyFrameLoaderClient.h 2019-09-11 20:12:07 UTC (rev 249775)
@@ -200,7 +200,6 @@
#endif
#if ENABLE(RESOURCE_LOAD_STATISTICS)
bool hasFrameSpecificStorageAccess() final { return false; }
- void setHasFrameSpecificStorageAccess(bool) final { }
#endif
};
Modified: branches/safari-608.2.11.1-branch/Source/WebCore/loader/FrameLoaderClient.h (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebCore/loader/FrameLoaderClient.h 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebCore/loader/FrameLoaderClient.h 2019-09-11 20:12:07 UTC (rev 249775)
@@ -377,7 +377,6 @@
#if ENABLE(RESOURCE_LOAD_STATISTICS)
virtual bool hasFrameSpecificStorageAccess() { return false; }
- virtual void setHasFrameSpecificStorageAccess(bool) { }
#endif
};
Modified: branches/safari-608.2.11.1-branch/Source/WebKit/ChangeLog (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebKit/ChangeLog 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebKit/ChangeLog 2019-09-11 20:12:07 UTC (rev 249775)
@@ -1,3 +1,35 @@
+2019-09-11 Alan Coon <alanc...@apple.com>
+
+ Apply patch. rdar://problem/55240886
+
+ 2019-09-11 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-09 Alan Coon <alanc...@apple.com>
Cherry-pick r249649. rdar://problem/55198064
Modified: branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2019-09-11 20:12:07 UTC (rev 249775)
@@ -130,9 +130,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();
@@ -173,9 +187,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
@@ -400,9 +415,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: branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h 2019-09-11 20:12:07 UTC (rev 249775)
@@ -26,6 +26,7 @@
#pragma once
#include <WebCore/FrameLoaderClient.h>
+#include <pal/SessionID.h>
namespace PAL {
class SessionID;
@@ -56,8 +57,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;
+ uint64_t frameID;
+ WebCore::PageIdentifier pageID;
+ };
+ void setHasFrameSpecificStorageAccess(FrameSpecificStorageAccessIdentifier&&);
#endif
private:
@@ -282,7 +289,7 @@
bool m_frameCameFromPageCache;
bool m_useIconLoadingClient { false };
#if ENABLE(RESOURCE_LOAD_STATISTICS)
- bool m_hasFrameSpecificStorageAccess { false };
+ Optional<FrameSpecificStorageAccessIdentifier> m_frameSpecificStorageAccessIdentifier;
#endif
};
Modified: branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebPage/WebFrame.h (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebPage/WebFrame.h 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebPage/WebFrame.h 2019-09-11 20:12:07 UTC (rev 249775)
@@ -171,6 +171,8 @@
void setFirstLayerTreeTransactionIDAfterDidCommitLoad(uint64_t 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: branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp (249774 => 249775)
--- branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-09-11 20:12:03 UTC (rev 249774)
+++ branches/safari-608.2.11.1-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-09-11 20:12:07 UTC (rev 249775)
@@ -6545,7 +6545,17 @@
void WebPage::requestStorageAccess(RegistrableDomain&& subFrameDomain, RegistrableDomain&& topFrameDomain, uint64_t frameID, CompletionHandler<void(WebCore::StorageAccessWasGranted, WebCore::StorageAccessPromptWasShown)>&& completionHandler)
{
- WebProcess::singleton().ensureNetworkProcessConnection().connection().sendWithAsyncReply(Messages::NetworkConnectionToWebProcess::RequestStorageAccess(sessionID(), WTFMove(subFrameDomain), WTFMove(topFrameDomain), frameID, m_pageID), WTFMove(completionHandler));
+ auto* frame = frameID ? WebProcess::singleton().webFrame(frameID) : nullptr;
+ if (!frame)
+ return completionHandler(WebCore::StorageAccessWasGranted::No, WebCore::StorageAccessPromptWasShown::No);
+
+ WebProcess::singleton().ensureNetworkProcessConnection().connection().sendWithAsyncReply(Messages::NetworkConnectionToWebProcess::RequestStorageAccess(sessionID(), WTFMove(subFrameDomain), WTFMove(topFrameDomain), frameID, m_pageID), [completionHandler = WTFMove(completionHandler), frame = makeRef(*frame), sessionID = sessionID(), pageID = m_pageID](StorageAccessWasGranted wasGranted, WebCore::StorageAccessPromptWasShown promptWasShown) mutable {
+ if (wasGranted == StorageAccessWasGranted::Yes) {
+ if (auto* frameLoaderClient = frame->frameLoaderClient())
+ frameLoaderClient->setHasFrameSpecificStorageAccess({ sessionID, frame->frameID(), pageID });
+ }
+ completionHandler(wasGranted, promptWasShown);
+ });
}
void WebPage::wasLoadedWithDataTransferFromPrevalentResource()
