Reduce use of SessionID::defaultSessionID() in WebKit
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 22 Sep 2019 06:04:48 +0000 (06:04 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 22 Sep 2019 06:04:48 +0000 (06:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202080

Reviewed by Alex Christensen.

Source/WebCore:

Reduce use of SessionID::defaultSessionID() in WebKit. Falling back to the default session
when you don't know which session to use is never a good idea and a potential privacy issue.

* Modules/websockets/WebSocketChannel.cpp:
(WebCore::WebSocketChannel::connect):
* dom/Document.cpp:
(WebCore::Document::logger):
* dom/Document.h:
* html/HTMLAnchorElement.cpp:
(WebCore::HTMLAnchorElement::parseAdClickAttribution const):
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::continueAfterContentPolicy):
* loader/EmptyFrameLoaderClient.h:
* loader/FrameLoaderClient.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestUserCSSStyleSheet):
(WebCore::CachedResourceLoader::updateCachedResourceWithCurrentRequest):
(WebCore::CachedResourceLoader::requestResource):
(WebCore::CachedResourceLoader::revalidateResource):
(WebCore::CachedResourceLoader::loadResource):
* loader/cache/CachedResourceLoader.h:
(WebCore::CachedResourceLoader::clearDocumentLoader):
* page/Frame.cpp:
* page/Frame.h:
* workers/service/ServiceWorkerContainer.cpp:
(WebCore::ServiceWorkerContainer::isAlwaysOnLoggingAllowed const):

Source/WebKit:

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::convertMainResourceLoadToDownload):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
* WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::convertMainResourceLoadToDownload):
* WebProcess/WebPage/WebFrame.h:

Source/WebKitLegacy/mac:

* WebCoreSupport/WebFrameLoaderClient.h:
* WebCoreSupport/WebFrameLoaderClient.mm:
(WebFrameLoaderClient::convertMainResourceLoadToDownload):

Source/WebKitLegacy/win:

* WebCoreSupport/WebFrameLoaderClient.cpp:
(WebFrameLoaderClient::convertMainResourceLoadToDownload):
* WebCoreSupport/WebFrameLoaderClient.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250196 268f45cc-cd09-0410-ab3c-d52691b4dbfc

24 files changed:
Source/WebCore/ChangeLog
Source/WebCore/Modules/websockets/WebSocketChannel.cpp
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/html/HTMLAnchorElement.cpp
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/EmptyFrameLoaderClient.h
Source/WebCore/loader/FrameLoaderClient.h
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/loader/cache/CachedResourceLoader.h
Source/WebCore/page/Frame.cpp
Source/WebCore/page/Frame.h
Source/WebCore/workers/service/ServiceWorkerContainer.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h
Source/WebKit/WebProcess/WebPage/WebFrame.cpp
Source/WebKit/WebProcess/WebPage/WebFrame.h
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h
Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm
Source/WebKitLegacy/win/ChangeLog
Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.h

index 190318d..1b38a59 100644 (file)
@@ -1,5 +1,39 @@
 2019-09-21  Chris Dumez  <cdumez@apple.com>
 
+        Reduce use of SessionID::defaultSessionID() in WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=202080
+
+        Reviewed by Alex Christensen.
+
+        Reduce use of SessionID::defaultSessionID() in WebKit. Falling back to the default session
+        when you don't know which session to use is never a good idea and a potential privacy issue.
+
+        * Modules/websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::connect):
+        * dom/Document.cpp:
+        (WebCore::Document::logger):
+        * dom/Document.h:
+        * html/HTMLAnchorElement.cpp:
+        (WebCore::HTMLAnchorElement::parseAdClickAttribution const):
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::continueAfterContentPolicy):
+        * loader/EmptyFrameLoaderClient.h:
+        * loader/FrameLoaderClient.h:
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::requestUserCSSStyleSheet):
+        (WebCore::CachedResourceLoader::updateCachedResourceWithCurrentRequest):
+        (WebCore::CachedResourceLoader::requestResource):
+        (WebCore::CachedResourceLoader::revalidateResource):
+        (WebCore::CachedResourceLoader::loadResource):
+        * loader/cache/CachedResourceLoader.h:
+        (WebCore::CachedResourceLoader::clearDocumentLoader):
+        * page/Frame.cpp:
+        * page/Frame.h:
+        * workers/service/ServiceWorkerContainer.cpp:
+        (WebCore::ServiceWorkerContainer::isAlwaysOnLoggingAllowed const):
+
+2019-09-21  Chris Dumez  <cdumez@apple.com>
+
         ServiceWorkerContextData does not need a sessionID
         https://bugs.webkit.org/show_bug.cgi?id=202087
 
index a07a876..db74a07 100644 (file)
@@ -104,13 +104,14 @@ WebSocketChannel::ConnectStatus WebSocketChannel::connect(const URL& requestedUR
     if (m_identifier)
         InspectorInstrumentation::didCreateWebSocket(m_document.get(), m_identifier, validatedURL->url);
 
-    if (Frame* frame = m_document->frame()) {
-        ref();
-        Page* page = frame->page();
-        PAL::SessionID sessionID = page ? page->sessionID() : PAL::SessionID::defaultSessionID();
-        String partition = m_document->domainForCachePartition();
-        m_handle = m_socketProvider->createSocketStreamHandle(m_handshake->url(), *this, sessionID, partition, frame->loader().networkingContext());
-    }
+    auto* frame = m_document->frame();
+    auto* page = m_document->page();
+    if (!frame || !page)
+        return ConnectStatus::KO;
+
+    ref();
+    String partition = m_document->domainForCachePartition();
+    m_handle = m_socketProvider->createSocketStreamHandle(m_handshake->url(), *this, page->sessionID(), partition, frame->loader().networkingContext());
     return ConnectStatus::OK;
 }
 
index 8ed2f88..232afe3 100644 (file)
@@ -5065,12 +5065,6 @@ URL Document::completeURL(const String& url) const
     return completeURL(url, m_baseURL);
 }
 
-Optional<PAL::SessionID> Document::sessionID() const
-{
-    auto* page = this->page();
-    return page ? makeOptional(page->sessionID()) : WTF::nullopt;
-}
-
 void Document::setPageCacheState(PageCacheState state)
 {
     if (m_pageCacheState == state)
@@ -5266,6 +5260,7 @@ void Document::storageBlockingStateDidChange()
     securityOrigin().setStorageBlockingPolicy(settings().storageBlockingPolicy());
 }
 
+// Used only by WebKitLegacy.
 void Document::privateBrowsingStateDidChange(PAL::SessionID sessionID)
 {
     if (m_logger)
@@ -7789,7 +7784,8 @@ Logger& Document::logger()
 {
     if (!m_logger) {
         m_logger = Logger::create(this);
-        m_logger->setEnabled(this, sessionID() && sessionID()->isAlwaysOnLoggingAllowed());
+        auto* page = this->page();
+        m_logger->setEnabled(this, page && page->sessionID().isAlwaysOnLoggingAllowed());
         m_logger->addObserver(*this);
     }
 
index 8c46d7e..70538c5 100644 (file)
@@ -683,7 +683,6 @@ public:
 
     WEBCORE_EXPORT URL completeURL(const String&) const final;
     URL completeURL(const String&, const URL& baseURLOverride) const;
-    WEBCORE_EXPORT Optional<PAL::SessionID> sessionID() const;
 
     String userAgent(const URL&) const final;
 
index a7a922a..597accb 100644 (file)
@@ -407,7 +407,8 @@ Optional<AdClickAttribution> HTMLAnchorElement::parseAdClickAttribution() const
     using Source = AdClickAttribution::Source;
     using Destination = AdClickAttribution::Destination;
 
-    if (!document().sessionID() || document().sessionID()->isEphemeral()
+    auto* page = document().page();
+    if (!page || page->sessionID().isEphemeral()
         || !RuntimeEnabledFeatures::sharedFeatures().adClickAttributionEnabled()
         || !UserGestureIndicator::processingUserGesture())
         return WTF::nullopt;
index 8293536..f3a063c 100644 (file)
@@ -967,15 +967,11 @@ void DocumentLoader::continueAfterContentPolicy(PolicyAction policy)
         // Download may use this knowledge for purposes unrelated to cookies, notably for setting file quarantine data.
         frameLoader()->setOriginalURLForDownloadRequest(m_request);
 
-        PAL::SessionID sessionID = PAL::SessionID::defaultSessionID();
-        if (frame() && frame()->page())
-            sessionID = frame()->page()->sessionID();
-
         if (m_request.url().protocolIsData()) {
             // We decode data URL internally, there is no resource load to convert.
             frameLoader()->client().startDownload(m_request);
         } else
-            frameLoader()->client().convertMainResourceLoadToDownload(this, sessionID, m_request, m_response);
+            frameLoader()->client().convertMainResourceLoadToDownload(this, m_request, m_response);
 
         // The main resource might be loading from the memory cache, or its loader might have gone missing.
         if (mainResourceLoader()) {
index 2f6d225..ec1a6a7 100644 (file)
@@ -56,7 +56,7 @@ class WEBCORE_EXPORT EmptyFrameLoaderClient : public FrameLoaderClient {
     void detachedFromParent2() final { }
     void detachedFromParent3() final { }
 
-    void convertMainResourceLoadToDownload(DocumentLoader*, PAL::SessionID, const ResourceRequest&, const ResourceResponse&) final { }
+    void convertMainResourceLoadToDownload(DocumentLoader*, const ResourceRequest&, const ResourceResponse&) final { }
 
     void assignIdentifierToInitialRequest(unsigned long, DocumentLoader*, const ResourceRequest&) final { }
     bool shouldUseCredentialStorage(DocumentLoader*, unsigned long) override { return false; }
index 7f68019..5dff610 100644 (file)
@@ -288,7 +288,7 @@ public:
     virtual void dispatchDidBecomeFrameset(bool) = 0; // Can change due to navigation or DOM modification.
 
     virtual bool canCachePage() const = 0;
-    virtual void convertMainResourceLoadToDownload(DocumentLoader*, PAL::SessionID, const ResourceRequest&, const ResourceResponse&) = 0;
+    virtual void convertMainResourceLoadToDownload(DocumentLoader*, const ResourceRequest&, const ResourceResponse&) = 0;
 
     virtual RefPtr<Frame> createFrame(const URL&, const String& name, HTMLFrameOwnerElement&, const String& referrer) = 0;
     virtual RefPtr<Widget> createPlugin(const IntSize&, HTMLPlugInElement&, const URL&, const Vector<String>&, const Vector<String>&, const String&, bool loadManually) = 0;
index fe12ec1..6b4f49f 100644 (file)
@@ -188,16 +188,6 @@ Frame* CachedResourceLoader::frame() const
     return m_documentLoader ? m_documentLoader->frame() : nullptr;
 }
 
-PAL::SessionID CachedResourceLoader::sessionID() const
-{
-    auto sessionID = PAL::SessionID::defaultSessionID();
-    if (auto* frame = this->frame()) {
-        if (auto* page = frame->page())
-            sessionID = page->sessionID();
-    }
-    return sessionID;
-}
-
 ResourceErrorOr<CachedResourceHandle<CachedImage>> CachedResourceLoader::requestImage(CachedResourceRequest&& request)
 {
     if (Frame* frame = this->frame()) {
@@ -247,7 +237,7 @@ CachedResourceHandle<CachedCSSStyleSheet> CachedResourceLoader::requestUserCSSSt
 
     auto& memoryCache = MemoryCache::singleton();
     if (request.allowsCaching()) {
-        if (CachedResource* existing = memoryCache.resourceForRequest(request.resourceRequest(), sessionID())) {
+        if (CachedResource* existing = memoryCache.resourceForRequest(request.resourceRequest(), page.sessionID())) {
             if (is<CachedCSSStyleSheet>(*existing))
                 return downcast<CachedCSSStyleSheet>(existing);
             memoryCache.remove(*existing);
@@ -687,14 +677,14 @@ static inline bool isResourceSuitableForDirectReuse(const CachedResource& resour
     return true;
 }
 
-CachedResourceHandle<CachedResource> CachedResourceLoader::updateCachedResourceWithCurrentRequest(const CachedResource& resource, CachedResourceRequest&& request, const PAL::SessionID& sessionID, const CookieJar* cookieJar)
+CachedResourceHandle<CachedResource> CachedResourceLoader::updateCachedResourceWithCurrentRequest(const CachedResource& resource, CachedResourceRequest&& request, const PAL::SessionID& sessionID, const CookieJar& cookieJar)
 {
     if (!isResourceSuitableForDirectReuse(resource, request)) {
         request.setCachingPolicy(CachingPolicy::DisallowCaching);
-        return loadResource(resource.type(), WTFMove(request), cookieJar);
+        return loadResource(resource.type(), sessionID, WTFMove(request), cookieJar);
     }
 
-    auto resourceHandle = createResource(resource.type(), WTFMove(request), sessionID, cookieJar);
+    auto resourceHandle = createResource(resource.type(), WTFMove(request), sessionID, &cookieJar);
     resourceHandle->loadFrom(resource);
     return resourceHandle;
 }
@@ -787,6 +777,13 @@ static FetchOptions::Destination destinationForType(CachedResource::Type type)
 
 ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requestResource(CachedResource::Type type, CachedResourceRequest&& request, ForPreload forPreload, DeferOption defer)
 {
+    if (!frame() || !frame()->page()) {
+        RELEASE_LOG_IF_ALLOWED("requestResource: failed because no frame or page");
+        return makeUnexpected(ResourceError { errorDomainWebKitInternal, 0, request.resourceRequest().url(), "Invalid loader state"_s });
+    }
+    auto& frame = *this->frame();
+    auto& page = *frame.page();
+
     request.setDestinationIfNotSet(destinationForType(type));
 
     // Entry point to https://fetch.spec.whatwg.org/#main-fetch.
@@ -800,7 +797,7 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
     if (Document* document = this->document())
         request.upgradeInsecureRequestIfNeeded(*document);
 
-    if (InspectorInstrumentation::willInterceptRequest(frame(), request.resourceRequest()))
+    if (InspectorInstrumentation::willInterceptRequest(&frame, request.resourceRequest()))
         request.setCachingPolicy(CachingPolicy::DisallowCaching);
 
     request.updateReferrerPolicy(document() ? document()->referrerPolicy() : ReferrerPolicy::NoReferrerWhenDowngrade);
@@ -809,7 +806,7 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
     LOG(ResourceLoading, "CachedResourceLoader::requestResource '%.255s', charset '%s', priority=%d, forPreload=%u", url.stringCenterEllipsizedToLength().latin1().data(), request.charset().latin1().data(), request.priority() ? static_cast<int>(request.priority().value()) : -1, forPreload == ForPreload::Yes);
 
     if (!url.isValid()) {
-        RELEASE_LOG_IF_ALLOWED("requestResource: URL is invalid (frame = %p)", frame());
+        RELEASE_LOG_IF_ALLOWED("requestResource: URL is invalid (frame = %p)", &frame);
         return makeUnexpected(ResourceError { errorDomainWebKitInternal, 0, url, "URL is invalid"_s });
     }
 
@@ -817,22 +814,21 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
 
     // We are passing url as well as request, as request url may contain a fragment identifier.
     if (!canRequest(type, url, request, forPreload)) {
-        RELEASE_LOG_IF_ALLOWED("requestResource: Not allowed to request resource (frame = %p)", frame());
+        RELEASE_LOG_IF_ALLOWED("requestResource: Not allowed to request resource (frame = %p)", &frame);
         return makeUnexpected(ResourceError { errorDomainWebKitInternal, 0, url, "Not allowed to request resource"_s, ResourceError::Type::AccessControl });
     }
 
 #if ENABLE(CONTENT_EXTENSIONS)
-    if (frame() && frame()->page() && m_documentLoader) {
+    if (m_documentLoader) {
         const auto& resourceRequest = request.resourceRequest();
-        auto* page = frame()->page();
-        auto results = page->userContentProvider().processContentRuleListsForLoad(resourceRequest.url(), ContentExtensions::toResourceType(type), *m_documentLoader);
+        auto results = page.userContentProvider().processContentRuleListsForLoad(resourceRequest.url(), ContentExtensions::toResourceType(type), *m_documentLoader);
         bool blockedLoad = results.summary.blockedLoad;
         bool madeHTTPS = results.summary.madeHTTPS;
-        request.applyResults(WTFMove(results), page);
+        request.applyResults(WTFMove(results), &page);
         if (blockedLoad) {
-            RELEASE_LOG_IF_ALLOWED("requestResource: Resource blocked by content blocker (frame = %p)", frame());
+            RELEASE_LOG_IF_ALLOWED("requestResource: Resource blocked by content blocker (frame = %p)", &frame);
             if (type == CachedResource::Type::MainResource) {
-                CachedResourceHandle<CachedResource> resource = createResource(type, WTFMove(request), page->sessionID(), &page->cookieJar());
+                CachedResourceHandle<CachedResource> resource = createResource(type, WTFMove(request), page.sessionID(), &page.cookieJar());
                 ASSERT(resource);
                 resource->error(CachedResource::Status::LoadError);
                 resource->setResourceError(ResourceError(ContentExtensions::WebKitContentBlockerDomain, 0, resourceRequest.url(), WEB_UI_STRING("The URL was blocked by a content blocker", "WebKitErrorBlockedByContentBlocker description")));
@@ -851,13 +847,13 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
     }
 #endif
 
-    if (frame() && m_documentLoader && !m_documentLoader->customHeaderFields().isEmpty()) {
+    if (m_documentLoader && !m_documentLoader->customHeaderFields().isEmpty()) {
         bool sameOriginRequest = false;
         auto requestedOrigin = SecurityOrigin::create(url);
         if (type == CachedResource::Type::MainResource) {
-            if (frame()->isMainFrame())
+            if (frame.isMainFrame())
                 sameOriginRequest = true;
-            else if (auto* topDocument = frame()->mainFrame().document())
+            else if (auto* topDocument = frame.mainFrame().document())
                 sameOriginRequest = topDocument->securityOrigin().isSameSchemeHostPort(requestedOrigin.get());
         } else if (document()) {
             sameOriginRequest = document()->topDocument().securityOrigin().isSameSchemeHostPort(requestedOrigin.get())
@@ -888,14 +884,14 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
         request.setDomainForCachePartition(*document());
 
     if (request.allowsCaching())
-        resource = memoryCache.resourceForRequest(request.resourceRequest(), sessionID());
+        resource = memoryCache.resourceForRequest(request.resourceRequest(), page.sessionID());
 
     if (resource && request.isLinkPreload() && !resource->isLinkPreload())
         resource->setLinkPreload();
 
-    logMemoryCacheResourceRequest(frame(), DiagnosticLoggingKeys::memoryCacheUsageKey(), resource ? DiagnosticLoggingKeys::inMemoryCacheKey() : DiagnosticLoggingKeys::notInMemoryCacheKey());
+    logMemoryCacheResourceRequest(&frame, DiagnosticLoggingKeys::memoryCacheUsageKey(), resource ? DiagnosticLoggingKeys::inMemoryCacheKey() : DiagnosticLoggingKeys::notInMemoryCacheKey());
 
-    auto* cookieJar = document() && document()->page() ? &document()->page()->cookieJar() : nullptr;
+    auto& cookieJar = page.cookieJar();
 
     RevalidationPolicy policy = determineRevalidationPolicy(type, request, resource.get(), forPreload, defer);
     switch (policy) {
@@ -904,12 +900,12 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
         FALLTHROUGH;
     case Load:
         if (resource)
-            logMemoryCacheResourceRequest(frame(), DiagnosticLoggingKeys::memoryCacheEntryDecisionKey(), DiagnosticLoggingKeys::unusedKey());
-        resource = loadResource(type, WTFMove(request), cookieJar);
+            logMemoryCacheResourceRequest(&frame, DiagnosticLoggingKeys::memoryCacheEntryDecisionKey(), DiagnosticLoggingKeys::unusedKey());
+        resource = loadResource(type, page.sessionID(), WTFMove(request), cookieJar);
         break;
     case Revalidate:
         if (resource)
-            logMemoryCacheResourceRequest(frame(), DiagnosticLoggingKeys::memoryCacheEntryDecisionKey(), DiagnosticLoggingKeys::revalidatingKey());
+            logMemoryCacheResourceRequest(&frame, DiagnosticLoggingKeys::memoryCacheEntryDecisionKey(), DiagnosticLoggingKeys::revalidatingKey());
         resource = revalidateResource(WTFMove(request), *resource);
         break;
     case Use:
@@ -919,14 +915,14 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
                 return makeUnexpected(WTFMove(*error));
         }
         if (shouldUpdateCachedResourceWithCurrentRequest(*resource, request)) {
-            resource = updateCachedResourceWithCurrentRequest(*resource, WTFMove(request), document()->page()->sessionID(), cookieJar);
+            resource = updateCachedResourceWithCurrentRequest(*resource, WTFMove(request), page.sessionID(), cookieJar);
             if (resource->status() != CachedResource::Status::Cached)
                 policy = Load;
         } else {
             ResourceError error;
             if (!shouldContinueAfterNotifyingLoadedFromMemoryCache(request, *resource, error))
                 return makeUnexpected(WTFMove(error));
-            logMemoryCacheResourceRequest(frame(), DiagnosticLoggingKeys::memoryCacheEntryDecisionKey(), DiagnosticLoggingKeys::usedKey());
+            logMemoryCacheResourceRequest(&frame, DiagnosticLoggingKeys::memoryCacheEntryDecisionKey(), DiagnosticLoggingKeys::usedKey());
             loadTiming.setResponseEnd(MonotonicTime::now());
 
             memoryCache.resourceAccessed(*resource);
@@ -938,7 +934,7 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
                     downcast<CachedRawResource>(resource.get())->finishedTimingForWorkerLoad(WTFMove(resourceTiming));
                 } else {
                     ASSERT(initiatorContext == InitiatorContext::Document);
-                    m_resourceTimingInfo.storeResourceTimingInitiatorInformation(resource, request.initiatorName(), frame());
+                    m_resourceTimingInfo.storeResourceTimingInitiatorInformation(resource, request.initiatorName(), &frame);
                     m_resourceTimingInfo.addResourceTiming(*resource.get(), *document(), WTFMove(resourceTiming));
                 }
             }
@@ -1002,7 +998,6 @@ CachedResourceHandle<CachedResource> CachedResourceLoader::revalidateResource(Ca
     ASSERT(!memoryCache.disabled());
     ASSERT(resource.canUseCacheValidator());
     ASSERT(!resource.resourceToRevalidate());
-    ASSERT(resource.sessionID() == sessionID());
     ASSERT(resource.allowsCaching());
 
     CachedResourceHandle<CachedResource> newResource = createResource(resource.type(), WTFMove(request), resource.sessionID(), resource.cookieJar());
@@ -1019,15 +1014,15 @@ CachedResourceHandle<CachedResource> CachedResourceLoader::revalidateResource(Ca
     return newResource;
 }
 
-CachedResourceHandle<CachedResource> CachedResourceLoader::loadResource(CachedResource::Type type, CachedResourceRequest&& request, const CookieJar* cookieJar)
+CachedResourceHandle<CachedResource> CachedResourceLoader::loadResource(CachedResource::Type type, PAL::SessionID sessionID, CachedResourceRequest&& request, const CookieJar& cookieJar)
 {
     auto& memoryCache = MemoryCache::singleton();
-    ASSERT(!request.allowsCaching() || !memoryCache.resourceForRequest(request.resourceRequest(), sessionID())
+    ASSERT(!request.allowsCaching() || !memoryCache.resourceForRequest(request.resourceRequest(), sessionID)
         || request.resourceRequest().cachePolicy() == ResourceRequestCachePolicy::DoNotUseAnyCache || request.resourceRequest().cachePolicy() == ResourceRequestCachePolicy::ReloadIgnoringCacheData || request.resourceRequest().cachePolicy() == ResourceRequestCachePolicy::RefreshAnyCacheData);
 
     LOG(ResourceLoading, "Loading CachedResource for '%s'.", request.resourceRequest().url().stringCenterEllipsizedToLength().latin1().data());
 
-    CachedResourceHandle<CachedResource> resource = createResource(type, WTFMove(request), sessionID(), cookieJar);
+    CachedResourceHandle<CachedResource> resource = createResource(type, WTFMove(request), sessionID, &cookieJar);
 
     if (resource->allowsCaching())
         memoryCache.add(*resource);
index 4e9c1a0..15204fe 100644 (file)
@@ -129,7 +129,6 @@ public:
     Document* document() const { return m_document.get(); } // Can be null
     void setDocument(Document* document) { m_document = makeWeakPtr(document); }
     void clearDocumentLoader() { m_documentLoader = nullptr; }
-    PAL::SessionID sessionID() const;
 
     void loadDone(LoadCompletionType, bool shouldPerformPostLoadActions = true);
 
@@ -168,7 +167,7 @@ private:
 
     ResourceErrorOr<CachedResourceHandle<CachedResource>> requestResource(CachedResource::Type, CachedResourceRequest&&, ForPreload = ForPreload::No, DeferOption = DeferOption::NoDefer);
     CachedResourceHandle<CachedResource> revalidateResource(CachedResourceRequest&&, CachedResource&);
-    CachedResourceHandle<CachedResource> loadResource(CachedResource::Type, CachedResourceRequest&&, const CookieJar*);
+    CachedResourceHandle<CachedResource> loadResource(CachedResource::Type, PAL::SessionID, CachedResourceRequest&&, const CookieJar&);
 
     void prepareFetch(CachedResource::Type, CachedResourceRequest&);
     void updateHTTPRequestHeaders(CachedResource::Type, CachedResourceRequest&);
@@ -179,7 +178,7 @@ private:
     RevalidationPolicy determineRevalidationPolicy(CachedResource::Type, CachedResourceRequest&, CachedResource* existingResource, ForPreload, DeferOption) const;
 
     bool shouldUpdateCachedResourceWithCurrentRequest(const CachedResource&, const CachedResourceRequest&);
-    CachedResourceHandle<CachedResource> updateCachedResourceWithCurrentRequest(const CachedResource&, CachedResourceRequest&&, const PAL::SessionID&, const CookieJar*);
+    CachedResourceHandle<CachedResource> updateCachedResourceWithCurrentRequest(const CachedResource&, CachedResourceRequest&&, const PAL::SessionID&, const CookieJar&);
 
     bool shouldContinueAfterNotifyingLoadedFromMemoryCache(const CachedResourceRequest&, CachedResource&, ResourceError&);
     bool checkInsecureContent(CachedResource::Type, const URL&) const;
index efaa39d..6466a60 100644 (file)
@@ -1013,10 +1013,4 @@ void Frame::selfOnlyDeref()
     deref();
 }
 
-PAL::SessionID Frame::sessionID() const
-{
-    auto* page = this->page();
-    return page ? page->sessionID() : PAL::SessionID::defaultSessionID();
-}
-
 } // namespace WebCore
index 3addc2b..d8b9716 100644 (file)
@@ -301,8 +301,6 @@ public:
     void selfOnlyRef();
     void selfOnlyDeref();
 
-    PAL::SessionID sessionID() const;
-
 private:
     friend class NavigationDisabler;
 
index b2654d0..95d2ce6 100644 (file)
@@ -622,8 +622,8 @@ bool ServiceWorkerContainer::isAlwaysOnLoggingAllowed() const
         return false;
 
     if (is<Document>(*context)) {
-        auto sessionID = downcast<Document>(*context).sessionID();
-        return sessionID && sessionID->isAlwaysOnLoggingAllowed();
+        auto* page = downcast<Document>(*context).page();
+        return page && page->sessionID().isAlwaysOnLoggingAllowed();
     }
 
     // FIXME: No logging inside service workers for now.
index 6394cd4..a38c50d 100644 (file)
@@ -1,5 +1,19 @@
 2019-09-21  Chris Dumez  <cdumez@apple.com>
 
+        Reduce use of SessionID::defaultSessionID() in WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=202080
+
+        Reviewed by Alex Christensen.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::convertMainResourceLoadToDownload):
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+        * WebProcess/WebPage/WebFrame.cpp:
+        (WebKit::WebFrame::convertMainResourceLoadToDownload):
+        * WebProcess/WebPage/WebFrame.h:
+
+2019-09-21  Chris Dumez  <cdumez@apple.com>
+
         Drop unnecessary NetworkProcess::m_sessionByConnection
         https://bugs.webkit.org/show_bug.cgi?id=202088
 
index 8947198..9fa15e0 100644 (file)
@@ -1564,9 +1564,9 @@ bool WebFrameLoaderClient::canCachePage() const
     return !m_frameHasCustomContentProvider;
 }
 
-void WebFrameLoaderClient::convertMainResourceLoadToDownload(DocumentLoader *documentLoader, PAL::SessionID sessionID, const ResourceRequest& request, const ResourceResponse& response)
+void WebFrameLoaderClient::convertMainResourceLoadToDownload(DocumentLoader *documentLoader, const ResourceRequest& request, const ResourceResponse& response)
 {
-    m_frame->convertMainResourceLoadToDownload(documentLoader, sessionID, request, response);
+    m_frame->convertMainResourceLoadToDownload(documentLoader, request, response);
 }
 
 RefPtr<Frame> WebFrameLoaderClient::createFrame(const URL& url, const String& name, HTMLFrameOwnerElement& ownerElement,
index 0468b6b..cde2ada 100644 (file)
@@ -217,7 +217,7 @@ private:
     void dispatchDidBecomeFrameset(bool) final;
 
     bool canCachePage() const final;
-    void convertMainResourceLoadToDownload(WebCore::DocumentLoader*, PAL::SessionID, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&) final;
+    void convertMainResourceLoadToDownload(WebCore::DocumentLoader*, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&) final;
 
     RefPtr<WebCore::Frame> createFrame(const URL&, const String& name, WebCore::HTMLFrameOwnerElement&, const String& referrer) final;
 
index b485c56..1dfe39d 100644 (file)
@@ -286,7 +286,7 @@ void WebFrame::startDownload(const WebCore::ResourceRequest& request, const Stri
     WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::StartDownload(policyDownloadID, request, suggestedName), 0);
 }
 
-void WebFrame::convertMainResourceLoadToDownload(DocumentLoader* documentLoader, PAL::SessionID sessionID, const ResourceRequest& request, const ResourceResponse& response)
+void WebFrame::convertMainResourceLoadToDownload(DocumentLoader* documentLoader, const ResourceRequest& request, const ResourceResponse& response)
 {
     ASSERT(m_policyDownloadID.downloadID());
 
index 3df056c..1b4dcbc 100644 (file)
@@ -92,7 +92,7 @@ public:
     void continueWillSubmitForm(uint64_t);
 
     void startDownload(const WebCore::ResourceRequest&, const String& suggestedName = { });
-    void convertMainResourceLoadToDownload(WebCore::DocumentLoader*, PAL::SessionID, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&);
+    void convertMainResourceLoadToDownload(WebCore::DocumentLoader*, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&);
 
     void addConsoleMessage(MessageSource, MessageLevel, const String&, uint64_t requestID = 0);
 
index 9b1bbe0..32fd792 100644 (file)
@@ -1,3 +1,14 @@
+2019-09-21  Chris Dumez  <cdumez@apple.com>
+
+        Reduce use of SessionID::defaultSessionID() in WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=202080
+
+        Reviewed by Alex Christensen.
+
+        * WebCoreSupport/WebFrameLoaderClient.h:
+        * WebCoreSupport/WebFrameLoaderClient.mm:
+        (WebFrameLoaderClient::convertMainResourceLoadToDownload):
+
 2019-09-20  Keith Rollin  <krollin@apple.com>
 
         Remove some support for < iOS 13
index 534bbb8..83240b9 100644 (file)
@@ -78,7 +78,7 @@ private:
     void detachedFromParent2() final;
     void detachedFromParent3() final;
 
-    void convertMainResourceLoadToDownload(WebCore::DocumentLoader*, PAL::SessionID, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&) final;
+    void convertMainResourceLoadToDownload(WebCore::DocumentLoader*, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&) final;
 
     void assignIdentifierToInitialRequest(unsigned long identifier, WebCore::DocumentLoader*, const WebCore::ResourceRequest&) final;
 
index 1376612..d5edd58 100644 (file)
@@ -302,7 +302,7 @@ void WebFrameLoaderClient::detachedFromParent3()
     m_webFrame->_private->webFrameView = nil;
 }
 
-void WebFrameLoaderClient::convertMainResourceLoadToDownload(DocumentLoader* documentLoader, PAL::SessionID, const ResourceRequest& request, const ResourceResponse& response)
+void WebFrameLoaderClient::convertMainResourceLoadToDownload(DocumentLoader* documentLoader, const ResourceRequest& request, const ResourceResponse& response)
 {
     WebView *webView = getWebView(m_webFrame.get());
     SubresourceLoader* mainResourceLoader = documentLoader->mainResourceLoader();
index 3f79e8a..962fcc8 100644 (file)
@@ -1,3 +1,14 @@
+2019-09-21  Chris Dumez  <cdumez@apple.com>
+
+        Reduce use of SessionID::defaultSessionID() in WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=202080
+
+        Reviewed by Alex Christensen.
+
+        * WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebFrameLoaderClient::convertMainResourceLoadToDownload):
+        * WebCoreSupport/WebFrameLoaderClient.h:
+
 2019-09-18  Chris Dumez  <cdumez@apple.com>
 
         BlobRegistry no longer needs SessionIDs
index 29b8547..d4fb5d7 100644 (file)
@@ -163,7 +163,7 @@ void WebFrameLoaderClient::detachedFromParent3()
     notImplemented();
 }
 
-void WebFrameLoaderClient::convertMainResourceLoadToDownload(DocumentLoader* documentLoader, PAL::SessionID, const ResourceRequest& request, const ResourceResponse& response)
+void WebFrameLoaderClient::convertMainResourceLoadToDownload(DocumentLoader* documentLoader, const ResourceRequest& request, const ResourceResponse& response)
 {
     COMPtr<IWebDownloadDelegate> downloadDelegate;
     COMPtr<IWebView> webView;
index 1a2d2f8..33a86ce 100644 (file)
@@ -67,7 +67,7 @@ public:
     void detachedFromParent2() override;
     void detachedFromParent3() override;
 
-    void convertMainResourceLoadToDownload(WebCore::DocumentLoader*, PAL::SessionID, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&) override;
+    void convertMainResourceLoadToDownload(WebCore::DocumentLoader*, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&) override;
     void assignIdentifierToInitialRequest(unsigned long identifier, WebCore::DocumentLoader*, const WebCore::ResourceRequest&) override;
 
     void dispatchWillSendRequest(WebCore::DocumentLoader*, unsigned long identifier, WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse) override;