Crash under WebKit::WebBackForwardCache::removeEntry()

https://bugs.webkit.org/show_bug.cgi?id=258698
rdar://111524465

Reviewed by Ryosuke Niwa.

In WebBackForwardCache::removeEntry(), the call to `item.setBackForwardCacheEntry(nullptr)`
may cause the `item` to get destroyed. However, we were using `item` on the next line for
logging purpose. To fix the bug, I am moving the logging before the setBackForwardCacheEntry()
call.

for hardening purposes, I am also updating m_itemsWithCachedPage to contain WeakPtrs instead
of raw pointers.

* Source/WebKit/Shared/WebBackForwardListItem.h:
* Source/WebKit/UIProcess/WebBackForwardCache.cpp:
(WebKit::WebBackForwardCache::removeEntry):
(WebKit::WebBackForwardCache::removeEntriesMatching):
(WebKit::WebBackForwardCache::clear):
* Source/WebKit/UIProcess/WebBackForwardCache.h:

Originally-landed-as: 259548.865@safari-7615-branch (cb256ae). rdar://111524465
Canonical link: https://commits.webkit.org/266453@main

Source/WebKit/Shared/WebBackForwardListItem.h

@@ -47,7 +47,7 @@ class SuspendedPageProxy;
 class WebBackForwardCache;
 class WebBackForwardCacheEntry;
 
-class WebBackForwardListItem : public API::ObjectImpl<API::Object::Type::BackForwardListItem> {
+class WebBackForwardListItem : public API::ObjectImpl<API::Object::Type::BackForwardListItem>, public CanMakeWeakPtr<WebBackForwardListItem> {
 public:
     static Ref<WebBackForwardListItem> create(BackForwardListItemState&&, WebPageProxyIdentifier);
     virtual ~WebBackForwardListItem();

Source/WebKit/UIProcess/WebBackForwardCache.cpp

@@ -100,8 +100,8 @@ void WebBackForwardCache::removeEntry(WebBackForwardListItem& item)
 {
     ASSERT(m_itemsWithCachedPage.contains(&item));
     m_itemsWithCachedPage.removeFirst(&item);
-    item.setBackForwardCacheEntry(nullptr);
     RELEASE_LOG(BackForwardCache, "WebBackForwardCache::removeEntry: item=%s, size=%u/%u", item.itemID().toString().utf8().data(), size(), capacity());
+    item.setBackForwardCacheEntry(nullptr); // item may be dead after this call.
 }
 
 void WebBackForwardCache::removeEntry(SuspendedPageProxy& suspendedPage)
@@ -157,7 +157,7 @@ void WebBackForwardCache::removeEntriesForPageAndProcess(WebPageProxy& page, Web
 void WebBackForwardCache::removeEntriesMatching(const Function<bool(WebBackForwardListItem&)>& matches)
 {
     Vector<Ref<WebBackForwardListItem>> itemsWithEntriesToClear;
-    m_itemsWithCachedPage.removeAllMatching([&](auto* item) {
+    m_itemsWithCachedPage.removeAllMatching([&](auto& item) {
         if (matches(*item)) {
             itemsWithEntriesToClear.append(*item);
             return true;
@@ -173,7 +173,7 @@ void WebBackForwardCache::clear()
 {
     RELEASE_LOG(BackForwardCache, "WebBackForwardCache::clear");
     auto itemsWithCachedPage = WTFMove(m_itemsWithCachedPage);
-    for (auto* item : itemsWithCachedPage)
+    for (auto& item : itemsWithCachedPage)
         item->setBackForwardCacheEntry(nullptr);
 }
 

Source/WebKit/UIProcess/WebBackForwardCache.h

@@ -69,7 +69,8 @@ class WebBackForwardCache {
 
     WebProcessPool& m_processPool;
     unsigned m_capacity { 0 };
-    Vector<WebBackForwardListItem*, 2> m_itemsWithCachedPage;
+    // Items cannot be null, we're using WeakPtr for hardening.
+    Vector<WeakPtr<WebBackForwardListItem>, 2> m_itemsWithCachedPage;
 };
 
 } // namespace WebKit