Changeset 251887 in webkit

Timestamp:

Oct 31, 2019 3:40:16 PM (5 years ago)

Author:

Russell Epstein

Message:

Unreviewed, rolling out r251861.

Caused 50+ Layout Test Crashes

Reverted changeset:

"Use SecurityOriginData in NetworkProcess where possible
without other changes"
​https://bugs.webkit.org/show_bug.cgi?id=203615
​https://trac.webkit.org/changeset/251861

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/SecurityOrigin.cpp (modified) (2 diffs)
• WebCore/page/SecurityOrigin.h (modified) (1 diff)
• WebCore/page/SecurityOriginData.h (modified) (1 diff)
• WebCore/page/csp/ContentSecurityPolicy.cpp (modified) (1 diff)
• WebCore/page/csp/ContentSecurityPolicy.h (modified) (1 diff)
• WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp (modified) (4 diffs)
• WebCore/page/csp/ContentSecurityPolicyDirectiveList.h (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/NetworkProcess/NetworkResourceLoadParameters.cpp (modified) (1 diff)
• WebKit/NetworkProcess/NetworkResourceLoadParameters.h (modified) (1 diff)
• WebKit/NetworkProcess/NetworkResourceLoader.cpp (modified) (1 diff)
• WebKit/WebProcess/Network/WebLoaderStrategy.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-10-31  Russell Epstein  <repstein@apple.com>
+
+        Unreviewed, rolling out r251861.
+
+        Caused 50+ Layout Test Crashes
+
+        Reverted changeset:
+
+        "Use SecurityOriginData in NetworkProcess where possible
+        without other changes"
+        https://bugs.webkit.org/show_bug.cgi?id=203615
+        https://trac.webkit.org/changeset/251861
+
 2019-10-31  Said Abou-Hallawa  <sabouhallawa@apple.com>
 

trunk/Source/WebCore/page/SecurityOrigin.cpp

@@ -172 +172 @@
 }
 
-SecurityOrigin::SecurityOrigin(const SecurityOrigin& other)
-    : m_data { other.m_data.isolatedCopy() }
-    , m_domain { other.m_domain.isolatedCopy() }
-    , m_filePath { other.m_filePath.isolatedCopy() }
-    , m_isUnique { other.m_isUnique }
-    , m_universalAccess { other.m_universalAccess }
-    , m_domainWasSetInDOM { other.m_domainWasSetInDOM }
-    , m_canLoadLocalResources { other.m_canLoadLocalResources }
-    , m_storageBlockingPolicy { other.m_storageBlockingPolicy }
-    , m_enforcesFilePathSeparation { other.m_enforcesFilePathSeparation }
-    , m_needsStorageAccessFromFileURLsQuirk { other.m_needsStorageAccessFromFileURLsQuirk }
-    , m_isPotentiallyTrustworthy { other.m_isPotentiallyTrustworthy }
-    , m_isLocal { other.m_isLocal }
+SecurityOrigin::SecurityOrigin(const SecurityOrigin* other)
+    : m_data { other->m_data.isolatedCopy() }
+    , m_domain { other->m_domain.isolatedCopy() }
+    , m_filePath { other->m_filePath.isolatedCopy() }
+    , m_isUnique { other->m_isUnique }
+    , m_universalAccess { other->m_universalAccess }
+    , m_domainWasSetInDOM { other->m_domainWasSetInDOM }
+    , m_canLoadLocalResources { other->m_canLoadLocalResources }
+    , m_storageBlockingPolicy { other->m_storageBlockingPolicy }
+    , m_enforcesFilePathSeparation { other->m_enforcesFilePathSeparation }
+    , m_needsStorageAccessFromFileURLsQuirk { other->m_needsStorageAccessFromFileURLsQuirk }
+    , m_isPotentiallyTrustworthy { other->m_isPotentiallyTrustworthy }
+    , m_isLocal { other->m_isLocal }
 {
 }
@@ -219 +219 @@
 Ref<SecurityOrigin> SecurityOrigin::isolatedCopy() const
 {
-    return adoptRef(*new SecurityOrigin(*this));
+    return adoptRef(*new SecurityOrigin(this));
 }
 

trunk/Source/WebCore/page/SecurityOrigin.h

@@ -222 +222 @@
     SecurityOrigin();
     explicit SecurityOrigin(const URL&);
-    explicit SecurityOrigin(const SecurityOrigin&);
+    explicit SecurityOrigin(const SecurityOrigin*);
 
     // FIXME: Rename this function to something more semantic.

trunk/Source/WebCore/page/SecurityOriginData.h

@@ -98 +98 @@
 void SecurityOriginData::encode(Encoder& encoder) const
 {
-    ASSERT(!isEmpty());
     encoder << protocol;
     encoder << host;

trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp

@@ -509 +509 @@
 }
 
-bool ContentSecurityPolicy::allowFrameAncestors(const Vector<SecurityOriginData>& ancestorOrigins, const URL& url, bool overrideContentSecurityPolicy) const
+bool ContentSecurityPolicy::allowFrameAncestors(const Vector<RefPtr<SecurityOrigin>>& ancestorOrigins, const URL& url, bool overrideContentSecurityPolicy) const
 {
     if (overrideContentSecurityPolicy)

trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h

@@ -100 +100 @@
 
     bool allowFrameAncestors(const Frame&, const URL&, bool overrideContentSecurityPolicy = false) const;
-    WEBCORE_EXPORT bool allowFrameAncestors(const Vector<SecurityOriginData>& ancestorOrigins, const URL&, bool overrideContentSecurityPolicy = false) const;
+    WEBCORE_EXPORT bool allowFrameAncestors(const Vector<RefPtr<SecurityOrigin>>& ancestorOrigins, const URL&, bool overrideContentSecurityPolicy = false) const;
     WEBCORE_EXPORT bool overridesXFrameOptions() const;
 

trunk/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp

@@ -80 +80 @@
 }
 
-static inline URL urlFromOrigin(const SecurityOriginData& origin)
-{
-    return { URL { }, origin.toString() };
-}
-
 static inline bool checkFrameAncestors(ContentSecurityPolicySourceListDirective* directive, const Frame& frame)
 {
@@ -98 +93 @@
 }
 
-static inline bool checkFrameAncestors(ContentSecurityPolicySourceListDirective* directive, const Vector<SecurityOriginData>& ancestorOrigins)
+static inline bool checkFrameAncestors(ContentSecurityPolicySourceListDirective* directive, const Vector<RefPtr<SecurityOrigin>>& ancestorOrigins)
 {
     if (!directive)
@@ -104 +99 @@
     bool didReceiveRedirectResponse = false;
     for (auto& origin : ancestorOrigins) {
-        URL originURL = urlFromOrigin(origin);
+        URL originURL = urlFromOrigin(*origin);
         if (!originURL.isValid() || !directive->allows(originURL, didReceiveRedirectResponse, ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::No))
             return false;
@@ -264 +259 @@
 }
 
-const ContentSecurityPolicyDirective* ContentSecurityPolicyDirectiveList::violatedDirectiveForFrameAncestorOrigins(const Vector<SecurityOriginData>& ancestorOrigins) const
+const ContentSecurityPolicyDirective* ContentSecurityPolicyDirectiveList::violatedDirectiveForFrameAncestorOrigins(const Vector<RefPtr<SecurityOrigin>>& ancestorOrigins) const
 {
     if (checkFrameAncestors(m_frameAncestors.get(), ancestorOrigins))

trunk/Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.h

@@ -63 +63 @@
     const ContentSecurityPolicyDirective* violatedDirectiveForFrame(const URL&, bool didReceiveRedirectResponse) const;
     const ContentSecurityPolicyDirective* violatedDirectiveForFrameAncestor(const Frame&) const;
-    const ContentSecurityPolicyDirective* violatedDirectiveForFrameAncestorOrigins(const Vector<SecurityOriginData>&) const;
+    const ContentSecurityPolicyDirective* violatedDirectiveForFrameAncestorOrigins(const Vector<RefPtr<SecurityOrigin>>&) const;
     const ContentSecurityPolicyDirective* violatedDirectiveForImage(const URL&, bool didReceiveRedirectResponse) const;
 #if ENABLE(APPLICATION_MANIFEST)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-10-31  Russell Epstein  <repstein@apple.com>
+
+        Unreviewed, rolling out r251861.
+
+        Caused 50+ Layout Test Crashes
+
+        Reverted changeset:
+
+        "Use SecurityOriginData in NetworkProcess where possible
+        without other changes"
+        https://bugs.webkit.org/show_bug.cgi?id=203615
+        https://trac.webkit.org/changeset/251861
+
 2019-10-31  Truitt Savell  <tsavell@apple.com>
 

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.cpp

@@ -93 +93 @@
         encoder << *sourceOrigin;
     encoder << static_cast<bool>(topOrigin);
-    if (topOrigin)
+    if (sourceOrigin)
         encoder << *topOrigin;
     encoder << options;

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.h

@@ -59 +59 @@
     WebCore::PreflightPolicy preflightPolicy { WebCore::PreflightPolicy::Consider };
     bool shouldEnableCrossOriginResourcePolicy { false };
-    Vector<WebCore::SecurityOriginData> frameAncestorOrigins;
+    Vector<RefPtr<WebCore::SecurityOrigin>> frameAncestorOrigins;
     bool isHTTPSUpgradeEnabled { false };
 

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

@@ -411 +411 @@
     case XFrameOptionsSameOrigin: {
         auto origin = SecurityOrigin::create(url);
+        auto topFrameOrigin = m_parameters.frameAncestorOrigins.last();
+        if (!origin->isSameSchemeHostPort(*topFrameOrigin))
+            return true;
         for (auto& ancestorOrigin : m_parameters.frameAncestorOrigins) {
-            if (!origin->isSameSchemeHostPort(ancestorOrigin.securityOrigin()))
+            if (!origin->isSameSchemeHostPort(*ancestorOrigin))
                 return true;
         }

trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp

@@ -337 +337 @@
 
     if (resourceLoader.options().mode == FetchOptions::Mode::Navigate) {
-        Vector<SecurityOriginData> frameAncestorOrigins;
+        Vector<RefPtr<SecurityOrigin>> frameAncestorOrigins;
         for (auto* frame = resourceLoader.frame()->tree().parent(); frame; frame = frame->tree().parent())
-            frameAncestorOrigins.append(frame->document()->securityOrigin().data());
+            frameAncestorOrigins.append(makeRefPtr(frame->document()->securityOrigin()));
         loadParameters.frameAncestorOrigins = WTFMove(frameAncestorOrigins);
     }