Changeset 240340 in webkit

Timestamp:

Jan 23, 2019 9:24:19 AM (5 years ago)

Author:

Michael Catanzaro

Message:

[SOUP] Clean up NetworkStorageSession
​https://bugs.webkit.org/show_bug.cgi?id=193707

Reviewed by Carlos Garcia Campos.

Source/WebCore:

A NetworkStorageSession now always has a SoupNetworkSession, so we can remove a lot of
complexity that is no longer needed. getOrCreateSoupNetworkSession can go away because we
know the session has always already been created. The soupNetworkSession getter can now
return a reference rather than a pointer, because it will never be NULL except after it has
been cleared with clearSoupNetworkSession (renamed), and that should only happen immediately
before process termination after nothing else is using it. Cookie jar syncing can also go
away; the NetworkStorageSession can now rely on the SoupNetworkSession to exist and just
use its cookie jar.

• platform/network/NetworkStorageSession.h:

(WebCore::NetworkStorageSession::soupNetworkSession const): Deleted.

• platform/network/soup/DNSResolveQueueSoup.cpp:

(WebCore::DNSResolveQueueSoup::updateIsUsingProxy):
(WebCore::DNSResolveQueueSoup::platformResolve):
(WebCore::DNSResolveQueueSoup::resolve):

• platform/network/soup/NetworkStorageSessionSoup.cpp:

(WebCore::NetworkStorageSession::NetworkStorageSession):
(WebCore::NetworkStorageSession::~NetworkStorageSession):
(WebCore::NetworkStorageSession::soupNetworkSession const):
(WebCore::NetworkStorageSession::clearSoupNetworkSession):
(WebCore::NetworkStorageSession::cookieStorage const):
(WebCore::NetworkStorageSession::setCookieStorage):
(WebCore::NetworkStorageSession::getOrCreateSoupNetworkSession const): Deleted.
(WebCore::NetworkStorageSession::clearSoupNetworkSessionAndCookieStorage): Deleted.

• platform/network/soup/SocketStreamHandleImplSoup.cpp:

(WebCore::SocketStreamHandleImpl::create):

Source/WebKit:

Restore an important call to clearSoupNetworkSession that was removed in r240292. Also,
adapt to NetworkStorageSessions API changes.

• NetworkProcess/CustomProtocols/soup/LegacyCustomProtocolManagerSoup.cpp:

(WebKit::LegacyCustomProtocolManager::registerScheme):

• NetworkProcess/soup/NetworkProcessMainSoup.cpp:
• NetworkProcess/soup/NetworkProcessSoup.cpp:

(WebKit::NetworkProcess::userPreferredLanguagesChanged):
(WebKit::NetworkProcess::setNetworkProxySettings):

• NetworkProcess/soup/NetworkSessionSoup.cpp:

(WebKit::NetworkSessionSoup::soupSession const):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/platform/network/NetworkStorageSession.h (modified) (2 diffs)
• WebCore/platform/network/soup/DNSResolveQueueSoup.cpp (modified) (3 diffs)
• WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp (modified) (2 diffs)
• WebCore/platform/network/soup/SocketStreamHandleImplSoup.cpp (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/NetworkProcess/CustomProtocols/soup/LegacyCustomProtocolManagerSoup.cpp (modified) (1 diff)
• WebKit/NetworkProcess/soup/NetworkProcessMainSoup.cpp (modified) (1 diff)
• WebKit/NetworkProcess/soup/NetworkProcessSoup.cpp (modified) (2 diffs)
• WebKit/NetworkProcess/soup/NetworkSessionSoup.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-01-23  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        [SOUP] Clean up NetworkStorageSession
+        https://bugs.webkit.org/show_bug.cgi?id=193707
+
+        Reviewed by Carlos Garcia Campos.
+
+        A NetworkStorageSession now always has a SoupNetworkSession, so we can remove a lot of
+        complexity that is no longer needed. getOrCreateSoupNetworkSession can go away because we
+        know the session has always already been created. The soupNetworkSession getter can now
+        return a reference rather than a pointer, because it will never be NULL except after it has
+        been cleared with clearSoupNetworkSession (renamed), and that should only happen immediately
+        before process termination after nothing else is using it. Cookie jar syncing can also go
+        away; the NetworkStorageSession can now rely on the SoupNetworkSession to exist and just
+        use its cookie jar.
+
+        * platform/network/NetworkStorageSession.h:
+        (WebCore::NetworkStorageSession::soupNetworkSession const): Deleted.
+        * platform/network/soup/DNSResolveQueueSoup.cpp:
+        (WebCore::DNSResolveQueueSoup::updateIsUsingProxy):
+        (WebCore::DNSResolveQueueSoup::platformResolve):
+        (WebCore::DNSResolveQueueSoup::resolve):
+        * platform/network/soup/NetworkStorageSessionSoup.cpp:
+        (WebCore::NetworkStorageSession::NetworkStorageSession):
+        (WebCore::NetworkStorageSession::~NetworkStorageSession):
+        (WebCore::NetworkStorageSession::soupNetworkSession const):
+        (WebCore::NetworkStorageSession::clearSoupNetworkSession):
+        (WebCore::NetworkStorageSession::cookieStorage const):
+        (WebCore::NetworkStorageSession::setCookieStorage):
+        (WebCore::NetworkStorageSession::getOrCreateSoupNetworkSession const): Deleted.
+        (WebCore::NetworkStorageSession::clearSoupNetworkSessionAndCookieStorage): Deleted.
+        * platform/network/soup/SocketStreamHandleImplSoup.cpp:
+        (WebCore::SocketStreamHandleImpl::create):
+
 2019-01-23  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/platform/network/NetworkStorageSession.h

@@ -98 +98 @@
     ~NetworkStorageSession();
 
-    SoupNetworkSession* soupNetworkSession() const { return m_session.get(); };
-    SoupNetworkSession& getOrCreateSoupNetworkSession() const;
-    void clearSoupNetworkSessionAndCookieStorage();
+    SoupNetworkSession& soupNetworkSession() const;
+    void clearSoupNetworkSession();
     SoupCookieJar* cookieStorage() const;
     void setCookieStorage(SoupCookieJar*);
@@ -169 +168 @@
 
     mutable std::unique_ptr<SoupNetworkSession> m_session;
-    GRefPtr<SoupCookieJar> m_cookieStorage;
     Function<void ()> m_cookieObserverHandler;
 #elif USE(CURL)

trunk/Source/WebCore/platform/network/soup/DNSResolveQueueSoup.cpp

@@ -93 +93 @@
 {
     GRefPtr<GProxyResolver> resolver;
-    g_object_get(globalDefaultNetworkStorageSessionAccessor()().getOrCreateSoupNetworkSession().soupSession(), "proxy-resolver", &resolver.outPtr(), nullptr);
+    g_object_get(globalDefaultNetworkStorageSessionAccessor()().soupNetworkSession().soupSession(), "proxy-resolver", &resolver.outPtr(), nullptr);
     ASSERT(resolver);
 
@@ -176 +176 @@
     ASSERT(isMainThread());
 
-    soup_session_prefetch_dns(globalDefaultNetworkStorageSessionAccessor()().getOrCreateSoupNetworkSession().soupSession(), hostname.utf8().data(), nullptr, resolvedCallback, nullptr);
+    soup_session_prefetch_dns(globalDefaultNetworkStorageSessionAccessor()().soupNetworkSession().soupSession(), hostname.utf8().data(), nullptr, resolvedCallback, nullptr);
 }
 
@@ -185 +185 @@
     auto address = adoptGRef(soup_address_new(hostname.utf8().data(), 0));
     auto cancellable = adoptGRef(g_cancellable_new());
-    soup_address_resolve_async(address.get(), soup_session_get_async_context(WebCore::globalDefaultNetworkStorageSessionAccessor()().getOrCreateSoupNetworkSession().soupSession()), cancellable.get(), resolvedWithObserverCallback, this);
+    soup_address_resolve_async(address.get(), soup_session_get_async_context(WebCore::globalDefaultNetworkStorageSessionAccessor()().soupNetworkSession().soupSession()), cancellable.get(), resolvedWithObserverCallback, this);
 
     g_object_set_data(G_OBJECT(address.get()), "identifier", GUINT_TO_POINTER(identifier));

trunk/Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp

@@ -58 +58 @@
     , m_session(WTFMove(session))
 {
-    setCookieStorage(m_session ? m_session->cookieJar() : nullptr);
+    ASSERT(m_session->cookieJar());
+    g_signal_connect_swapped(m_session->cookieJar(), "changed", G_CALLBACK(cookiesDidChange), this);
 }
 
 NetworkStorageSession::~NetworkStorageSession()
 {
-    g_signal_handlers_disconnect_matched(m_cookieStorage.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
-}
-
-SoupNetworkSession& NetworkStorageSession::getOrCreateSoupNetworkSession() const
-{
-    if (!m_session)
-        m_session = std::make_unique<SoupNetworkSession>(m_sessionID, m_cookieStorage.get());
-    return *m_session;
-}
-
-void NetworkStorageSession::clearSoupNetworkSessionAndCookieStorage()
-{
+    clearSoupNetworkSession();
+}
+
+SoupNetworkSession& NetworkStorageSession::soupNetworkSession() const
+{
+    ASSERT(m_session);
+    return *m_session.get();
+};
+
+void NetworkStorageSession::clearSoupNetworkSession()
+{
+    if (m_session) {
+        ASSERT(m_session->cookieJar());
+        g_signal_handlers_disconnect_matched(m_session->cookieJar(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
+    }
+
     m_session = nullptr;
     m_cookieObserverHandler = nullptr;
-    m_cookieStorage = nullptr;
 }
 
@@ -88 +92 @@
 SoupCookieJar* NetworkStorageSession::cookieStorage() const
 {
-    RELEASE_ASSERT(!m_session || m_session->cookieJar() == m_cookieStorage.get());
-    return m_cookieStorage.get();
+    ASSERT(m_session);
+    ASSERT(m_session->cookieJar());
+    return m_session->cookieJar();
 }
 
 void NetworkStorageSession::setCookieStorage(SoupCookieJar* jar)
 {
-    if (m_cookieStorage)
-        g_signal_handlers_disconnect_matched(m_cookieStorage.get(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
-
-    // We always have a valid cookieStorage.
-    if (jar)
-        m_cookieStorage = jar;
-    else {
-        m_cookieStorage = adoptGRef(soup_cookie_jar_new());
-        soup_cookie_jar_set_accept_policy(m_cookieStorage.get(), SOUP_COOKIE_JAR_ACCEPT_NO_THIRD_PARTY);
-    }
-    g_signal_connect_swapped(m_cookieStorage.get(), "changed", G_CALLBACK(cookiesDidChange), this);
-    if (m_session && m_session->cookieJar() != m_cookieStorage.get())
-        m_session->setCookieJar(m_cookieStorage.get());
+    ASSERT(jar);
+    ASSERT(m_session);
+    ASSERT(m_session->cookieJar());
+
+    if (m_session->cookieJar() == jar)
+        return;
+
+    g_signal_handlers_disconnect_matched(m_session->cookieJar(), G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
+    m_session->setCookieJar(jar);
+    g_signal_connect_swapped(m_session->cookieJar(), "changed", G_CALLBACK(cookiesDidChange), this);
 }
 

trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImplSoup.cpp

@@ -94 +94 @@
     auto uri = urlToSoupURI(url);
     Ref<SocketStreamHandle> protectedSocketStreamHandle = socket.copyRef();
-    soup_session_connect_async(networkStorageSession->getOrCreateSoupNetworkSession().soupSession(), uri.get(), socket->m_cancellable.get(),
+    soup_session_connect_async(networkStorageSession->soupNetworkSession().soupSession(), uri.get(), socket->m_cancellable.get(),
         url.protocolIs("wss") ? reinterpret_cast<SoupSessionConnectProgressCallback>(connectProgressCallback) : nullptr,
         reinterpret_cast<GAsyncReadyCallback>(connectedCallback), &protectedSocketStreamHandle.leakRef());

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-01-23  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        [SOUP] Clean up NetworkStorageSession
+        https://bugs.webkit.org/show_bug.cgi?id=193707
+
+        Reviewed by Carlos Garcia Campos.
+
+        Restore an important call to clearSoupNetworkSession that was removed in r240292. Also,
+        adapt to NetworkStorageSessions API changes.
+
+        * NetworkProcess/CustomProtocols/soup/LegacyCustomProtocolManagerSoup.cpp:
+        (WebKit::LegacyCustomProtocolManager::registerScheme):
+        * NetworkProcess/soup/NetworkProcessMainSoup.cpp:
+        * NetworkProcess/soup/NetworkProcessSoup.cpp:
+        (WebKit::NetworkProcess::userPreferredLanguagesChanged):
+        (WebKit::NetworkProcess::setNetworkProxySettings):
+        * NetworkProcess/soup/NetworkSessionSoup.cpp:
+        (WebKit::NetworkSessionSoup::soupSession const):
+
 2019-01-23  Philippe Normand  <pnormand@igalia.com>
 

trunk/Source/WebKit/NetworkProcess/CustomProtocols/soup/LegacyCustomProtocolManagerSoup.cpp

@@ -104 +104 @@
     genericRequestClass->schemes = const_cast<const char**>(reinterpret_cast<char**>(m_registeredSchemes->pdata));
     lastCreatedNetworkProcess()->forEachNetworkStorageSession([](const auto& session) {
-        if (auto* soupSession = session.soupNetworkSession())
-            soupSession->setupCustomProtocols();
+        session.soupNetworkSession().setupCustomProtocols();
     });
 }

trunk/Source/WebKit/NetworkProcess/soup/NetworkProcessMainSoup.cpp

@@ -38 +38 @@
     void platformFinalize() override
     {
+        // Needed to destroy the SoupSession and SoupCookieJar, e.g. to avoid
+        // leaking SQLite temporary journaling files.
+        NetworkProcess::singleton().defaultStorageSession().clearSoupNetworkSession();
     }
 };

trunk/Source/WebKit/NetworkProcess/soup/NetworkProcessSoup.cpp

@@ -97 +97 @@
     SoupNetworkSession::setInitialAcceptLanguages(acceptLanguages);
     forEachNetworkStorageSession([&acceptLanguages](const auto& session) {
-        if (auto* soupSession = session.soupNetworkSession())
-            soupSession->setAcceptLanguages(acceptLanguages);
+        session.soupNetworkSession().setAcceptLanguages(acceptLanguages);
     });
 }
@@ -170 +169 @@
     SoupNetworkSession::setProxySettings(settings);
     forEachNetworkStorageSession([](const auto& session) {
-        if (auto* soupSession = session.soupNetworkSession())
-            soupSession->setupProxy();
+        session.soupNetworkSession().setupProxy();
     });
 }

trunk/Source/WebKit/NetworkProcess/soup/NetworkSessionSoup.cpp

@@ -53 +53 @@
 SoupSession* NetworkSessionSoup::soupSession() const
 {
-    return networkStorageSession().getOrCreateSoupNetworkSession().soupSession();
+    return networkStorageSession().soupNetworkSession().soupSession();
 }