[webkit-changes] [184930] trunk/Source

Wed, 27 May 2015 17:31:13 -0700

Title: [184930] trunk/Source
Revision
184930
Author
[email protected]
Date
2015-05-27 17:30:59 -0700 (Wed, 27 May 2015)

Log Message

[WK2] Local storage areas should get torn down when they have no remaining references.
<https://webkit.org/b/143339>
<rdar://problem/20156436>

Reviewed by Darin Adler.

Source/WebCore:

Add StorageArea::securityOrigin() implementations.

* loader/EmptyClients.cpp:
* storage/StorageArea.h:

Source/WebKit:

Add StorageArea::securityOrigin() implementation.

* Storage/StorageAreaImpl.h:

Source/WebKit2:

Let StorageNamespaceImpl own its StorageAreaMaps weakly instead of through RefPtr.
Ownership is flipped so that StorageAreaMap refs the StorageNamespaceImpl instead.
This allows the StorageAreaMaps to get destroyed once all of its clients are gone.

Practically speaking, this means that the garbage collector now decides when local
storage databases can be closed, instead of us keeping them open for the lifetime
of the web process.

For session storage, it works a bit differently. In the web process, they get torn
down when their last client disappears, but they stay alive in the UI process.
If/when the web process asks the UI process to open session storage for an origin,
the UI process checks if one already exists, and if so, just updates the ID of the
old storage with the new one provided by the web process.

* UIProcess/Storage/StorageManager.cpp:
(WebKit::StorageManager::StorageArea::isSessionStorage):
(WebKit::StorageManager::createTransientLocalStorageMap):
(WebKit::StorageManager::createSessionStorageMap):
(WebKit::StorageManager::destroyStorageMap):
* WebProcess/Storage/StorageAreaImpl.cpp:
(WebKit::StorageAreaImpl::securityOrigin):
* WebProcess/Storage/StorageAreaImpl.h:
* WebProcess/Storage/StorageAreaMap.cpp:
(WebKit::StorageAreaMap::StorageAreaMap):
(WebKit::StorageAreaMap::~StorageAreaMap):
* WebProcess/Storage/StorageAreaMap.h:
(WebKit::StorageAreaMap::securityOrigin):
* WebProcess/Storage/StorageNamespaceImpl.cpp:
(WebKit::StorageNamespaceImpl::didDestroyStorageAreaMap):
(WebKit::StorageNamespaceImpl::storageArea):
* WebProcess/Storage/StorageNamespaceImpl.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (184929 => 184930)


--- trunk/Source/WebCore/ChangeLog	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebCore/ChangeLog	2015-05-28 00:30:59 UTC (rev 184930)
@@ -1,3 +1,16 @@
+2015-05-27  Andreas Kling  <[email protected]>
+
+        [WK2] Local storage areas should get torn down when they have no remaining references.
+        <https://webkit.org/b/143339>
+        <rdar://problem/20156436>
+
+        Reviewed by Darin Adler.
+
+        Add StorageArea::securityOrigin() implementations.
+
+        * loader/EmptyClients.cpp:
+        * storage/StorageArea.h:
+
 2015-05-27  Dean Jackson  <[email protected]>
 
         img.currentSrc problem in strict mode with old picturefill

Modified: trunk/Source/WebCore/loader/EmptyClients.cpp (184929 => 184930)


--- trunk/Source/WebCore/loader/EmptyClients.cpp	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebCore/loader/EmptyClients.cpp	2015-05-28 00:30:59 UTC (rev 184930)
@@ -63,6 +63,7 @@
         virtual bool canAccessStorage(Frame*) override { return false; }
         virtual StorageType storageType() const override { return LocalStorage; }
         virtual size_t memoryBytesUsedByCache() override { return 0; }
+        SecurityOrigin& securityOrigin() override { return SecurityOrigin::createUnique(); }
     };
 
     struct EmptyStorageNamespace final : public StorageNamespace {

Modified: trunk/Source/WebCore/storage/StorageArea.h (184929 => 184930)


--- trunk/Source/WebCore/storage/StorageArea.h	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebCore/storage/StorageArea.h	2015-05-28 00:30:59 UTC (rev 184930)
@@ -34,6 +34,7 @@
 namespace WebCore {
 
 class Frame;
+class SecurityOrigin;
 class StorageSyncManager;
 typedef int ExceptionCode;
 enum StorageType { LocalStorage, SessionStorage };
@@ -58,6 +59,8 @@
     virtual void incrementAccessCount() { }
     virtual void decrementAccessCount() { }
     virtual void closeDatabaseIfIdle() { }
+
+    virtual SecurityOrigin& securityOrigin() = 0;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebKit/ChangeLog (184929 => 184930)


--- trunk/Source/WebKit/ChangeLog	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit/ChangeLog	2015-05-28 00:30:59 UTC (rev 184930)
@@ -1,3 +1,15 @@
+2015-05-27  Andreas Kling  <[email protected]>
+
+        [WK2] Local storage areas should get torn down when they have no remaining references.
+        <https://webkit.org/b/143339>
+        <rdar://problem/20156436>
+
+        Reviewed by Darin Adler.
+
+        Add StorageArea::securityOrigin() implementation.
+
+        * Storage/StorageAreaImpl.h:
+
 2015-05-22  Tim Horton  <[email protected]>
 
         Remove action menu support

Modified: trunk/Source/WebKit/Storage/StorageAreaImpl.h (184929 => 184930)


--- trunk/Source/WebKit/Storage/StorageAreaImpl.h	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit/Storage/StorageAreaImpl.h	2015-05-28 00:30:59 UTC (rev 184930)
@@ -60,6 +60,8 @@
     virtual void decrementAccessCount() override;
     virtual void closeDatabaseIfIdle() override;
 
+    SecurityOrigin& securityOrigin() override { return *m_securityOrigin; }
+
     PassRefPtr<StorageAreaImpl> copy();
     void close();

Modified: trunk/Source/WebKit2/ChangeLog (184929 => 184930)


--- trunk/Source/WebKit2/ChangeLog	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit2/ChangeLog	2015-05-28 00:30:59 UTC (rev 184930)
@@ -1,3 +1,43 @@
+2015-05-27  Andreas Kling  <[email protected]>
+
+        [WK2] Local storage areas should get torn down when they have no remaining references.
+        <https://webkit.org/b/143339>
+        <rdar://problem/20156436>
+
+        Reviewed by Darin Adler.
+
+        Let StorageNamespaceImpl own its StorageAreaMaps weakly instead of through RefPtr.
+        Ownership is flipped so that StorageAreaMap refs the StorageNamespaceImpl instead.
+        This allows the StorageAreaMaps to get destroyed once all of its clients are gone.
+
+        Practically speaking, this means that the garbage collector now decides when local
+        storage databases can be closed, instead of us keeping them open for the lifetime
+        of the web process.
+
+        For session storage, it works a bit differently. In the web process, they get torn
+        down when their last client disappears, but they stay alive in the UI process.
+        If/when the web process asks the UI process to open session storage for an origin,
+        the UI process checks if one already exists, and if so, just updates the ID of the
+        old storage with the new one provided by the web process.
+
+        * UIProcess/Storage/StorageManager.cpp:
+        (WebKit::StorageManager::StorageArea::isSessionStorage):
+        (WebKit::StorageManager::createTransientLocalStorageMap):
+        (WebKit::StorageManager::createSessionStorageMap):
+        (WebKit::StorageManager::destroyStorageMap):
+        * WebProcess/Storage/StorageAreaImpl.cpp:
+        (WebKit::StorageAreaImpl::securityOrigin):
+        * WebProcess/Storage/StorageAreaImpl.h:
+        * WebProcess/Storage/StorageAreaMap.cpp:
+        (WebKit::StorageAreaMap::StorageAreaMap):
+        (WebKit::StorageAreaMap::~StorageAreaMap):
+        * WebProcess/Storage/StorageAreaMap.h:
+        (WebKit::StorageAreaMap::securityOrigin):
+        * WebProcess/Storage/StorageNamespaceImpl.cpp:
+        (WebKit::StorageNamespaceImpl::didDestroyStorageAreaMap):
+        (WebKit::StorageNamespaceImpl::storageArea):
+        * WebProcess/Storage/StorageNamespaceImpl.h:
+
 2015-05-27  Antti Koivisto  <[email protected]>
 
         Disable network cache for old clients

Modified: trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp (184929 => 184930)


--- trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp	2015-05-28 00:30:59 UTC (rev 184930)
@@ -63,6 +63,8 @@
     const HashMap<String, String>& items();
     void clear();
 
+    bool isSessionStorage() const { return !m_localStorageNamespace; }
+
 private:
     explicit StorageArea(LocalStorageNamespace*, RefPtr<SecurityOrigin>&&, unsigned quotaInBytes);
 
@@ -738,6 +740,23 @@
     // FIXME: This should be a message check.
     ASSERT(m_storageAreasByConnection.isValidKey({ &connection, storageMapID }));
 
+    Ref<SecurityOrigin> origin = securityOriginData.securityOrigin();
+
+    // See if we already have session storage for this connection/origin combo.
+    // If so, update the map with the new ID, otherwise keep on trucking.
+    for (auto it = m_storageAreasByConnection.begin(), end = m_storageAreasByConnection.end(); it != end; ++it) {
+        if (it->key.first != &connection)
+            continue;
+        Ref<StorageArea> area = *it->value;
+        if (!area->isSessionStorage())
+            continue;
+        if (!origin->isSameSchemeHostPort(area->securityOrigin()))
+            continue;
+        m_storageAreasByConnection.remove(it);
+        m_storageAreasByConnection.add({ &connection, storageMapID }, WTF::move(area));
+        return;
+    }
+
     auto& slot = m_storageAreasByConnection.add({ &connection, storageMapID }, nullptr).iterator->value;
 
     // FIXME: This should be a message check.
@@ -774,7 +793,7 @@
     // FIXME: This should be a message check.
     ASSERT(&connection == sessionStorageNamespace->allowedConnection());
 
-    auto storageArea = sessionStorageNamespace->getOrCreateStorageArea(securityOriginData.securityOrigin());
+    auto storageArea = sessionStorageNamespace->getOrCreateStorageArea(securityOriginData.securityOrigin().ptr());
     storageArea->addListener(connection, storageMapID);
 
     slot = WTF::move(storageArea);
@@ -793,6 +812,10 @@
         return;
     }
 
+    // Don't remove session storage maps. The web process may reconnect and expect the data to still be around.
+    if (it->value->isSessionStorage())
+        return;
+
     it->value->removeListener(connection, storageMapID);
     m_storageAreasByConnection.remove(connectionAndStorageMapIDPair);
 }

Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageAreaImpl.cpp (184929 => 184930)


--- trunk/Source/WebKit2/WebProcess/Storage/StorageAreaImpl.cpp	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageAreaImpl.cpp	2015-05-28 00:30:59 UTC (rev 184930)
@@ -127,4 +127,9 @@
     ASSERT_NOT_REACHED();
 }
 
+WebCore::SecurityOrigin& StorageAreaImpl::securityOrigin()
+{
+    return m_storageAreaMap->securityOrigin();
+}
+
 } // namespace WebKit

Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageAreaImpl.h (184929 => 184930)


--- trunk/Source/WebKit2/WebProcess/Storage/StorageAreaImpl.h	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageAreaImpl.h	2015-05-28 00:30:59 UTC (rev 184930)
@@ -31,6 +31,10 @@
 #include <wtf/HashCountedSet.h>
 #include <wtf/HashMap.h>
 
+namespace WebCore {
+class SecurityOrigin;
+}
+
 namespace WebKit {
 
 class StorageAreaMap;
@@ -59,6 +63,7 @@
     virtual void incrementAccessCount() override;
     virtual void decrementAccessCount() override;
     virtual void closeDatabaseIfIdle() override;
+    WebCore::SecurityOrigin& securityOrigin() override;
 
     uint64_t m_storageAreaID;
     RefPtr<StorageAreaMap> m_storageAreaMap;

Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.cpp (184929 => 184930)


--- trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.cpp	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.cpp	2015-05-28 00:30:59 UTC (rev 184930)
@@ -59,7 +59,8 @@
 }
 
 StorageAreaMap::StorageAreaMap(StorageNamespaceImpl* storageNamespace, Ref<WebCore::SecurityOrigin>&& securityOrigin)
-    : m_storageMapID(generateStorageMapID())
+    : m_storageNamespace(*storageNamespace)
+    , m_storageMapID(generateStorageMapID())
     , m_storageType(storageNamespace->storageType())
     , m_storageNamespaceID(storageNamespace->storageNamespaceID())
     , m_quotaInBytes(storageNamespace->quotaInBytes())
@@ -89,6 +90,8 @@
 {
     WebProcess::singleton().parentProcessConnection()->send(Messages::StorageManager::DestroyStorageMap(m_storageMapID), 0);
     WebProcess::singleton().removeMessageReceiver(Messages::StorageAreaMap::messageReceiverName(), m_storageMapID);
+
+    m_storageNamespace->didDestroyStorageAreaMap(*this);
 }
 
 unsigned StorageAreaMap::length()

Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.h (184929 => 184930)


--- trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.h	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.h	2015-05-28 00:30:59 UTC (rev 184930)
@@ -59,6 +59,8 @@
     void clear(WebCore::Frame* sourceFrame, StorageAreaImpl* sourceArea);
     bool contains(const String& key);
 
+    WebCore::SecurityOrigin& securityOrigin() { return m_securityOrigin.get(); }
+
 private:
     StorageAreaMap(StorageNamespaceImpl*, Ref<WebCore::SecurityOrigin>&&);
 
@@ -82,6 +84,8 @@
     void dispatchSessionStorageEvent(uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString);
     void dispatchLocalStorageEvent(uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString);
 
+    Ref<StorageNamespaceImpl> m_storageNamespace;
+
     uint64_t m_storageMapID;
 
     WebCore::StorageType m_storageType;

Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageNamespaceImpl.cpp (184929 => 184930)


--- trunk/Source/WebKit2/WebProcess/Storage/StorageNamespaceImpl.cpp	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageNamespaceImpl.cpp	2015-05-28 00:30:59 UTC (rev 184930)
@@ -67,13 +67,23 @@
 {
 }
 
+void StorageNamespaceImpl::didDestroyStorageAreaMap(StorageAreaMap& map)
+{
+    m_storageAreaMaps.remove(&map.securityOrigin());
+}
+
 PassRefPtr<StorageArea> StorageNamespaceImpl::storageArea(PassRefPtr<SecurityOrigin> securityOrigin)
 {
+    RefPtr<StorageAreaMap> map;
+
     auto& slot = m_storageAreaMaps.add(securityOrigin.get(), nullptr).iterator->value;
-    if (!slot)
-        slot = StorageAreaMap::create(this, *securityOrigin);
+    if (!slot) {
+        map = StorageAreaMap::create(this, *securityOrigin);
+        slot = map.get();
+    } else
+        map = slot;
 
-    return StorageAreaImpl::create(slot);
+    return StorageAreaImpl::create(WTF::move(map));
 }
 
 PassRefPtr<StorageNamespace> StorageNamespaceImpl::copy(Page* newPage)

Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageNamespaceImpl.h (184929 => 184930)


--- trunk/Source/WebKit2/WebProcess/Storage/StorageNamespaceImpl.h	2015-05-28 00:25:52 UTC (rev 184929)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageNamespaceImpl.h	2015-05-28 00:30:59 UTC (rev 184930)
@@ -49,6 +49,8 @@
     WebCore::SecurityOrigin* topLevelOrigin() const { return m_topLevelOrigin.get(); }
     unsigned quotaInBytes() const { return m_quotaInBytes; }
 
+    void didDestroyStorageAreaMap(StorageAreaMap&);
+
 private:
     explicit StorageNamespaceImpl(WebCore::StorageType, uint64_t storageNamespaceID, WebCore::SecurityOrigin* topLevelOrigin, unsigned quotaInBytes);
 
@@ -63,7 +65,7 @@
 
     const unsigned m_quotaInBytes;
 
-    HashMap<RefPtr<WebCore::SecurityOrigin>, RefPtr<StorageAreaMap>> m_storageAreaMaps;
+    HashMap<RefPtr<WebCore::SecurityOrigin>, StorageAreaMap*> m_storageAreaMaps;
 };
 
 } // namespace WebKit

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to