Diff
Modified: branches/safari-603-branch/Source/WebCore/ChangeLog (211709 => 211710)
--- branches/safari-603-branch/Source/WebCore/ChangeLog 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebCore/ChangeLog 2017-02-06 06:17:32 UTC (rev 211710)
@@ -1,5 +1,44 @@
2017-02-05 Matthew Hanson <matthew_han...@apple.com>
+ Merge r211569. rdar://problem/30229990
+
+ 2017-02-02 Chris Dumez <cdu...@apple.com>
+
+ [Crash] com.apple.WebKit.WebContent at WebKit: WebKit::WebPage::fromCorePage()
+ https://bugs.webkit.org/show_bug.cgi?id=167738
+ <rdar://problem/30229990>
+
+ Reviewed by Andreas Kling.
+
+ Upon destruction of a Page, we destroy the BackForwardClient, which is supposed
+ to keep track of HistoryItems associated to this particular page and remove them
+ from the PageCache. Given the crash trace, the issue seems to be that some
+ HistoryItems associated with the Page sometimes linger in the PageCache *after*
+ the Page has been destroyed, which leads to crashes later on when pruning the
+ PageCache.
+
+ In order to make the process more robust, this patch refactors the code so that
+ the Page is now in charge of removing all its associated HistoryItems from the
+ PageCache instead of relying on the BackForwardClient. Also, instead of having
+ the Page keep track of which HistoryItems are associated with it (which is
+ error prone), we now scan all PageCache entries instead to find which ones are
+ associated with the Page. While this is in theory slower, this is much safer
+ and in practice not an issue because the PageCache usually has 3-5 entries.
+
+ No new tests, could not reproduce.
+
+ * history/CachedPage.cpp:
+ (WebCore::CachedPage::CachedPage):
+ * history/CachedPage.h:
+ (WebCore::CachedPage::page):
+ * history/PageCache.cpp:
+ (WebCore::PageCache::removeAllItemsForPage):
+ * history/PageCache.h:
+ * page/Page.cpp:
+ (WebCore::Page::~Page):
+
+2017-02-05 Matthew Hanson <matthew_han...@apple.com>
+
Merge r211551. rdar://problem/26685576
2017-02-02 Yongjun Zhang <yongjun_zh...@apple.com>
Modified: branches/safari-603-branch/Source/WebCore/history/CachedPage.cpp (211709 => 211710)
--- branches/safari-603-branch/Source/WebCore/history/CachedPage.cpp 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebCore/history/CachedPage.cpp 2017-02-06 06:17:32 UTC (rev 211710)
@@ -50,7 +50,8 @@
DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, cachedPageCounter, ("CachedPage"));
CachedPage::CachedPage(Page& page)
- : m_expirationTime(monotonicallyIncreasingTime() + page.settings().backForwardCacheExpirationInterval())
+ : m_page(page)
+ , m_expirationTime(monotonicallyIncreasingTime() + page.settings().backForwardCacheExpirationInterval())
, m_cachedMainFrame(std::make_unique<CachedFrame>(page.mainFrame()))
{
#ifndef NDEBUG
Modified: branches/safari-603-branch/Source/WebCore/history/CachedPage.h (211709 => 211710)
--- branches/safari-603-branch/Source/WebCore/history/CachedPage.h 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebCore/history/CachedPage.h 2017-02-06 06:17:32 UTC (rev 211710)
@@ -42,6 +42,7 @@
void restore(Page&);
void clear();
+ Page& page() const { return m_page; }
Document* document() const { return m_cachedMainFrame->document(); }
DocumentLoader* documentLoader() const { return m_cachedMainFrame->documentLoader(); }
@@ -58,6 +59,7 @@
void markForContentsSizeChanged() { m_needsUpdateContentsSize = true; }
private:
+ Page& m_page;
double m_expirationTime;
std::unique_ptr<CachedFrame> m_cachedMainFrame;
#if ENABLE(VIDEO_TRACK)
Modified: branches/safari-603-branch/Source/WebCore/history/PageCache.cpp (211709 => 211710)
--- branches/safari-603-branch/Source/WebCore/history/PageCache.cpp 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebCore/history/PageCache.cpp 2017-02-06 06:17:32 UTC (rev 211710)
@@ -439,6 +439,19 @@
return cachedPage;
}
+void PageCache::removeAllItemsForPage(Page& page)
+{
+ for (auto it = m_items.begin(); it != m_items.end();) {
+ // Increment iterator first so it stays invalid after the removal.
+ auto current = it;
+ ++it;
+ if (&(*current)->m_cachedPage->page() == &page) {
+ (*current)->m_cachedPage = nullptr;
+ m_items.remove(current);
+ }
+ }
+}
+
CachedPage* PageCache::get(HistoryItem& item, Page* page)
{
CachedPage* cachedPage = item.m_cachedPage.get();
Modified: branches/safari-603-branch/Source/WebCore/history/PageCache.h (211709 => 211710)
--- branches/safari-603-branch/Source/WebCore/history/PageCache.h 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebCore/history/PageCache.h 2017-02-06 06:17:32 UTC (rev 211710)
@@ -57,6 +57,8 @@
CachedPage* get(HistoryItem&, Page*);
std::unique_ptr<CachedPage> take(HistoryItem&, Page*);
+ void removeAllItemsForPage(Page&);
+
unsigned pageCount() const { return m_items.size(); }
WEBCORE_EXPORT unsigned frameCount() const;
Modified: branches/safari-603-branch/Source/WebCore/page/Page.cpp (211709 => 211710)
--- branches/safari-603-branch/Source/WebCore/page/Page.cpp 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebCore/page/Page.cpp 2017-02-06 06:17:32 UTC (rev 211710)
@@ -318,6 +318,7 @@
m_scrollingCoordinator->pageDestroyed();
backForward().close();
+ PageCache::singleton().removeAllItemsForPage(*this);
#ifndef NDEBUG
pageCounter.decrement();
Modified: branches/safari-603-branch/Source/WebKit/mac/ChangeLog (211709 => 211710)
--- branches/safari-603-branch/Source/WebKit/mac/ChangeLog 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebKit/mac/ChangeLog 2017-02-06 06:17:32 UTC (rev 211710)
@@ -1,5 +1,23 @@
2017-02-05 Matthew Hanson <matthew_han...@apple.com>
+ Merge r211569. rdar://problem/30229990
+
+ 2017-02-02 Chris Dumez <cdu...@apple.com>
+
+ [Crash] com.apple.WebKit.WebContent at WebKit: WebKit::WebPage::fromCorePage()
+ https://bugs.webkit.org/show_bug.cgi?id=167738
+ <rdar://problem/30229990>
+
+ Reviewed by Andreas Kling.
+
+ The BackForwardClient no longer needs to worry about removing HistoryItems
+ from the PageCache now that WebCore takes care of it.
+
+ * History/BackForwardList.mm:
+ (BackForwardList::close):
+
+2017-02-05 Matthew Hanson <matthew_han...@apple.com>
+
Merge r211551. rdar://problem/26685576
2017-02-02 Yongjun Zhang <yongjun_zh...@apple.com>
Modified: branches/safari-603-branch/Source/WebKit/mac/History/BackForwardList.mm (211709 => 211710)
--- branches/safari-603-branch/Source/WebKit/mac/History/BackForwardList.mm 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebKit/mac/History/BackForwardList.mm 2017-02-06 06:17:32 UTC (rev 211710)
@@ -225,8 +225,6 @@
void BackForwardList::close()
{
- for (auto& item : m_entries)
- PageCache::singleton().remove(item);
m_entries.clear();
m_entryHash.clear();
m_webView = nullptr;
Modified: branches/safari-603-branch/Source/WebKit/win/BackForwardList.cpp (211709 => 211710)
--- branches/safari-603-branch/Source/WebKit/win/BackForwardList.cpp 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebKit/win/BackForwardList.cpp 2017-02-06 06:17:32 UTC (rev 211710)
@@ -226,8 +226,6 @@
void BackForwardList::close()
{
- for (auto& item : m_entries)
- PageCache::singleton().remove(item);
m_entries.clear();
m_entryHash.clear();
m_closed = true;
Modified: branches/safari-603-branch/Source/WebKit/win/ChangeLog (211709 => 211710)
--- branches/safari-603-branch/Source/WebKit/win/ChangeLog 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebKit/win/ChangeLog 2017-02-06 06:17:32 UTC (rev 211710)
@@ -1,5 +1,23 @@
2017-02-05 Matthew Hanson <matthew_han...@apple.com>
+ Merge r211569. rdar://problem/30229990
+
+ 2017-02-02 Chris Dumez <cdu...@apple.com>
+
+ [Crash] com.apple.WebKit.WebContent at WebKit: WebKit::WebPage::fromCorePage()
+ https://bugs.webkit.org/show_bug.cgi?id=167738
+ <rdar://problem/30229990>
+
+ Reviewed by Andreas Kling.
+
+ The BackForwardClient no longer needs to worry about removing HistoryItems
+ from the PageCache now that WebCore takes care of it.
+
+ * BackForwardList.cpp:
+ (BackForwardList::close):
+
+2017-02-05 Matthew Hanson <matthew_han...@apple.com>
+
Merge r211584. rdar://problem/29994156
2017-02-02 Per Arne Vollan <pvol...@apple.com>
Modified: branches/safari-603-branch/Source/WebKit2/ChangeLog (211709 => 211710)
--- branches/safari-603-branch/Source/WebKit2/ChangeLog 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebKit2/ChangeLog 2017-02-06 06:17:32 UTC (rev 211710)
@@ -1,5 +1,26 @@
2017-02-05 Matthew Hanson <matthew_han...@apple.com>
+ Merge r211569. rdar://problem/30229990
+
+ 2017-02-02 Chris Dumez <cdu...@apple.com>
+
+ [Crash] com.apple.WebKit.WebContent at WebKit: WebKit::WebPage::fromCorePage()
+ https://bugs.webkit.org/show_bug.cgi?id=167738
+ <rdar://problem/30229990>
+
+ Reviewed by Andreas Kling.
+
+ The BackForwardClient no longer needs to worry about removing HistoryItems
+ from the PageCache now that WebCore takes care of it.
+
+ * WebProcess/WebPage/WebBackForwardListProxy.cpp:
+ (WebKit::WebBackForwardListProxy::addItemFromUIProcess):
+ (WebKit::WebBackForwardListProxy::addItem):
+ (WebKit::WebBackForwardListProxy::close):
+ * WebProcess/WebPage/WebBackForwardListProxy.h:
+
+2017-02-05 Matthew Hanson <matthew_han...@apple.com>
+
Merge r211565. rdar://problem/28896113
2017-02-01 Anders Carlsson <ander...@apple.com>
Modified: branches/safari-603-branch/Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.cpp (211709 => 211710)
--- branches/safari-603-branch/Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.cpp 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.cpp 2017-02-06 06:17:32 UTC (rev 211710)
@@ -100,8 +100,6 @@
ASSERT(!historyItemToIDMap().contains(item.ptr()));
ASSERT(!idToHistoryItemMap().contains(itemID));
- m_associatedItemIDs.add(itemID);
-
historyItemToIDMap().set<ItemAndPageID>(item.ptr(), { .itemID = itemID, .pageID = pageID });
idToHistoryItemMap().set(itemID, item.ptr());
}
@@ -154,8 +152,6 @@
ASSERT(!idToHistoryItemMap().contains(itemID));
- m_associatedItemIDs.add(itemID);
-
historyItemToIDMap().set<ItemAndPageID>(item.ptr(), { .itemID = itemID, .pageID = m_page->pageID() });
idToHistoryItemMap().set(itemID, item.ptr());
@@ -214,12 +210,6 @@
void WebBackForwardListProxy::close()
{
- for (auto& itemID : m_associatedItemIDs) {
- if (HistoryItem* item = itemForID(itemID))
- WebCore::PageCache::singleton().remove(*item);
- }
-
- m_associatedItemIDs.clear();
m_page = nullptr;
}
Modified: branches/safari-603-branch/Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.h (211709 => 211710)
--- branches/safari-603-branch/Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.h 2017-02-06 06:17:27 UTC (rev 211709)
+++ branches/safari-603-branch/Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.h 2017-02-06 06:17:32 UTC (rev 211710)
@@ -60,7 +60,6 @@
void close() override;
WebPage* m_page;
- HashSet<uint64_t> m_associatedItemIDs;
};
} // namespace WebKit
