Changeset 247860 in webkit

Timestamp:

Jul 26, 2019 12:29:55 AM (5 years ago)

Author:

commit-queue@webkit.org

Message:

Make storing cross-origin top-level prefetches in HTTP cache optional
​https://bugs.webkit.org/show_bug.cgi?id=199499

Patch by Rob Buis <​rbuis@igalia.com> on 2019-07-26
Reviewed by Youenn Fablet.

Source/WebCore:

Add Internals API for adding an event listener to prefetch load.

Test: http/wpt/prefetch/link-prefetch-skip-disk-cache.html

• bindings/js/JSEventListener.h:
• dom/EventTarget.h:

(WebCore::EventTarget::addEventListener):

• dom/Node.h:
• html/HTMLLinkElement.h:
• testing/Internals.cpp:

(WebCore::Internals::addPrefetchLoadEventListener):

• testing/Internals.h:
• testing/Internals.idl:

Source/WebKit:

Currently when we navigate we check if there is a cross-origin top-level
prefetch we can use for the navigation. The current solution uses the
HTTP cache as a way to store the prefetch and to immediately use it for
the navigation load. However this solution fails in case the prefetch is
not cacheable.

Instead of this, simulate a HTTP Cache entry to process the load and
afterwards try to store the navigation load into the HTTP cache, where
normal store decisions will be applied.

Test: http/wpt/prefetch/link-prefetch-skip-disk-cache.html

• NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::retrieveCacheEntry):
(WebKit::NetworkResourceLoader::retrieveCacheEntryInternal):

• NetworkProcess/NetworkResourceLoader.h:

LayoutTests:

Remove unneeded Cache-Control headers and verify that Cache-Control
effects whether the navigation gets stored in the disk cache or not.

• http/tests/cache/resources/prefetched-main-resource-iframe.php:
• http/tests/cache/resources/prefetched-main-resource.php:
• http/wpt/prefetch/link-prefetch-skip-disk-cache-expected.txt: Added.
• http/wpt/prefetch/link-prefetch-skip-disk-cache.html: Added.
• http/wpt/prefetch/resources/main-resource-skip-disk-cache.py: Added.

(main):

• http/wpt/prefetch/resources/navigate-skip-disk-cache.html: Added.
• platform/mac-wk1/TestExpectations:
• platform/win/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/cache/resources/prefetched-main-resource-iframe.php (modified) (2 diffs)
• LayoutTests/http/tests/cache/resources/prefetched-main-resource.php (modified) (1 diff)
• LayoutTests/http/wpt/prefetch/link-prefetch-skip-disk-cache-expected.txt (added)
• LayoutTests/http/wpt/prefetch/link-prefetch-skip-disk-cache.html (added)
• LayoutTests/http/wpt/prefetch/resources/main-resource-skip-disk-cache.py (added)
• LayoutTests/http/wpt/prefetch/resources/navigate-skip-disk-cache.html (added)
• LayoutTests/platform/mac-wk1/TestExpectations (modified) (1 diff)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/bindings/js/JSEventListener.h (modified) (1 diff)
• Source/WebCore/dom/EventTarget.h (modified) (1 diff)
• Source/WebCore/dom/Node.h (modified) (1 diff)
• Source/WebCore/html/HTMLLinkElement.h (modified) (1 diff)
• Source/WebCore/testing/Internals.cpp (modified) (2 diffs)
• Source/WebCore/testing/Internals.h (modified) (2 diffs)
• Source/WebCore/testing/Internals.idl (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (modified) (2 diffs)
• Source/WebKit/NetworkProcess/NetworkResourceLoader.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-07-26  Rob Buis  <rbuis@igalia.com>
+
+        Make storing cross-origin top-level prefetches in HTTP cache optional
+        https://bugs.webkit.org/show_bug.cgi?id=199499
+
+        Reviewed by Youenn Fablet.
+
+        Remove unneeded Cache-Control headers and verify that Cache-Control
+        effects whether the navigation gets stored in the disk cache or not.
+
+        * http/tests/cache/resources/prefetched-main-resource-iframe.php:
+        * http/tests/cache/resources/prefetched-main-resource.php:
+        * http/wpt/prefetch/link-prefetch-skip-disk-cache-expected.txt: Added.
+        * http/wpt/prefetch/link-prefetch-skip-disk-cache.html: Added.
+        * http/wpt/prefetch/resources/main-resource-skip-disk-cache.py: Added.
+        (main):
+        * http/wpt/prefetch/resources/navigate-skip-disk-cache.html: Added.
+        * platform/mac-wk1/TestExpectations:
+        * platform/win/TestExpectations:
+
 2019-07-25  Commit Queue  <commit-queue@webkit.org>
 

trunk/LayoutTests/http/tests/cache/resources/prefetched-main-resource-iframe.php

@@ -1 +1 @@
 <?php
 if ($_SERVER["HTTP_PURPOSE"] == "prefetch") {
-    header('Cache-Control: max-age=3600');
-    header("Access-Control-Allow-Origin: http://127.0.0.1:8000");
-
     echo "<script>";
     echo "parent.window.postMessage('FAIL', '*');";
@@ -11 +8 @@
 }
 
-header("Access-Control-Allow-Origin: http://127.0.0.1:8000");
-
 echo "<script>";
 echo "parent.window.postMessage('PASS', '*');";

trunk/LayoutTests/http/tests/cache/resources/prefetched-main-resource.php

@@ -7 +7 @@
 <script>
 
-if (window.testRunner)
-   testRunner.notifyDone();
+if (window.testRunner) {
+  fetch('http://localhost:8000/cache/resources/prefetched-main-resource.php').then(function(response) {
+    if (internals.fetchResponseSource(response) != "Disk cache") {
+      document.getElementById('log').innerText = 'FAIL: resource is not in the disk cache.';
+    }
+    testRunner.notifyDone();
+  });
+}
 
 </script>
-<?php
-if ($_SERVER["HTTP_PURPOSE"] == "prefetch") {
-    print('PASS');
-} else {
-    print('FAIL');
-}
-?>
+<div id="log"><?php if ($_SERVER["HTTP_PURPOSE"] == "prefetch") echo 'PASS'; else echo 'FAIL'; ?></div>
 </body>
 </html>

trunk/LayoutTests/platform/mac-wk1/TestExpectations

@@ -737 +737 @@
 
 webkit.org/b/197371 http/wpt/prefetch/link-prefetch-main-resource-redirect.html [ Skip ]
+webkit.org/b/199499 http/wpt/prefetch/link-prefetch-skip-disk-cache.html [ Skip ]
 
 webkit.org/b/199632 http/wpt/prefetch/beforeload.html [ Skip ]

trunk/LayoutTests/platform/win/TestExpectations

@@ -4425 +4425 @@
 
 webkit.org/b/197371 http/wpt/prefetch/link-prefetch-main-resource-redirect.html [ Skip ]
+webkit.org/b/199499 http/wpt/prefetch/link-prefetch-skip-disk-cache.html [ Skip ]
 
 webkit.org/b/199632 http/wpt/prefetch/beforeload.html [ Skip ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-07-26  Rob Buis  <rbuis@igalia.com>
+
+        Make storing cross-origin top-level prefetches in HTTP cache optional
+        https://bugs.webkit.org/show_bug.cgi?id=199499
+
+        Reviewed by Youenn Fablet.
+
+        Add Internals API for adding an event listener to prefetch load.
+
+        Test: http/wpt/prefetch/link-prefetch-skip-disk-cache.html
+
+        * bindings/js/JSEventListener.h:
+        * dom/EventTarget.h:
+        (WebCore::EventTarget::addEventListener):
+        * dom/Node.h:
+        * html/HTMLLinkElement.h:
+        * testing/Internals.cpp:
+        (WebCore::Internals::addPrefetchLoadEventListener):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2019-07-25  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/bindings/js/JSEventListener.h

@@ -39 +39 @@
 class JSEventListener : public EventListener {
 public:
-    static Ref<JSEventListener> create(JSC::JSObject* listener, JSC::JSObject* wrapper, bool isAttribute, DOMWrapperWorld&);
-    static RefPtr<JSEventListener> create(JSC::JSValue listener, JSC::JSObject& wrapper, bool isAttribute, DOMWrapperWorld&);
+    WEBCORE_EXPORT static Ref<JSEventListener> create(JSC::JSObject* listener, JSC::JSObject* wrapper, bool isAttribute, DOMWrapperWorld&);
+    WEBCORE_EXPORT static RefPtr<JSEventListener> create(JSC::JSValue listener, JSC::JSObject& wrapper, bool isAttribute, DOMWrapperWorld&);
 
     virtual ~JSEventListener();

trunk/Source/WebCore/dom/EventTarget.h

@@ -89 +89 @@
     WEBCORE_EXPORT ExceptionOr<bool> dispatchEventForBindings(Event&);
 
-    virtual bool addEventListener(const AtomString& eventType, Ref<EventListener>&&, const AddEventListenerOptions& = { });
+    WEBCORE_EXPORT virtual bool addEventListener(const AtomString& eventType, Ref<EventListener>&&, const AddEventListenerOptions& = { });
     virtual bool removeEventListener(const AtomString& eventType, EventListener&, const ListenerOptions&);
 

trunk/Source/WebCore/dom/Node.h

@@ -473 +473 @@
     ScriptExecutionContext* scriptExecutionContext() const final; // Implemented in Document.h
 
-    bool addEventListener(const AtomString& eventType, Ref<EventListener>&&, const AddEventListenerOptions&) override;
+    WEBCORE_EXPORT bool addEventListener(const AtomString& eventType, Ref<EventListener>&&, const AddEventListenerOptions&) override;
     bool removeEventListener(const AtomString& eventType, EventListener&, const ListenerOptions&) override;
 

trunk/Source/WebCore/html/HTMLLinkElement.h

@@ -49 +49 @@
 
     URL href() const;
-    const AtomString& rel() const;
+    WEBCORE_EXPORT const AtomString& rel() const;
 
     String target() const final;

trunk/Source/WebCore/testing/Internals.cpp

@@ -69 +69 @@
 #include "Element.h"
 #include "EventHandler.h"
+#include "EventListener.h"
+#include "EventNames.h"
 #include "ExtendableEvent.h"
 #include "ExtensionStyleSheets.h"
@@ -5145 +5147 @@
 }
 
+void Internals::addPrefetchLoadEventListener(HTMLLinkElement& link, RefPtr<EventListener>&& listener)
+{
+    if (RuntimeEnabledFeatures::sharedFeatures().linkPrefetchEnabled() && equalLettersIgnoringASCIICase(link.rel(), "prefetch"))
+        link.addEventListener(eventNames().loadEvent, listener.releaseNonNull(), false);
+}
+
 } // namespace WebCore

trunk/Source/WebCore/testing/Internals.h

@@ -60 +60 @@
 class Document;
 class Element;
+class EventListener;
 class ExtendableEvent;
 class FetchResponse;
@@ -867 +868 @@
     TextIndicatorInfo textIndicatorForRange(const Range&, TextIndicatorOptions);
 
+    void addPrefetchLoadEventListener(HTMLLinkElement&, RefPtr<EventListener>&&);
+
 private:
     explicit Internals(Document&);

trunk/Source/WebCore/testing/Internals.idl

@@ -782 +782 @@
 
     TextIndicatorInfo textIndicatorForRange(Range range, TextIndicatorOptions options);
-};
+
+    void addPrefetchLoadEventListener(HTMLLinkElement link, EventListener? callback);
+};

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-07-26  Rob Buis  <rbuis@igalia.com>
+
+        Make storing cross-origin top-level prefetches in HTTP cache optional
+        https://bugs.webkit.org/show_bug.cgi?id=199499
+
+        Reviewed by Youenn Fablet.
+
+        Currently when we navigate we check if there is a cross-origin top-level
+        prefetch we can use for the navigation. The current solution uses the
+        HTTP cache as a way to store the prefetch and to immediately use it for
+        the navigation load. However this solution fails in case the prefetch is
+        not cacheable.
+
+        Instead of this, simulate a HTTP Cache entry to process the load and
+        afterwards try to store the navigation load into the HTTP cache, where
+        normal store decisions will be applied.
+
+        Test: http/wpt/prefetch/link-prefetch-skip-disk-cache.html
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::retrieveCacheEntry):
+        (WebKit::NetworkResourceLoader::retrieveCacheEntryInternal):
+        * NetworkProcess/NetworkResourceLoader.h:
+
 2019-07-25  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

@@ -219 +219 @@
         if (auto* session = m_connection->networkProcess().networkSession(sessionID())) {
             if (auto entry = session->prefetchCache().take(request.url())) {
+                // FIXME: Deal with credentials (https://bugs.webkit.org/show_bug.cgi?id=200000)
                 if (!entry->redirectRequest.isNull()) {
+                    auto cacheEntry = m_cache->makeRedirectEntry(request, entry->response, entry->redirectRequest);
+                    loader->retrieveCacheEntryInternal(WTFMove(cacheEntry), ResourceRequest { request });
                     auto maxAgeCap = validateCacheEntryForMaxAgeCapValidation(request, entry->redirectRequest, entry->response);
                     m_cache->storeRedirect(request, entry->response, entry->redirectRequest, maxAgeCap);
-                } else
-                    m_cache->store(request, entry->response, entry->releaseBuffer(), nullptr);
+                    return;
+                }
+                auto buffer = entry->releaseBuffer();
+                auto cacheEntry = m_cache->makeEntry(request, entry->response, buffer.copyRef());
+                loader->retrieveCacheEntryInternal(WTFMove(cacheEntry), ResourceRequest { request });
+                m_cache->store(request, entry->response, WTFMove(buffer), nullptr);
+                return;
             }
         }
@@ -240 +248 @@
             return;
         }
+        loader->retrieveCacheEntryInternal(WTFMove(entry), WTFMove(request));
+    });
+}
+
+void NetworkResourceLoader::retrieveCacheEntryInternal(std::unique_ptr<NetworkCache::Entry>&& entry, WebCore::ResourceRequest&& request)
+{
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
-        if (entry->hasReachedPrevalentResourceAgeCap()) {
-            RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Resource has reached prevalent resource age cap (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
-            m_cacheEntryForMaxAgeCapValidation = WTFMove(entry);
-            ResourceRequest revalidationRequest = originalRequest();
-            loader->startNetworkLoad(WTFMove(revalidationRequest), FirstLoad::Yes);
-            return;
-        }
+    if (entry->hasReachedPrevalentResourceAgeCap()) {
+        RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Resource has reached prevalent resource age cap (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
+        m_cacheEntryForMaxAgeCapValidation = WTFMove(entry);
+        ResourceRequest revalidationRequest = originalRequest();
+        startNetworkLoad(WTFMove(revalidationRequest), FirstLoad::Yes);
+        return;
+    }
 #endif
-        if (entry->redirectRequest()) {
-            RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Handling redirect (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
-            loader->dispatchWillSendRequestForCacheEntry(WTFMove(request), WTFMove(entry));
-            return;
-        }
-        if (loader->m_parameters.needsCertificateInfo && !entry->response().certificateInfo()) {
-            RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Resource does not have required certificate (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
-            loader->startNetworkLoad(WTFMove(request), FirstLoad::Yes);
-            return;
-        }
-        if (entry->needsValidation() || request.cachePolicy() == WebCore::ResourceRequestCachePolicy::RefreshAnyCacheData) {
-            RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Validating cache entry (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
-            loader->validateCacheEntry(WTFMove(entry));
-            return;
-        }
-        RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Retrieved resource from cache (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
-        loader->didRetrieveCacheEntry(WTFMove(entry));
-    });
+    if (entry->redirectRequest()) {
+        RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Handling redirect (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
+        dispatchWillSendRequestForCacheEntry(WTFMove(request), WTFMove(entry));
+        return;
+    }
+    if (m_parameters.needsCertificateInfo && !entry->response().certificateInfo()) {
+        RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Resource does not have required certificate (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
+        startNetworkLoad(WTFMove(request), FirstLoad::Yes);
+        return;
+    }
+    if (entry->needsValidation() || request.cachePolicy() == WebCore::ResourceRequestCachePolicy::RefreshAnyCacheData) {
+        RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Validating cache entry (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
+        validateCacheEntry(WTFMove(entry));
+        return;
+    }
+    RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Retrieved resource from cache (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID.toUInt64(), m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
+    didRetrieveCacheEntry(WTFMove(entry));
 }
 

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h

@@ -130 +130 @@
     void tryStoreAsCacheEntry();
     void retrieveCacheEntry(const WebCore::ResourceRequest&);
+    void retrieveCacheEntryInternal(std::unique_ptr<NetworkCache::Entry>&&, WebCore::ResourceRequest&&);
     void didRetrieveCacheEntry(std::unique_ptr<NetworkCache::Entry>);
     void sendResultForCacheEntry(std::unique_ptr<NetworkCache::Entry>);