Attachment 373203 Details for Bug 120266 – Patch

[patch] Patch

bug-120266-20190630200750.patch (text/plain), 52.33 KB, created by Darin Adler on 2019-06-30 20:07:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Darin Adler

Created: 2019-06-30 20:07:51 PDT

Size: 52.33 KB

patch

obsolete

>Subversion Revision: 246960
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 88158cd1950f437d4f8ac3ca2ee9bb683256f5ca..b38a11398817c7bd82b98f9977b1c4c14aea75ca 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,141 @@
>+2019-06-30  Darin Adler  <darin@apple.com>
>+
>+        No need for isURLAllowed function in Frame
>+        https://bugs.webkit.org/show_bug.cgi?id=120266
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * css/SelectorCheckerTestFunctions.h:
>+        (WebCore::matchesFullScreenPseudoClass): Use is<> instead of
>+        the isFrameElementBase function. Also updated includes and spacing.
>+
>+        * dom/Document.cpp:
>+        (WebCore::Document::resolveStyle): Shortened an old, long comment
>+        that happened to mention HTMLFrameElementBase::isURLAllowed by name.
>+
>+        * dom/Element.h: Deleted isFrameElementBase.
>+
>+        * html/HTMLAppletElement.cpp:
>+        (WebCore::HTMLAppletElement::updateWidget): Tweaked comment.
>+
>+        * html/HTMLBodyElement.cpp:
>+        (WebCore::HTMLBodyElement::insertedIntoAncestor): Moved comment from
>+        here to didFinishInsertingNode, since the code it's talking about is there.
>+        Also got rid of unnecessary local variable.
>+        (WebCore::HTMLBodyElement::didFinishInsertingNode): Moved comment here
>+        and elaborated a bit. Also changed the code to copy margin attributes in a
>+        simpler way without requiring special getter functions. There is a slight
>+        behavior change here, but seems unlikely to affect any websites or tests.
>+        Would be good to add better test coverage, which I think should be done
>+        when fixing the problems described in the FIXME.
>+
>+        * html/HTMLEmbedElement.cpp:
>+        (WebCore::HTMLEmbedElement::updateWidget): allowedToLoadFrameURL -> canLoadURL
>+        Also Tweaked comments.
>+
>+        * html/HTMLFrameElement.cpp:
>+        (WebCore::HTMLFrameElement::rendererIsNeeded): isURLAllowed -> canLoad
>+
>+        * html/HTMLFrameElementBase.cpp:
>+        (WebCore::HTMLFrameElementBase::HTMLFrameElementBase): Removed initial
>+        values for no-longer-needed data members.
>+
>+        (WebCore::HTMLFrameElementBase::canLoadScriptURL const): Renamed from
>+        isURLAllowed. Also refactored so it now calls canLoadURL.
>+        (WebCore::HTMLFrameElementBase::canLoad const): Renamed from isURLAllowed.
>+        Also added FIXME comments about multiple ways that this code incorrectly
>+        handles the empty string. Might inspire some tests and future fixes.
>+        (WebCore::HTMLFrameElementBase::canLoadURL const): Renamed from isURLAllowed.
>+        Added an overload that takes a relative URL string largely to make this
>+        more parallel with the similar code in HTMLPlugInImageElement. Refactored
>+        to call canAddSubframe and isProhibitedSelfReference, removing the need
>+        to call Frame::isURLAllowed.
>+        (WebCore::HTMLFrameElementBase::openURL): isURLAllowed -> canLoad
>+        (WebCore::HTMLFrameElementBase::parseAttribute): Removed unneeded code
>+        to parse marginwidth, marginheight, and scrolling, which are now done
>+        as needed instead. Might have to add code back if we ever add code to
>+        make these cause dynamic updates when changed, but that was not the
>+        behavior of the existing code.
>+        (WebCore::HTMLFrameElementBase::scrollingMode const): Changed to call
>+        attributeWithoutSynchronization rather than reading from a data member.
>+
>+        * html/HTMLFrameElementBase.h: Moved scrollingMode into the .cpp file.
>+        Removed marginWidth and marginHeight since they are no longer called.
>+        Made canContainRangeEndPoint override be private. Renamed isURLAllowed
>+        to canLoad/canLoadScriptURL/canLoadURL. Removed unneeded isFrameElementBase
>+        since is<HTMLFrameElementBase> already works without it. Removed unneeded
>+        data members m_scrolling, m_marginWidth, and m_marginHeight.
>+
>+        * html/HTMLFrameOwnerElement.cpp:
>+        (WebCore::HTMLFrameOwnerElement::HTMLFrameOwnerElement): Moved initialization
>+        of m_contentFrame and m_sandboxFlags to the header.
>+        (WebCore::HTMLFrameOwnerElement::clearContentFrame): 0 -> nullptr
>+        (WebCore::HTMLFrameOwnerElement::canAddSubframe const): Added. Code was moved
>+        here from multiple functions that enforced the subframe count limit.
>+        (WebCore::HTMLFrameOwnerElement::isProhibitedSelfReference const): Added.
>+        Moved code here from Frame::isURLAllowed so it's easy to use as needed.
>+
>+        * html/HTMLFrameOwnerElement.h: Renamed isURLAllowed to canLoadScriptURL.
>+        Added protected canAddSubframe and isProhibitedSelfReference functions.
>+        Initialized the data members here.
>+
>+        * html/HTMLIFrameElement.cpp:
>+        (WebCore::HTMLIFrameElement::sandbox): Added braces for multi-line if statement.
>+        (WebCore::HTMLIFrameElement::parseAttribute): Removed unneeded storage of the
>+        allow attribute value. Added missing code to invalidate m_featurePolicy when
>+        the allow attribute changes. This could change behavior, but unlikely to affect
>+        any websites of tests. Would be good to add a test for this.
>+        (WebCore::HTMLIFrameElement::rendererIsNeeded): isURLAllowed -> canLoad. Also
>+        reordered the code to be more efficient for display: none (why not?).
>+        (WebCore::HTMLIFrameElement::featurePolicy const): Use attributeWithoutSynchronization
>+        to read the allow attribute rather than using a data member.
>+
>+        * html/HTMLIFrameElement.h: Removed the unneeded allow function member and
>+        m_allow data member since DOM reflection takes care of the function and the
>+        featurePolicy function can get the attribute without a data member.
>+
>+        * html/HTMLObjectElement.cpp:
>+        (WebCore::HTMLObjectElement::updateWidget): allowedToLoadFrameURL -> canLoadURL
>+        Also tweaked comments.
>+
>+        * html/HTMLPlugInElement.cpp:
>+        (WebCore::HTMLPlugInElement::canLoadScriptURL const): Added. Code moved here
>+        from HTMLFrameOwnerElement::isURLAllowed. Added comment about why this is not
>+        necessarily correct and might need improvement.
>+
>+        * html/HTMLPlugInElement.h: Added private override of canLoadScriptURL.
>+
>+        * html/HTMLPlugInImageElement.cpp:
>+        (WebCore::HTMLPlugInImageElement::canLoadURL const): Renamed from
>+        allowedToLoadFrameURL and refactored into two functions to match the
>+        structure of the similar function in HTMLFrameElementBase. Also refactored
>+        to use canAddSubframe and isProhibitedSelfReference rather than Frame::isURLAllowed.
>+        (WebCore::HTMLPlugInImageElement::canLoadPlugInContent const): Renamed from
>+        allowedToLoadPluginContent.
>+        (WebCore::HTMLPlugInImageElement::requestObject): allowedToLoadPluginContent
>+        -> canLoadPlugInContent
>+
>+        * html/HTMLPlugInImageElement.h: Renamed functions as mentioned above.
>+
>+        * loader/SubframeLoader.cpp:
>+        (WebCore::SubframeLoader::requestFrame): isURLAllowed -> canLoadScriptURL
>+
>+        * page/FocusController.h: Removed unneeded forward declaration of HTMLFrameOwnerElement.
>+
>+        * page/Frame.cpp:
>+        (WebCore::Frame::isURLAllowed const): Deleted.
>+        * page/Frame.h: Ditto.
>+
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::init): Removed code to initialize m_margins, since nothing
>+        was looking at it after that.
>+        (WebCore::FrameView::setMarginWidth): Deleted.
>+        (WebCore::FrameView::setMarginHeight): Deleted.
>+
>+        * page/FrameView.h: Removed marginWidth, marginHeight, setMarginWidth,
>+        setMarginHeight, and m_margins because there was no code anywhere calling
>+        marginWidth or marginHeight, and no other side effects of these functions.
>+
> 2019-06-30  Andres Gonzalez  <andresg_22@apple.com>
> 
>         Enhance support of aria-haspopup per ARIA 1.1 specification.
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 83e36d72dd7463c9dbf0ce68d6f5f7e2c79bf5f2..7626750cecf15e4dbc7347d6d4ef65d800cfb0bf 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-06-30  Darin Adler  <darin@apple.com>
>+
>+        No need for isURLAllowed function in Frame
>+        https://bugs.webkit.org/show_bug.cgi?id=120266
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * WebProcess/Automation/WebAutomationSessionProxy.cpp:
>+        (WebKit::WebAutomationSessionProxy::resolveChildFrameWithNodeHandle):
>+        Use is<HTMLFrameElementBase> instead of isFrameElementBase.
>+
> 2019-06-30  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [Win] Multiline mode of tooltip control does word-wrapping very slowly
>diff --git a/Source/WebCore/css/SelectorCheckerTestFunctions.h b/Source/WebCore/css/SelectorCheckerTestFunctions.h
>index c3f784b57c9406db1d00cd8fb4adca6719b2606f..631b384ebcbb4bf927bf010ca7d8ed877ce7ffce 100644
>--- a/Source/WebCore/css/SelectorCheckerTestFunctions.h
>+++ b/Source/WebCore/css/SelectorCheckerTestFunctions.h
>@@ -28,6 +28,8 @@
> 
> #include "FocusController.h"
> #include "FullscreenManager.h"
>+#include "HTMLFrameElement.h"
>+#include "HTMLIFrameElement.h"
> #include "HTMLInputElement.h"
> #include "HTMLOptionElement.h"
> #include "RenderScrollbar.h"
>@@ -344,13 +346,14 @@ ALWAYS_INLINE bool scrollbarMatchesCornerPresentPseudoClass(const SelectorChecke
> }
> 
> #if ENABLE(FULLSCREEN_API)
>+
> ALWAYS_INLINE bool matchesFullScreenPseudoClass(const Element& element)
> {
>     // While a Document is in the fullscreen state, and the document's current fullscreen
>     // element is an element in the document, the 'full-screen' pseudoclass applies to
>     // that element. Also, an <iframe>, <object> or <embed> element whose child browsing
>     // context's Document is in the fullscreen state has the 'full-screen' pseudoclass applied.
>-    if (element.isFrameElementBase() && element.containsFullScreenElement())
>+    if (is<HTMLFrameElementBase>(element) && element.containsFullScreenElement())
>         return true;
>     if (!element.document().fullscreenManager().isFullscreen())
>         return false;
>@@ -384,9 +387,11 @@ ALWAYS_INLINE bool matchesFullScreenControlsHiddenPseudoClass(const Element& ele
>         return false;
>     return element.document().fullscreenManager().areFullscreenControlsHidden();
> }
>+
> #endif
> 
> #if ENABLE(VIDEO_TRACK)
>+
> ALWAYS_INLINE bool matchesFutureCuePseudoClass(const Element& element)
> {
>     return is<WebVTTElement>(element) && !downcast<WebVTTElement>(element).isPastNode();
>@@ -396,6 +401,7 @@ ALWAYS_INLINE bool matchesPastCuePseudoClass(const Element& element)
> {
>     return is<WebVTTElement>(element) && downcast<WebVTTElement>(element).isPastNode();
> }
>+
> #endif
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 78e59127508d98f97539858761b4dd3649e2aad2..934448fb8c6970e52c7357947fdea71c9799d5b7 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -1886,12 +1886,8 @@ void Document::resolveStyle(ResolveStyleType type)
>             element->updateShadowTree();
>     }
> 
>-    // FIXME: We should update style on our ancestor chain before proceeding (especially for seamless),
>-    // however doing so currently causes several tests to crash, as Frame::setDocument calls Document::attach
>-    // before setting the DOMWindow on the Frame, or the SecurityOrigin on the document. The attach, in turn
>-    // resolves style (here) and then when we resolve style on the parent chain, we may end up
>-    // re-attaching our containing iframe, which when asked HTMLFrameElementBase::isURLAllowed
>-    // hits a null-dereference due to security code always assuming the document has a SecurityOrigin.
>+    // FIXME: We should update style on our ancestor chain before proceeding, however doing so at
>+    // the time this comment was originally written caused several tests to crash.
> 
>     {
>         ScriptDisallowedScope::InMainThread scriptDisallowedScope;
>diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h
>index 4475d758a580e61a7ffb6e9596a81089a73b7cb6..d3c9240601f4074ee8d26688535af72a77cfbe54 100644
>--- a/Source/WebCore/dom/Element.h
>+++ b/Source/WebCore/dom/Element.h
>@@ -468,7 +468,6 @@ public:
>     virtual bool isRequiredFormControl() const { return false; }
>     virtual bool isInRange() const { return false; }
>     virtual bool isOutOfRange() const { return false; }
>-    virtual bool isFrameElementBase() const { return false; }
>     virtual bool isUploadButton() const { return false; }
>     virtual bool isSliderContainerElement() const { return false; }
> 
>diff --git a/Source/WebCore/html/HTMLAppletElement.cpp b/Source/WebCore/html/HTMLAppletElement.cpp
>index c8f5f5da86c8b23f08df008aba46940e2f8d5bbb..44511673cd7d6ea173b856e5d7c0f723fbb6fb26 100644
>--- a/Source/WebCore/html/HTMLAppletElement.cpp
>+++ b/Source/WebCore/html/HTMLAppletElement.cpp
>@@ -117,9 +117,8 @@ void HTMLAppletElement::updateWidget(CreatePlugins createPlugins)
> #if PLATFORM(IOS_FAMILY)
>     UNUSED_PARAM(createPlugins);
> #else
>-    // FIXME: It's sadness that we have this special case here.
>-    //        See http://trac.webkit.org/changeset/25128 and
>-    //        plugins/netscape-plugin-setwindow-size.html
>+    // FIXME: It's unfortunate that we have this special case here.
>+    // See http://trac.webkit.org/changeset/25128 and the plugins/netscape-plugin-setwindow-size.html test.
>     if (createPlugins == CreatePlugins::No)
>         return;
> 
>diff --git a/Source/WebCore/html/HTMLBodyElement.cpp b/Source/WebCore/html/HTMLBodyElement.cpp
>index 49a660deb5bcb5c5c8d1e422277a5b74ab8c4901..5f8a8c660f7581c69461e89fc2e4ebaf976f8af0 100644
>--- a/Source/WebCore/html/HTMLBodyElement.cpp
>+++ b/Source/WebCore/html/HTMLBodyElement.cpp
>@@ -3,7 +3,7 @@
>  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
>  *           (C) 2000 Simon Hausmann (hausmann@kde.org)
>  *           (C) 2001 Dirk Mueller (mueller@kde.org)
>- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -186,32 +186,27 @@ Node::InsertedIntoAncestorResult HTMLBodyElement::insertedIntoAncestor(Insertion
>     HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
>     if (!insertionType.connectedToDocument)
>         return InsertedIntoAncestorResult::Done;
>-
>-    // FIXME: It's surprising this is web compatible since it means a marginwidth and marginheight attribute can
>-    // magically appear on the <body> of all documents embedded through <iframe> or <frame>.
>-    // FIXME: Perhaps this code should be in attach() instead of here.
>-    auto ownerElement = makeRefPtr(document().ownerElement());
>-    if (!is<HTMLFrameElementBase>(ownerElement))
>+    if (!is<HTMLFrameElementBase>(document().ownerElement()))
>         return InsertedIntoAncestorResult::Done;
>-
>     return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
> }
> 
> void HTMLBodyElement::didFinishInsertingNode()
> {
>-    auto ownerElement = makeRefPtr(document().ownerElement());
>-    RELEASE_ASSERT(is<HTMLFrameElementBase>(ownerElement));
>-    auto& ownerFrameElement = downcast<HTMLFrameElementBase>(*ownerElement);
>-
>-    // Read values from the owner before setting any attributes, since setting an attribute can run arbitrary
>-    // JavaScript, which might delete the owner element.
>-    int marginWidth = ownerFrameElement.marginWidth();
>-    int marginHeight = ownerFrameElement.marginHeight();
>-
>-    if (marginWidth != -1)
>-        setIntegralAttribute(marginwidthAttr, marginWidth);
>-    if (marginHeight != -1)
>-        setIntegralAttribute(marginheightAttr, marginHeight);
>+    ASSERT(is<HTMLFrameElementBase>(document().ownerElement()));
>+    auto ownerElement = makeRef(*document().ownerElement());
>+
>+    // FIXME: It's surprising this is web compatible since it means marginwidth and marginheight attributes
>+    // appear or get overwritten on body elements of a document embedded through <iframe> or <frame>.
>+    // Better to find a way to do addHTMLLengthToStyle based on the attributes from the frame element,
>+    // without modifying the body element's attributes. Could also add code so we can respond to updates
>+    // to the frame element attributes.
>+    auto marginWidth = ownerElement->attributeWithoutSynchronization(marginwidthAttr);
>+    if (!marginWidth.isNull())
>+        setAttributeWithoutSynchronization(marginwidthAttr, marginWidth);
>+    auto marginHeight = ownerElement->attributeWithoutSynchronization(marginheightAttr);
>+    if (!marginHeight.isNull())
>+        setAttributeWithoutSynchronization(marginheightAttr, marginHeight);
> }
> 
> bool HTMLBodyElement::isURLAttribute(const Attribute& attribute) const
>diff --git a/Source/WebCore/html/HTMLEmbedElement.cpp b/Source/WebCore/html/HTMLEmbedElement.cpp
>index aac407be2a2426a08bd247e6e759a0f826f97852..205735406bf1f1e32c22598560175f6b965b7b03 100644
>--- a/Source/WebCore/html/HTMLEmbedElement.cpp
>+++ b/Source/WebCore/html/HTMLEmbedElement.cpp
>@@ -154,14 +154,13 @@ void HTMLEmbedElement::updateWidget(CreatePlugins createPlugins)
> 
>     // Note these pass m_url and m_serviceType to allow better code sharing with
>     // <object> which modifies url and serviceType before calling these.
>-    if (!allowedToLoadFrameURL(m_url)) {
>+    if (!canLoadURL(m_url)) {
>         setNeedsWidgetUpdate(false);
>         return;
>     }
> 
>-    // FIXME: It's sadness that we have this special case here.
>-    //        See http://trac.webkit.org/changeset/25128 and
>-    //        plugins/netscape-plugin-setwindow-size.html
>+    // FIXME: It's unfortunate that we have this special case here.
>+    // See http://trac.webkit.org/changeset/25128 and the plugins/netscape-plugin-setwindow-size.html test.
>     if (createPlugins == CreatePlugins::No && wouldLoadAsPlugIn(m_url, m_serviceType))
>         return;
> 
>@@ -186,8 +185,9 @@ void HTMLEmbedElement::updateWidget(CreatePlugins createPlugins)
>     if (!renderer()) // Do not load the plugin if beforeload removed this element or its renderer.
>         return;
> 
>-    // beforeLoad could have changed the document. Make sure the URL is still safe to load.
>-    if (!allowedToLoadFrameURL(m_url))
>+    // Dispatching a beforeLoad event could have executed code that changed the document.
>+    // Make sure the URL is still safe to load.
>+    if (!canLoadURL(m_url))
>         return;
> 
>     // FIXME: beforeLoad could have detached the renderer!  Just like in the <object> case above.
>diff --git a/Source/WebCore/html/HTMLFrameElement.cpp b/Source/WebCore/html/HTMLFrameElement.cpp
>index dcd92d0ac7eabf523f6327c438f91b958fec54a6..3ce6aa6dcb0776f5d5bb4ae38eab66bd0b072ba1 100644
>--- a/Source/WebCore/html/HTMLFrameElement.cpp
>+++ b/Source/WebCore/html/HTMLFrameElement.cpp
>@@ -51,7 +51,7 @@ Ref<HTMLFrameElement> HTMLFrameElement::create(const QualifiedName& tagName, Doc
> bool HTMLFrameElement::rendererIsNeeded(const RenderStyle&)
> {
>     // For compatibility, frames render even when display: none is set.
>-    return isURLAllowed();
>+    return canLoad();
> }
> 
> RenderPtr<RenderElement> HTMLFrameElement::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)
>diff --git a/Source/WebCore/html/HTMLFrameElementBase.cpp b/Source/WebCore/html/HTMLFrameElementBase.cpp
>index b9030110b74e441239fbfe99e53a356d699442b3..f9dd90a5111b8efc64930e53c4d090e68846ef5b 100644
>--- a/Source/WebCore/html/HTMLFrameElementBase.cpp
>+++ b/Source/WebCore/html/HTMLFrameElementBase.cpp
>@@ -3,7 +3,7 @@
>  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
>  *           (C) 2000 Simon Hausmann (hausmann@kde.org)
>  *           (C) 2001 Dirk Mueller (mueller@kde.org)
>- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -48,45 +48,47 @@ using namespace HTMLNames;
> 
> HTMLFrameElementBase::HTMLFrameElementBase(const QualifiedName& tagName, Document& document)
>     : HTMLFrameOwnerElement(tagName, document)
>-    , m_scrolling(ScrollbarAuto)
>-    , m_marginWidth(-1)
>-    , m_marginHeight(-1)
> {
>     setHasCustomStyleResolveCallbacks();
> }
> 
>-bool HTMLFrameElementBase::isURLAllowed() const
>+bool HTMLFrameElementBase::canLoadScriptURL(const URL& scriptURL) const
> {
>-    if (m_URL.isEmpty())
>-        return true;
>+    return canLoadURL(scriptURL);
>+}
> 
>-    return isURLAllowed(document().completeURL(m_URL));
>+bool HTMLFrameElementBase::canLoad() const
>+{
>+    // 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);
> }
> 
>-bool HTMLFrameElementBase::isURLAllowed(const URL& completeURL) const
>+bool HTMLFrameElementBase::canLoadURL(const String& relativeURL) const
> {
>-    if (document().page() && document().page()->subframeCount() >= Page::maxNumberOfFrames)
>-        return false;
>+    return canLoadURL(document().completeURL(relativeURL));
>+}
> 
>-    if (completeURL.isEmpty())
>-        return true;
>+// 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> contentDoc = this->contentDocument();
>-        if (contentDoc && !ScriptController::canAccessFromCurrentOrigin(contentDoc->frame(), document()))
>+        RefPtr<Document> contentDocument = this->contentDocument();
>+        if (contentDocument && !ScriptController::canAccessFromCurrentOrigin(contentDocument->frame(), document()))
>             return false;
>     }
> 
>-    RefPtr<Frame> parentFrame = document().frame();
>-    if (parentFrame)
>-        return parentFrame->isURLAllowed(completeURL);
>-
>-    return true;
>+    return !isProhibitedSelfReference(completeURL);
> }
> 
> void HTMLFrameElementBase::openURL(LockHistory lockHistory, LockBackForwardList lockBackForwardList)
> {
>-    if (!isURLAllowed())
>+    if (!canLoad())
>         return;
> 
>     if (m_URL.isEmpty())
>@@ -109,20 +111,7 @@ void HTMLFrameElementBase::parseAttribute(const QualifiedName& name, const AtomS
>         setLocation("about:srcdoc");
>     else if (name == srcAttr && !hasAttributeWithoutSynchronization(srcdocAttr))
>         setLocation(stripLeadingAndTrailingHTMLSpaces(value));
>-    else if (name == marginwidthAttr) {
>-        m_marginWidth = value.toInt();
>-        // FIXME: If we are already attached, this has no effect.
>-    } else if (name == marginheightAttr) {
>-        m_marginHeight = value.toInt();
>-        // FIXME: If we are already attached, this has no effect.
>-    } else if (name == scrollingAttr) {
>-        // Auto and yes both simply mean "allow scrolling." No means "don't allow scrolling."
>-        if (equalLettersIgnoringASCIICase(value, "auto") || equalLettersIgnoringASCIICase(value, "yes"))
>-            m_scrolling = ScrollbarAuto;
>-        else if (equalLettersIgnoringASCIICase(value, "no"))
>-            m_scrolling = ScrollbarAlwaysOff;
>-        // FIXME: If we are already attached, this has no effect.
>-    } else
>+    else
>         HTMLFrameOwnerElement::parseAttribute(name, value);
> }
> 
>@@ -229,4 +218,10 @@ int HTMLFrameElementBase::height()
>     return renderBox()->height();
> }
> 
>+ScrollbarMode HTMLFrameElementBase::scrollingMode() const
>+{
>+    return equalLettersIgnoringASCIICase(attributeWithoutSynchronization(scrollingAttr), "no")
>+        ? ScrollbarAlwaysOff : ScrollbarAuto;
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/html/HTMLFrameElementBase.h b/Source/WebCore/html/HTMLFrameElementBase.h
>index 5016f8f6ce597857b4b6b96020ea853a415abfbd..d1f4e202516c8a93c8e33f261b6ae7013f38436a 100644
>--- a/Source/WebCore/html/HTMLFrameElementBase.h
>+++ b/Source/WebCore/html/HTMLFrameElementBase.h
>@@ -2,7 +2,7 @@
>  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
>  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
>  *           (C) 2000 Simon Hausmann <hausmann@kde.org>
>- * Copyright (C) 2004, 2006, 2008, 2009 Apple Inc. All rights reserved.
>+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -39,22 +39,15 @@ public:
>     WEBCORE_EXPORT void setLocation(const String&);
>     void setLocation(JSC::ExecState&, const String&);
> 
>-    ScrollbarMode scrollingMode() const final { return m_scrolling; }
>-    
>-    int marginWidth() const { return m_marginWidth; }
>-    int marginHeight() const { return m_marginHeight; }
>+    ScrollbarMode scrollingMode() const final;
> 
>     WEBCORE_EXPORT int width();
>     WEBCORE_EXPORT int height();
> 
>-    bool canContainRangeEndPoint() const final { return false; }
>-
>-    bool isURLAllowed(const URL&) const override;
>-
> protected:
>     HTMLFrameElementBase(const QualifiedName&, Document&);
> 
>-    bool isURLAllowed() const;
>+    bool canLoad() const;
> 
>     void parseAttribute(const QualifiedName&, const AtomString&) override;
>     InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
>@@ -62,22 +55,22 @@ protected:
>     void didAttachRenderers() override;
> 
> private:
>+    bool canLoadScriptURL(const URL&) const final;
>+
>+    bool canLoadURL(const String& relativeURL) const;
>+    bool canLoadURL(const URL&) const;
>+
>+    bool canContainRangeEndPoint() const final { return false; }
>+
>     bool supportsFocus() const final;
>     void setFocus(bool) final;
>     
>     bool isURLAttribute(const Attribute&) const final;
>     bool isHTMLContentAttribute(const Attribute&) const final;
> 
>-    bool isFrameElementBase() const final { return true; }
>-
>     void openURL(LockHistory = LockHistory::Yes, LockBackForwardList = LockBackForwardList::Yes);
> 
>     AtomString m_URL;
>-
>-    ScrollbarMode m_scrolling;
>-
>-    int m_marginWidth;
>-    int m_marginHeight;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/html/HTMLFrameOwnerElement.cpp b/Source/WebCore/html/HTMLFrameOwnerElement.cpp
>index d6f3d10585ecadab1bf96c11979d179aab816dfc..cc53039c31b2aa0885f48eb2e9e63195eeb37254 100644
>--- a/Source/WebCore/html/HTMLFrameOwnerElement.cpp
>+++ b/Source/WebCore/html/HTMLFrameOwnerElement.cpp
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2006, 2007, 2009 Apple Inc. All rights reserved.
>+ * Copyright (C) 2006-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -25,6 +25,7 @@
> #include "Frame.h"
> #include "FrameLoader.h"
> #include "RenderWidget.h"
>+#include "ScriptController.h"
> #include "ShadowRoot.h"
> #include "SVGDocument.h"
> #include "StyleTreeResolver.h"
>@@ -37,8 +38,6 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(HTMLFrameOwnerElement);
> 
> HTMLFrameOwnerElement::HTMLFrameOwnerElement(const QualifiedName& tagName, Document& document)
>     : HTMLElement(tagName, document)
>-    , m_contentFrame(nullptr)
>-    , m_sandboxFlags(SandboxNone)
> {
> }
> 
>@@ -69,7 +68,7 @@ void HTMLFrameOwnerElement::clearContentFrame()
>     if (!m_contentFrame)
>         return;
> 
>-    m_contentFrame = 0;
>+    m_contentFrame = nullptr;
> 
>     for (RefPtr<ContainerNode> node = this; node; node = node->parentOrShadowHostNode())
>         node->decrementConnectedSubframeCount();
>@@ -77,8 +76,7 @@ void HTMLFrameOwnerElement::clearContentFrame()
> 
> void HTMLFrameOwnerElement::disconnectContentFrame()
> {
>-    if (RefPtr<Frame> frame = contentFrame()) {
>-        Ref<Frame> protect(*frame);
>+    if (RefPtr<Frame> frame = m_contentFrame) {
>         frame->loader().frameDetached();
>         frame->disconnectOwnerElement();
>     }
>@@ -130,6 +128,27 @@ void HTMLFrameOwnerElement::scheduleInvalidateStyleAndLayerComposition()
>         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.
>+    bool foundOneSelfReference = false;
>+    for (auto* frame = document().frame(); frame; frame = frame->tree().parent()) {
>+        if (equalIgnoringFragmentIdentifier(frame->document()->url(), completeURL)) {
>+            if (foundOneSelfReference)
>+                return true;
>+            foundOneSelfReference = true;
>+        }
>+    }
>+    return false;
>+}
>+
> bool SubframeLoadingDisabler::canLoadFrame(HTMLFrameOwnerElement& owner)
> {
>     for (RefPtr<ContainerNode> node = &owner; node; node = node->parentOrShadowHostNode()) {
>diff --git a/Source/WebCore/html/HTMLFrameOwnerElement.h b/Source/WebCore/html/HTMLFrameOwnerElement.h
>index 0e9bf36d29da15e79d80e68b239cebb6c7f2478d..5eecffa94ca4ef97ff58e8c86715e623e53049e6 100644
>--- a/Source/WebCore/html/HTMLFrameOwnerElement.h
>+++ b/Source/WebCore/html/HTMLFrameOwnerElement.h
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2006, 2007, 2009 Apple Inc. All rights reserved.
>+ * Copyright (C) 2006-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -27,7 +27,6 @@
> 
> namespace WebCore {
> 
>-class DOMWindow;
> class Frame;
> class RenderWidget;
> class SVGDocument;
>@@ -59,20 +58,22 @@ public:
> 
>     void scheduleInvalidateStyleAndLayerComposition();
> 
>-    virtual bool isURLAllowed(const URL&) const { return true; }
>+    virtual bool canLoadScriptURL(const URL&) const = 0;
> 
>     virtual ReferrerPolicy referrerPolicy() const { return ReferrerPolicy::EmptyString; }
> 
> protected:
>     HTMLFrameOwnerElement(const QualifiedName& tagName, Document&);
>     void setSandboxFlags(SandboxFlags);
>+    bool canAddSubframe() const;
>+    bool isProhibitedSelfReference(const URL&) const;
> 
> private:
>     bool isKeyboardFocusable(KeyboardEvent*) const override;
>     bool isFrameOwnerElement() const final { return true; }
> 
>-    Frame* m_contentFrame;
>-    SandboxFlags m_sandboxFlags;
>+    Frame* m_contentFrame { nullptr };
>+    SandboxFlags m_sandboxFlags { SandboxNone };
> };
> 
> class SubframeLoadingDisabler {
>diff --git a/Source/WebCore/html/HTMLIFrameElement.cpp b/Source/WebCore/html/HTMLIFrameElement.cpp
>index fa07d7185c07314764bd771c1b241de57af27514..5fe4a5d8a888acaeec7ae29f54a5ccc063f27932 100644
>--- a/Source/WebCore/html/HTMLIFrameElement.cpp
>+++ b/Source/WebCore/html/HTMLIFrameElement.cpp
>@@ -3,7 +3,7 @@
>  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
>  *           (C) 2000 Simon Hausmann (hausmann@kde.org)
>  *           (C) 2001 Dirk Mueller (mueller@kde.org)
>- * Copyright (C) 2004, 2006, 2008, 2009 Apple Inc. All rights reserved.
>+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
>  * Copyright (C) 2009 Ericsson AB. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>@@ -53,10 +53,11 @@ Ref<HTMLIFrameElement> HTMLIFrameElement::create(const QualifiedName& tagName, D
> 
> DOMTokenList& HTMLIFrameElement::sandbox()
> {
>-    if (!m_sandbox)
>+    if (!m_sandbox) {
>         m_sandbox = std::make_unique<DOMTokenList>(*this, sandboxAttr, [](Document&, StringView token) {
>             return SecurityContext::isSupportedSandboxPolicy(token);
>         });
>+    }
>     return *m_sandbox;
> }
> 
>@@ -97,14 +98,14 @@ void HTMLIFrameElement::parseAttribute(const QualifiedName& name, const AtomStri
>         if (!invalidTokens.isNull())
>             document().addConsoleMessage(MessageSource::Other, MessageLevel::Error, "Error while parsing the 'sandbox' attribute: " + invalidTokens);
>     } else if (name == allowAttr)
>-        m_allow = value;
>+        m_featurePolicy = WTF::nullopt;
>     else
>         HTMLFrameElementBase::parseAttribute(name, value);
> }
> 
> bool HTMLIFrameElement::rendererIsNeeded(const RenderStyle& style)
> {
>-    return isURLAllowed() && style.display() != DisplayType::None;
>+    return style.display() != DisplayType::None && canLoad();
> }
> 
> RenderPtr<RenderElement> HTMLIFrameElement::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)
>@@ -153,7 +154,7 @@ ReferrerPolicy HTMLIFrameElement::referrerPolicy() const
> const FeaturePolicy& HTMLIFrameElement::featurePolicy() const
> {
>     if (!m_featurePolicy)
>-        m_featurePolicy = FeaturePolicy::parse(document(), m_allow);
>+        m_featurePolicy = FeaturePolicy::parse(document(), attributeWithoutSynchronization(allowAttr));
>     return *m_featurePolicy;
> }
> 
>diff --git a/Source/WebCore/html/HTMLIFrameElement.h b/Source/WebCore/html/HTMLIFrameElement.h
>index 08d92c8362958c6ea91c7887683dcf3918c3e14e..9da1735b7c64667ee986df7b49aa586f93a2de08 100644
>--- a/Source/WebCore/html/HTMLIFrameElement.h
>+++ b/Source/WebCore/html/HTMLIFrameElement.h
>@@ -2,7 +2,7 @@
>  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
>  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
>  *           (C) 2000 Simon Hausmann <hausmann@kde.org>
>- * Copyright (C) 2004, 2006, 2008, 2009 Apple Inc. All rights reserved.
>+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -39,7 +39,6 @@ public:
>     DOMTokenList& sandbox();
> 
>     RenderIFrame* renderer() const;
>-    const String& allow() const { return m_allow; }
> 
>     void setReferrerPolicyForBindings(const AtomString&);
>     String referrerPolicyForBindings() const;
>@@ -62,7 +61,6 @@ private:
>     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
> 
>     std::unique_ptr<DOMTokenList> m_sandbox;
>-    String m_allow;
>     mutable Optional<FeaturePolicy> m_featurePolicy;
> };
> 
>diff --git a/Source/WebCore/html/HTMLObjectElement.cpp b/Source/WebCore/html/HTMLObjectElement.cpp
>index 31ea9c2ed6a173af01c3da0c2eff38be2f9d9818..a391ca8c2c5f7f577b61ddf94c9dd4902c28879b 100644
>--- a/Source/WebCore/html/HTMLObjectElement.cpp
>+++ b/Source/WebCore/html/HTMLObjectElement.cpp
>@@ -265,14 +265,13 @@ void HTMLObjectElement::updateWidget(CreatePlugins createPlugins)
>     parametersForPlugin(paramNames, paramValues, url, serviceType);
> 
>     // Note: url is modified above by parametersForPlugin.
>-    if (!allowedToLoadFrameURL(url)) {
>+    if (!canLoadURL(url)) {
>         setNeedsWidgetUpdate(false);
>         return;
>     }
> 
>-    // FIXME: It's sadness that we have this special case here.
>-    //        See http://trac.webkit.org/changeset/25128 and
>-    //        plugins/netscape-plugin-setwindow-size.html
>+    // FIXME: It's unfortunate that we have this special case here.
>+    // See http://trac.webkit.org/changeset/25128 and the plugins/netscape-plugin-setwindow-size.html test.
>     if (createPlugins == CreatePlugins::No && wouldLoadAsPlugIn(url, serviceType))
>         return;
> 
>@@ -283,7 +282,9 @@ void HTMLObjectElement::updateWidget(CreatePlugins createPlugins)
>     if (!renderer()) // Do not load the plugin if beforeload removed this element or its renderer.
>         return;
> 
>-    bool success = beforeLoadAllowedLoad && hasValidClassId() && allowedToLoadFrameURL(url);
>+    // Dispatching a beforeLoad event could have executed code that changed the document.
>+    // Make sure the URL is still safe to load.
>+    bool success = beforeLoadAllowedLoad && hasValidClassId() && canLoadURL(url);
>     if (success)
>         success = requestObject(url, serviceType, paramNames, paramValues);
>     if (!success && hasFallbackContent())
>diff --git a/Source/WebCore/html/HTMLPlugInElement.cpp b/Source/WebCore/html/HTMLPlugInElement.cpp
>index 6532462bfbe13584a9b6ab2df3f4b6e404f57339..2918213bb7101a5ddd902aa9cbfea57261eaed0e 100644
>--- a/Source/WebCore/html/HTMLPlugInElement.cpp
>+++ b/Source/WebCore/html/HTMLPlugInElement.cpp
>@@ -2,7 +2,7 @@
>  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
>  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
>  *           (C) 2000 Stefan Schimanski (1Stein@gmx.de)
>- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -379,14 +379,14 @@ static ReplacementPlugin* pluginReplacementForType(const URL& url, const String&
>     return nullptr;
> }
> 
>-bool HTMLPlugInElement::requestObject(const String& url, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues)
>+bool HTMLPlugInElement::requestObject(const String& relativeURL, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues)
> {
>     if (m_pluginReplacement)
>         return true;
> 
>     URL completedURL;
>-    if (!url.isEmpty())
>-        completedURL = document().completeURL(url);
>+    if (!relativeURL.isEmpty())
>+        completedURL = document().completeURL(relativeURL);
> 
>     ReplacementPlugin* replacement = pluginReplacementForType(completedURL, mimeType);
>     if (!replacement || !replacement->isEnabledBySettings(document().settings()))
>@@ -499,4 +499,10 @@ bool HTMLPlugInElement::isReplacementObscured()
>     return false;
> }
> 
>+bool HTMLPlugInElement::canLoadScriptURL(const URL&) const
>+{
>+    // FIXME: Probably want to at least check canAddSubframe.
>+    return true;
>+}
>+
> }
>diff --git a/Source/WebCore/html/HTMLPlugInElement.h b/Source/WebCore/html/HTMLPlugInElement.h
>index 57c5c21daa56a83c8709ac1f82e3d8501bbada39..2b8ee4358e6e2ac03dd2cd8470374a6ff8ea54c6 100644
>--- a/Source/WebCore/html/HTMLPlugInElement.h
>+++ b/Source/WebCore/html/HTMLPlugInElement.h
>@@ -1,7 +1,7 @@
> /*
>  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
>  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
>- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -123,6 +123,7 @@ private:
> 
>     bool isKeyboardFocusable(KeyboardEvent*) const override;
>     bool isPluginElement() const final;
>+    bool canLoadScriptURL(const URL&) const final;
> 
>     RefPtr<JSC::Bindings::Instance> m_instance;
>     Timer m_swapRendererTimer;
>diff --git a/Source/WebCore/html/HTMLPlugInImageElement.cpp b/Source/WebCore/html/HTMLPlugInImageElement.cpp
>index 7f6289acbc10193fce85d45bd2e3459e86c04a8a..67300fa3fd140818321a221ac1c310dfc623bab4 100644
>--- a/Source/WebCore/html/HTMLPlugInImageElement.cpp
>+++ b/Source/WebCore/html/HTMLPlugInImageElement.cpp
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2008-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -154,23 +154,35 @@ bool HTMLPlugInImageElement::isImageType()
>     return Image::supportsType(m_serviceType);
> }
> 
>-// We don't use m_url, as it may not be the final URL that the object loads, depending on <param> values.
>-bool HTMLPlugInImageElement::allowedToLoadFrameURL(const String& url)
>+bool HTMLPlugInImageElement::canLoadURL(const String& relativeURL) const
> {
>-    URL completeURL = document().completeURL(url);
>-    if (contentFrame() && WTF::protocolIsJavaScript(completeURL) && !document().securityOrigin().canAccess(contentDocument()->securityOrigin()))
>+    return canLoadURL(document().completeURL(relativeURL));
>+}
>+
>+// Note that unlike HTMLFrameElementBase::canLoadURL this uses ScriptController::canAccessFromCurrentOrigin.
>+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;
>-    return document().frame()->isURLAllowed(completeURL);
>+
>+    if (WTF::protocolIsJavaScript(completeURL)) {
>+        RefPtr<Document> contentDocument = this->contentDocument();
>+        if (contentDocument && !document().securityOrigin().canAccess(contentDocument->securityOrigin()))
>+            return false;
>+    }
>+
>+    return !isProhibitedSelfReference(completeURL);
> }
> 
> // We don't use m_url, or m_serviceType as they may not be the final values
> // that <object> uses depending on <param> values.
>-bool HTMLPlugInImageElement::wouldLoadAsPlugIn(const String& url, const String& serviceType)
>+bool HTMLPlugInImageElement::wouldLoadAsPlugIn(const String& relativeURL, const String& serviceType)
> {
>     ASSERT(document().frame());
>     URL completedURL;
>-    if (!url.isEmpty())
>-        completedURL = document().completeURL(url);
>+    if (!relativeURL.isEmpty())
>+        completedURL = document().completeURL(relativeURL);
>     return document().frame()->loader().client().objectContentType(completedURL, serviceType) == ObjectContentType::PlugIn;
> }
> 
>@@ -762,15 +774,15 @@ void HTMLPlugInImageElement::defaultEventHandler(Event& event)
>     HTMLPlugInElement::defaultEventHandler(event);
> }
> 
>-bool HTMLPlugInImageElement::allowedToLoadPluginContent(const String& url, const String& mimeType) const
>+bool HTMLPlugInImageElement::canLoadPlugInContent(const String& relativeURL, const String& mimeType) const
> {
>     // Elements in user agent show tree should load whatever the embedding document policy is.
>     if (isInUserAgentShadowTree())
>         return true;
> 
>     URL completedURL;
>-    if (!url.isEmpty())
>-        completedURL = document().completeURL(url);
>+    if (!relativeURL.isEmpty())
>+        completedURL = document().completeURL(relativeURL);
> 
>     ASSERT(document().contentSecurityPolicy());
>     const ContentSecurityPolicy& contentSecurityPolicy = *document().contentSecurityPolicy();
>@@ -785,22 +797,22 @@ bool HTMLPlugInImageElement::allowedToLoadPluginContent(const String& url, const
>     return contentSecurityPolicy.allowPluginType(mimeType, declaredMimeType, completedURL);
> }
> 
>-bool HTMLPlugInImageElement::requestObject(const String& url, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues)
>+bool HTMLPlugInImageElement::requestObject(const String& relativeURL, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues)
> {
>     ASSERT(document().frame());
> 
>-    if (url.isEmpty() && mimeType.isEmpty())
>+    if (relativeURL.isEmpty() && mimeType.isEmpty())
>         return false;
> 
>-    if (!allowedToLoadPluginContent(url, mimeType)) {
>+    if (!canLoadPlugInContent(relativeURL, mimeType)) {
>         renderEmbeddedObject()->setPluginUnavailabilityReason(RenderEmbeddedObject::PluginBlockedByContentSecurityPolicy);
>         return false;
>     }
> 
>-    if (HTMLPlugInElement::requestObject(url, mimeType, paramNames, paramValues))
>+    if (HTMLPlugInElement::requestObject(relativeURL, mimeType, paramNames, paramValues))
>         return true;
>     
>-    return document().frame()->loader().subframeLoader().requestObject(*this, url, getNameAttribute(), mimeType, paramNames, paramValues);
>+    return document().frame()->loader().subframeLoader().requestObject(*this, relativeURL, getNameAttribute(), mimeType, paramNames, paramValues);
> }
> 
> void HTMLPlugInImageElement::updateImageLoaderWithNewURLSoon()
>diff --git a/Source/WebCore/html/HTMLPlugInImageElement.h b/Source/WebCore/html/HTMLPlugInImageElement.h
>index ce0ea361cb4d50e6132dd6f8ace6f5b6cdf88f8f..644ead6573b2d3f08d95302b2d2d3a6a9984de3c 100644
>--- a/Source/WebCore/html/HTMLPlugInImageElement.h
>+++ b/Source/WebCore/html/HTMLPlugInImageElement.h
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2008-2019 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -85,8 +85,8 @@ protected:
>     HTMLImageLoader* imageLoader() { return m_imageLoader.get(); }
>     void updateImageLoaderWithNewURLSoon();
> 
>-    bool allowedToLoadFrameURL(const String& url);
>-    bool wouldLoadAsPlugIn(const String& url, const String& serviceType);
>+    bool canLoadURL(const String& relativeURL) const;
>+    bool wouldLoadAsPlugIn(const String& relativeURL, const String& serviceType);
> 
>     void scheduleUpdateForAfterStyleResolution();
> 
>@@ -97,7 +97,8 @@ private:
>     bool isPlugInImageElement() const final { return true; }
>     bool isRestartedPlugin() const final { return m_isRestartedPlugin; }
> 
>-    bool allowedToLoadPluginContent(const String& url, const String& mimeType) const;
>+    bool canLoadPlugInContent(const String& relativeURL, const String& mimeType) const;
>+    bool canLoadURL(const URL&) const;
> 
>     void didAddUserAgentShadowRoot(ShadowRoot&) final;
> 
>diff --git a/Source/WebCore/loader/SubframeLoader.cpp b/Source/WebCore/loader/SubframeLoader.cpp
>index 9622c4648367d90e46bab108ff54787f33943669..3f40e4056f4ef48e48179377bf42461476ec0464 100644
>--- a/Source/WebCore/loader/SubframeLoader.cpp
>+++ b/Source/WebCore/loader/SubframeLoader.cpp
>@@ -100,7 +100,7 @@ bool SubframeLoader::requestFrame(HTMLFrameOwnerElement& ownerElement, const Str
>     if (!frame)
>         return false;
> 
>-    if (!scriptURL.isEmpty() && ownerElement.isURLAllowed(scriptURL)) {
>+    if (!scriptURL.isEmpty() && ownerElement.canLoadScriptURL(scriptURL)) {
>         // FIXME: Some sites rely on the javascript:'' loading synchronously, which is why we have this special case.
>         // Blink has the same workaround (https://bugs.chromium.org/p/chromium/issues/detail?id=923585).
>         if (urlString == "javascript:''" || urlString == "javascript:\"\"")
>diff --git a/Source/WebCore/page/FocusController.h b/Source/WebCore/page/FocusController.h
>index b17bb6697578d81b4515c6098f3992a0fb29b6f2..6cd600f782e72f4177cbeb0d6fa446c1c7833c2d 100644
>--- a/Source/WebCore/page/FocusController.h
>+++ b/Source/WebCore/page/FocusController.h
>@@ -39,7 +39,6 @@ class Document;
> class Element;
> class FocusNavigationScope;
> class Frame;
>-class HTMLFrameOwnerElement;
> class IntRect;
> class KeyboardEvent;
> class Node;
>diff --git a/Source/WebCore/page/Frame.cpp b/Source/WebCore/page/Frame.cpp
>index 615e69180370f36bfee6fcbb39c11bd1bd1b4367..5c92b258d99ae70ce66e552eaf25e76f7663ea67 100644
>--- a/Source/WebCore/page/Frame.cpp
>+++ b/Source/WebCore/page/Frame.cpp
>@@ -963,23 +963,6 @@ void Frame::deviceOrPageScaleFactorChanged()
>         root->compositor().deviceOrPageScaleFactorChanged();
> }
> 
>-bool Frame::isURLAllowed(const URL& url) const
>-{
>-    // We allow one level of self-reference because some sites depend on that,
>-    // but we don't allow more than one.
>-    if (m_page->subframeCount() >= Page::maxNumberOfFrames)
>-        return false;
>-    bool foundSelfReference = false;
>-    for (const Frame* frame = this; frame; frame = frame->tree().parent()) {
>-        if (equalIgnoringFragmentIdentifier(frame->document()->url(), url)) {
>-            if (foundSelfReference)
>-                return false;
>-            foundSelfReference = true;
>-        }
>-    }
>-    return true;
>-}
>-
> bool Frame::isAlwaysOnLoggingAllowed() const
> {
>     return page() && page()->isAlwaysOnLoggingAllowed();
>diff --git a/Source/WebCore/page/Frame.h b/Source/WebCore/page/Frame.h
>index 42742dd6f8e7a8712344e289c7fede9725e2e875..30c5f84f97837d2cb7ee1ce71aadba401d2f1e53 100644
>--- a/Source/WebCore/page/Frame.h
>+++ b/Source/WebCore/page/Frame.h
>@@ -286,7 +286,6 @@ public:
>     void resumeActiveDOMObjectsAndAnimations();
>     bool activeDOMObjectsAndAnimationsSuspended() const { return m_activeDOMObjectsAndAnimationsSuspendedCount > 0; }
> 
>-    bool isURLAllowed(const URL&) const;
>     WEBCORE_EXPORT bool isAlwaysOnLoggingAllowed() const;
> 
> // ========
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 8b0de6bc6cc4733be9ed7e0c86f1da2c53771c48..97e90bddcd758b02e1e7532e6ddb04a3f9049438 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -316,28 +316,18 @@ void FrameView::init()
> {
>     reset();
> 
>-    m_margins = LayoutSize(-1, -1); // undefined
>     m_size = LayoutSize();
> 
>-    // Propagate the marginwidth/height and scrolling modes to the view.
>-    Element* ownerElement = frame().ownerElement();
>-    if (is<HTMLFrameElementBase>(ownerElement)) {
>-        HTMLFrameElementBase& frameElement = downcast<HTMLFrameElementBase>(*ownerElement);
>-        if (frameElement.scrollingMode() == ScrollbarAlwaysOff)
>-            setCanHaveScrollbars(false);
>-        LayoutUnit marginWidth = frameElement.marginWidth();
>-        LayoutUnit marginHeight = frameElement.marginHeight();
>-        if (marginWidth != -1)
>-            setMarginWidth(marginWidth);
>-        if (marginHeight != -1)
>-            setMarginHeight(marginHeight);
>-    }
>+    // Propagate the scrolling mode to the view.
>+    auto* ownerElement = frame().ownerElement();
>+    if (is<HTMLFrameElementBase>(ownerElement) && downcast<HTMLFrameElementBase>(*ownerElement).scrollingMode() == ScrollbarAlwaysOff)
>+        setCanHaveScrollbars(false);
> 
>     Page* page = frame().page();
>     if (page && page->chrome().client().shouldPaintEntireContents())
>         setPaintsEntireContents(true);
> }
>-    
>+
> void FrameView::prepareForDetach()
> {
>     detachCustomScrollbars();
>@@ -491,18 +481,6 @@ bool FrameView::scheduleAnimation()
>     return true;
> }
> 
>-void FrameView::setMarginWidth(LayoutUnit w)
>-{
>-    // make it update the rendering area when set
>-    m_margins.setWidth(w);
>-}
>-
>-void FrameView::setMarginHeight(LayoutUnit h)
>-{
>-    // make it update the rendering area when set
>-    m_margins.setHeight(h);
>-}
>-
> FrameFlattening FrameView::effectiveFrameFlattening() const
> {
> #if PLATFORM(IOS_FAMILY)
>diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
>index bf94012f2ff2a5a0b788ef31595038413dfd08da..4075cb4a41f5b880f15f81262b0b413a9ea97919 100644
>--- a/Source/WebCore/page/FrameView.h
>+++ b/Source/WebCore/page/FrameView.h
>@@ -4,7 +4,7 @@
>              (C) 1998, 1999 Torben Weis (weis@kde.org)
>              (C) 1999 Lars Knoll (knoll@kde.org)
>              (C) 1999 Antti Koivisto (koivisto@kde.org)
>-   Copyright (C) 2004-2017 Apple Inc. All rights reserved.
>+   Copyright (C) 2004-2019 Apple Inc. All rights reserved.
> 
>    This library is free software; you can redistribute it and/or
>    modify it under the terms of the GNU Library General Public
>@@ -95,11 +95,6 @@ public:
>     int mapFromLayoutToCSSUnits(LayoutUnit) const;
>     LayoutUnit mapFromCSSToLayoutUnits(int) const;
> 
>-    LayoutUnit marginWidth() const { return m_margins.width(); } // -1 means default
>-    LayoutUnit marginHeight() const { return m_margins.height(); } // -1 means default
>-    void setMarginWidth(LayoutUnit);
>-    void setMarginHeight(LayoutUnit);
>-
>     WEBCORE_EXPORT void setCanHaveScrollbars(bool) final;
>     WEBCORE_EXPORT void updateCanHaveScrollbars();
> 
>@@ -840,7 +835,6 @@ private:
>     MonotonicTime m_lastPaintTime;
> 
>     LayoutSize m_size;
>-    LayoutSize m_margins;
> 
>     Color m_baseBackgroundColor { Color::white };
>     IntSize m_lastViewportSize;
>diff --git a/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp b/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp
>index 68b3d89cbdc668e218fad704bf25a363dcd349ba..e6353ccdd54adfa5aa5905b8973034bc1ce956f2 100644
>--- a/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp
>+++ b/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp
>@@ -47,7 +47,8 @@
> #include <WebCore/Frame.h>
> #include <WebCore/FrameTree.h>
> #include <WebCore/FrameView.h>
>-#include <WebCore/HTMLFrameElementBase.h>
>+#include <WebCore/HTMLFrameElement.h>
>+#include <WebCore/HTMLIFrameElement.h>
> #include <WebCore/HTMLOptGroupElement.h>
> #include <WebCore/HTMLOptionElement.h>
> #include <WebCore/HTMLSelectElement.h>
>@@ -368,12 +369,12 @@ void WebAutomationSessionProxy::resolveChildFrameWithNodeHandle(WebCore::PageIde
>     }
> 
>     WebCore::Element* coreElement = elementForNodeHandle(*frame, nodeHandle);
>-    if (!coreElement || !coreElement->isFrameElementBase()) {
>+    if (!is<WebCore::HTMLFrameElementBase>(coreElement)) {
>         completionHandler(frameNotFoundErrorType, 0);
>         return;
>     }
> 
>-    WebCore::Frame* coreFrameFromElement = static_cast<WebCore::HTMLFrameElementBase*>(coreElement)->contentFrame();
>+    WebCore::Frame* coreFrameFromElement = downcast<WebCore::HTMLFrameElementBase>(*coreElement).contentFrame();
>     if (!coreFrameFromElement) {
>         completionHandler(frameNotFoundErrorType, 0);
>         return;

Actions: View | Formatted Diff | Diff

Attachments on bug 120266: 209578 | 209583 | 209586 | 209594 | 373203