Diff
Modified: trunk/LayoutTests/ChangeLog (247923 => 247924)
--- trunk/LayoutTests/ChangeLog 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/LayoutTests/ChangeLog 2019-07-29 21:57:39 UTC (rev 247924)
@@ -1,3 +1,14 @@
+2019-07-29 Sergei Glazunov <[email protected]>
+
+ The maximum subframe count check should not be skipped for empty URLs.
+ https://bugs.webkit.org/show_bug.cgi?id=200032
+
+ Reviewed by Ryosuke Niwa.
+
+ * fast/dom/connected-subframe-counter-overflow-expected.txt: Added.
+ * fast/dom/connected-subframe-counter-overflow.html: Added.
+ * fast/frames/lots-of-iframes-expected.txt:
+
2019-07-29 Youenn Fablet <[email protected]>
REGRESSION: WebSockets no longer work in Service Workers
Added: trunk/LayoutTests/fast/dom/connected-subframe-counter-overflow-expected.txt (0 => 247924)
--- trunk/LayoutTests/fast/dom/connected-subframe-counter-overflow-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/connected-subframe-counter-overflow-expected.txt 2019-07-29 21:57:39 UTC (rev 247924)
@@ -0,0 +1,10 @@
+The connected subframe counter should not overflow and make `disconnectSubframes` leave active subframes in a detached DOM node.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.firstChild.contentWindow is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/connected-subframe-counter-overflow.html (0 => 247924)
--- trunk/LayoutTests/fast/dom/connected-subframe-counter-overflow.html (rev 0)
+++ trunk/LayoutTests/fast/dom/connected-subframe-counter-overflow.html 2019-07-29 21:57:39 UTC (rev 247924)
@@ -0,0 +1,23 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description('The connected subframe counter should not overflow and make `disconnectSubframes` leave active subframes in a detached DOM node.');
+
+const FRAME_COUNT = 1024;
+
+let container = document.body.appendChild(document.createElement('div'));
+for (let i = 0; i < FRAME_COUNT; ++i) {
+ let frame = container.appendChild(document.createElement('iframe'));
+ frame.style.display = 'none';
+}
+container.remove();
+
+shouldBeNull('container.firstChild.contentWindow');
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/fast/frames/lots-of-iframes-expected.txt (247923 => 247924)
--- trunk/LayoutTests/fast/frames/lots-of-iframes-expected.txt 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/LayoutTests/fast/frames/lots-of-iframes-expected.txt 2019-07-29 21:57:39 UTC (rev 247924)
@@ -1,2 +1,3 @@
Sucessfully created 1000 frames.
Successfully blocked creation of frame number 1001.
+
Modified: trunk/Source/WebCore/ChangeLog (247923 => 247924)
--- trunk/Source/WebCore/ChangeLog 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/Source/WebCore/ChangeLog 2019-07-29 21:57:39 UTC (rev 247924)
@@ -1,3 +1,27 @@
+2019-07-29 Sergei Glazunov <[email protected]>
+
+ The maximum subframe count check should not be skipped for empty URLs.
+ https://bugs.webkit.org/show_bug.cgi?id=200032
+
+ Reviewed by Ryosuke Niwa.
+
+ Move the check closer to the actual frame creation code in `loadSubframe`.
+
+ Test: fast/dom/connected-subframe-counter-overflow.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::prepareForDestruction): Assert that all child frames have been detached.
+ * html/HTMLFrameElementBase.cpp:
+ (WebCore::HTMLFrameElementBase::canLoad const):
+ (WebCore::HTMLFrameElementBase::canLoadURL const):
+ * html/HTMLFrameOwnerElement.cpp:
+ (WebCore::HTMLFrameOwnerElement::canAddSubframe const): Deleted.
+ * html/HTMLFrameOwnerElement.h:
+ * html/HTMLPlugInImageElement.cpp:
+ (WebCore::HTMLPlugInImageElement::canLoadURL const):
+ * loader/SubframeLoader.cpp:
+ (WebCore::SubframeLoader::loadSubframe):
+
2019-07-29 Zalan Bujtas <[email protected]>
[ContentChangeObserver] ChromeClient::observedContentChange() name is misleading
Modified: trunk/Source/WebCore/dom/Document.cpp (247923 => 247924)
--- trunk/Source/WebCore/dom/Document.cpp 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/Source/WebCore/dom/Document.cpp 2019-07-29 21:57:39 UTC (rev 247924)
@@ -2487,6 +2487,7 @@
NavigationDisabler navigationDisabler(m_frame);
disconnectDescendantFrames();
}
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!m_frame || !m_frame->tree().childCount());
if (m_domWindow && m_frame)
m_domWindow->willDetachDocumentFromFrame();
Modified: trunk/Source/WebCore/html/HTMLFrameElementBase.cpp (247923 => 247924)
--- trunk/Source/WebCore/html/HTMLFrameElementBase.cpp 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/Source/WebCore/html/HTMLFrameElementBase.cpp 2019-07-29 21:57:39 UTC (rev 247924)
@@ -61,7 +61,6 @@
{
// FIXME: Why is it valuable to return true when m_URL is empty?
// FIXME: After openURL replaces an empty URL with the blank URL, this may no longer necessarily return true.
- // FIXME: It does not seem correct to skip the maximum subframe count check when m_URL is empty.
return m_URL.isEmpty() || canLoadURL(m_URL);
}
@@ -73,10 +72,6 @@
// Note that unlike HTMLPlugInImageElement::canLoadURL this uses ScriptController::canAccessFromCurrentOrigin.
bool HTMLFrameElementBase::canLoadURL(const URL& completeURL) const
{
- // FIXME: This assumes we are adding a new subframe; incorrectly prevents modifying an existing one once we are at the limit.
- if (!canAddSubframe())
- return false;
-
if (WTF::protocolIsJavaScript(completeURL)) {
RefPtr<Document> contentDocument = this->contentDocument();
if (contentDocument && !ScriptController::canAccessFromCurrentOrigin(contentDocument->frame(), document()))
Modified: trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp (247923 => 247924)
--- trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp 2019-07-29 21:57:39 UTC (rev 247924)
@@ -128,13 +128,6 @@
invalidateStyleAndLayerComposition();
}
-bool HTMLFrameOwnerElement::canAddSubframe() const
-{
- // FIXME: Might be safer to return false when page is null, but need to test in case we rely on returning true.
- auto* page = document().page();
- return !page || page->subframeCount() < Page::maxNumberOfFrames;
-}
-
bool HTMLFrameOwnerElement::isProhibitedSelfReference(const URL& completeURL) const
{
// We allow one level of self-reference because some websites depend on that, but we don't allow more than one.
Modified: trunk/Source/WebCore/html/HTMLFrameOwnerElement.h (247923 => 247924)
--- trunk/Source/WebCore/html/HTMLFrameOwnerElement.h 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/Source/WebCore/html/HTMLFrameOwnerElement.h 2019-07-29 21:57:39 UTC (rev 247924)
@@ -65,7 +65,6 @@
protected:
HTMLFrameOwnerElement(const QualifiedName& tagName, Document&);
void setSandboxFlags(SandboxFlags);
- bool canAddSubframe() const;
bool isProhibitedSelfReference(const URL&) const;
private:
Modified: trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp (247923 => 247924)
--- trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp 2019-07-29 21:57:39 UTC (rev 247924)
@@ -159,13 +159,9 @@
return canLoadURL(document().completeURL(relativeURL));
}
-// Note that unlike HTMLFrameElementBase::canLoadURL this uses ScriptController::canAccessFromCurrentOrigin.
+// Note that unlike HTMLFrameElementBase::canLoadURL this uses SecurityOrigin::canAccess.
bool HTMLPlugInImageElement::canLoadURL(const URL& completeURL) const
{
- // FIXME: This assumes we are adding a new subframe; incorrectly prevents modifying an existing one once we are at the limit.
- if (!canAddSubframe())
- return false;
-
if (WTF::protocolIsJavaScript(completeURL)) {
RefPtr<Document> contentDocument = this->contentDocument();
if (contentDocument && !document().securityOrigin().canAccess(contentDocument->securityOrigin()))
Modified: trunk/Source/WebCore/loader/SubframeLoader.cpp (247923 => 247924)
--- trunk/Source/WebCore/loader/SubframeLoader.cpp 2019-07-29 21:43:42 UTC (rev 247923)
+++ trunk/Source/WebCore/loader/SubframeLoader.cpp 2019-07-29 21:57:39 UTC (rev 247924)
@@ -329,6 +329,9 @@
if (!SubframeLoadingDisabler::canLoadFrame(ownerElement))
return nullptr;
+ if (!m_frame.page() || m_frame.page()->subframeCount() >= Page::maxNumberOfFrames)
+ return nullptr;
+
ReferrerPolicy policy = ownerElement.referrerPolicy();
if (policy == ReferrerPolicy::EmptyString)
policy = document->referrerPolicy();
