Modified: trunk/Source/WebKit2/UIProcess/WebBackForwardList.cpp (121353 => 121354)
--- trunk/Source/WebKit2/UIProcess/WebBackForwardList.cpp 2012-06-27 18:28:20 UTC (rev 121353)
+++ trunk/Source/WebKit2/UIProcess/WebBackForwardList.cpp 2012-06-27 18:28:52 UTC (rev 121354)
@@ -43,17 +43,28 @@
WebBackForwardList::~WebBackForwardList()
{
+ // A WebBackForwardList should never be destroyed unless it's associated page has been closed or is invalid.
+ ASSERT((!m_page && !m_hasCurrentIndex) || !m_page->isValid());
}
void WebBackForwardList::pageClosed()
{
+ // We should have always started out with an m_page and we should never close the page twice.
+ ASSERT(m_page);
+
if (m_page) {
size_t size = m_entries.size();
- for (size_t i = 0; i < size; ++i)
+ for (size_t i = 0; i < size; ++i) {
+ ASSERT(m_entries[i]);
+ if (!m_entries[i])
+ continue;
m_page->backForwardRemovedItem(m_entries[i]->itemID());
+ }
}
m_page = 0;
+ m_entries.clear();
+ m_hasCurrentIndex = false;
}
void WebBackForwardList::addItem(WebBackForwardListItem* newItem)
@@ -81,15 +92,22 @@
m_page->backForwardRemovedItem(m_entries[0]->itemID());
removedItems.append(m_entries[0].release());
m_entries.remove(0);
+
if (m_entries.isEmpty())
m_hasCurrentIndex = false;
else
m_currentIndex--;
}
} else {
- // If we have no current item index, we should have no other entries before adding this new item.
+ // If we have no current item index we should also not have any entries.
+ ASSERT(m_entries.isEmpty());
+
+ // But just in case it does happen in practice we'll get back in to a consistent state now before adding the new item.
size_t size = m_entries.size();
for (size_t i = 0; i < size; ++i) {
+ ASSERT(m_entries[i]);
+ if (!m_entries[i])
+ continue;
m_page->backForwardRemovedItem(m_entries[i]->itemID());
removedItems.append(m_entries[i].release());
}
@@ -97,6 +115,7 @@
}
if (!m_hasCurrentIndex) {
+ ASSERT(m_entries.isEmpty());
m_currentIndex = 0;
m_hasCurrentIndex = true;
} else
@@ -115,7 +134,7 @@
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
- if (!m_entries.size() || !item)
+ if (!m_entries.size() || !item || !m_page || !m_hasCurrentIndex)
return;
unsigned index = 0;
@@ -125,8 +144,7 @@
}
if (index < m_entries.size()) {
m_currentIndex = index;
- if (m_page)
- m_page->didChangeBackForwardList(0, 0);
+ m_page->didChangeBackForwardList(0, 0);
}
}
@@ -134,28 +152,28 @@
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
- return m_hasCurrentIndex ? m_entries[m_currentIndex].get() : 0;
+ return m_page && m_hasCurrentIndex ? m_entries[m_currentIndex].get() : 0;
}
WebBackForwardListItem* WebBackForwardList::backItem()
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
- return m_hasCurrentIndex && m_currentIndex ? m_entries[m_currentIndex - 1].get() : 0;
+ return m_page && m_hasCurrentIndex && m_currentIndex ? m_entries[m_currentIndex - 1].get() : 0;
}
WebBackForwardListItem* WebBackForwardList::forwardItem()
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
- return m_hasCurrentIndex && m_entries.size() && m_currentIndex < m_entries.size() - 1 ? m_entries[m_currentIndex + 1].get() : 0;
+ return m_page && m_hasCurrentIndex && m_entries.size() && m_currentIndex < m_entries.size() - 1 ? m_entries[m_currentIndex + 1].get() : 0;
}
WebBackForwardListItem* WebBackForwardList::itemAtIndex(int index)
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
- if (!m_hasCurrentIndex)
+ if (!m_hasCurrentIndex || !m_page)
return 0;
// Do range checks without doing math on index to avoid overflow.
@@ -172,20 +190,23 @@
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
- return !m_hasCurrentIndex ? 0 : m_currentIndex;
+ return m_page && m_hasCurrentIndex ? m_currentIndex : 0;
}
int WebBackForwardList::forwardListCount()
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
- return !m_hasCurrentIndex ? 0 : static_cast<int>(m_entries.size()) - (m_currentIndex + 1);
+ return m_page && m_hasCurrentIndex ? m_entries.size() - (m_currentIndex + 1) : 0;
}
PassRefPtr<ImmutableArray> WebBackForwardList::backListAsImmutableArrayWithLimit(unsigned limit)
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
+ if (!m_page || !m_hasCurrentIndex)
+ return ImmutableArray::create();
+
unsigned backListSize = static_cast<unsigned>(backListCount());
unsigned size = std::min(backListSize, limit);
if (!size)
@@ -195,8 +216,10 @@
vector.reserveInitialCapacity(size);
ASSERT(backListSize >= size);
- for (unsigned i = backListSize - size; i < backListSize; ++i)
+ for (unsigned i = backListSize - size; i < backListSize; ++i) {
+ ASSERT(m_entries[i]);
vector.uncheckedAppend(m_entries[i].get());
+ }
return ImmutableArray::adopt(vector);
}
@@ -205,6 +228,9 @@
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
+ if (!m_page || !m_hasCurrentIndex)
+ return ImmutableArray::create();
+
unsigned size = std::min(static_cast<unsigned>(forwardListCount()), limit);
if (!size)
return ImmutableArray::create();
@@ -214,8 +240,10 @@
unsigned last = m_currentIndex + size;
ASSERT(last < m_entries.size());
- for (unsigned i = m_currentIndex + 1; i <= last; ++i)
+ for (unsigned i = m_currentIndex + 1; i <= last; ++i) {
+ ASSERT(m_entries[i]);
vector.uncheckedAppend(m_entries[i].get());
+ }
return ImmutableArray::adopt(vector);
}
@@ -225,22 +253,42 @@
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
size_t size = m_entries.size();
- if (size <= 1)
+ if (!m_page || size <= 1)
return;
RefPtr<WebBackForwardListItem> currentItem = this->currentItem();
+ Vector<RefPtr<APIObject> > removedItems;
- if (m_page) {
+ if (!currentItem) {
+ // We should only ever have no current item if we also have no current item index.
+ ASSERT(!m_hasCurrentIndex);
+
+ // But just in case it does happen in practice we should get back into a consistent state now.
for (size_t i = 0; i < size; ++i) {
- if (m_entries[i] != currentItem)
- m_page->backForwardRemovedItem(m_entries[i]->itemID());
+ ASSERT(m_entries[i]);
+ if (!m_entries[i])
+ continue;
+
+ m_page->backForwardRemovedItem(m_entries[i]->itemID());
+ removedItems.append(m_entries[i].release());
}
+
+ m_entries.clear();
+ m_hasCurrentIndex = false;
+ m_page->didChangeBackForwardList(0, &removedItems);
+
+ return;
}
- Vector<RefPtr<APIObject> > removedItems;
- removedItems.reserveCapacity(m_entries.size() - 1);
- for (size_t i = 0; i < m_entries.size(); ++i) {
- if (i != m_currentIndex)
+ for (size_t i = 0; i < size; ++i) {
+ ASSERT(m_entries[i]);
+ if (m_entries[i] && m_entries[i] != currentItem)
+ m_page->backForwardRemovedItem(m_entries[i]->itemID());
+ }
+
+ removedItems.reserveCapacity(size - 1);
+ for (size_t i = 0; i < size; ++i) {
+ if (i != m_currentIndex && m_hasCurrentIndex && m_entries[i])
removedItems.append(m_entries[i].release());
}
@@ -254,8 +302,7 @@
m_hasCurrentIndex = false;
}
- if (m_page)
- m_page->didChangeBackForwardList(0, &removedItems);
+ m_page->didChangeBackForwardList(0, &removedItems);
}
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/cf/WebBackForwardListCF.cpp (121353 => 121354)
--- trunk/Source/WebKit2/UIProcess/cf/WebBackForwardListCF.cpp 2012-06-27 18:28:20 UTC (rev 121353)
+++ trunk/Source/WebKit2/UIProcess/cf/WebBackForwardListCF.cpp 2012-06-27 18:28:52 UTC (rev 121354)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010, 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2010, 2011, 2012 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -44,6 +44,10 @@
return uniqueHistoryItemID;
}
+static CFIndex currentVersion = 1;
+DEFINE_STATIC_GETTER(CFNumberRef, SessionHistoryCurrentVersion, (CFNumberCreate(0, kCFNumberCFIndexType, ¤tVersion)));
+
+DEFINE_STATIC_GETTER(CFStringRef, SessionHistoryVersionKey, (CFSTR("SessionHistoryVersion")));
DEFINE_STATIC_GETTER(CFStringRef, SessionHistoryCurrentIndexKey, (CFSTR("SessionHistoryCurrentIndex")));
DEFINE_STATIC_GETTER(CFStringRef, SessionHistoryEntriesKey, (CFSTR("SessionHistoryEntries")));
DEFINE_STATIC_GETTER(CFStringRef, SessionHistoryEntryTitleKey, (CFSTR("SessionHistoryEntryTitle")));
@@ -51,23 +55,41 @@
DEFINE_STATIC_GETTER(CFStringRef, SessionHistoryEntryOriginalURLKey, (CFSTR("SessionHistoryEntryOriginalURL")));
DEFINE_STATIC_GETTER(CFStringRef, SessionHistoryEntryDataKey, (CFSTR("SessionHistoryEntryData")));
+static bool extractBackForwardListEntriesFromArray(CFArrayRef, BackForwardListItemVector&);
+
+static CFDictionaryRef createEmptySessionHistoryDictionary()
+{
+ static const void* keys[1] = { SessionHistoryVersionKey() };
+ static const void* values[1] = { SessionHistoryCurrentVersion() };
+
+ return CFDictionaryCreate(0, keys, values, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+}
+
CFDictionaryRef WebBackForwardList::createCFDictionaryRepresentation(WebPageProxy::WebPageProxySessionStateFilterCallback filter, void* context) const
{
ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
+ if (!m_hasCurrentIndex) {
+ // We represent having no current index by writing out an empty dictionary (besides the version).
+ return createEmptySessionHistoryDictionary();
+ }
+
RetainPtr<CFMutableArrayRef> entries(AdoptCF, CFArrayCreateMutable(0, m_entries.size(), &kCFTypeArrayCallBacks));
// We may need to update the current index to account for entries that are filtered by the callback.
CFIndex currentIndex = m_currentIndex;
+ bool hasCurrentIndex = true;
for (size_t i = 0; i < m_entries.size(); ++i) {
// If we somehow ended up with a null entry then we should consider the data invalid and not save session history at all.
ASSERT(m_entries[i]);
- if (!m_entries[i])
+ if (!m_entries[i]) {
+ LOG(SessionState, "WebBackForwardList contained a null entry at index %lu", i);
return 0;
+ }
if (filter && !filter(toAPI(m_page), WKPageGetSessionHistoryURLValueType(), toURLRef(m_entries[i]->originalURL().impl()), context)) {
- if (i <= static_cast<size_t>(m_currentIndex))
+ if (i <= m_currentIndex)
currentIndex--;
continue;
}
@@ -87,26 +109,62 @@
CFArrayAppendValue(entries.get(), entryDictionary.get());
}
- // If all items before and including the current item were filtered then currentIndex will be -1.
- // Assuming we didn't start out with NoCurrentItemIndex, we should store "current" to point at the first item.
- if (currentIndex == -1 && m_hasCurrentIndex && CFArrayGetCount(entries.get()))
- currentIndex = 0;
+ ASSERT(currentIndex == -1 || (currentIndex > -1 && currentIndex < CFArrayGetCount(entries.get())));
+ if (currentIndex < -1 || currentIndex >= CFArrayGetCount(entries.get())) {
+ LOG(SessionState, "Filtering entries to be saved resulted in an inconsistent state that we cannot represent");
+ return 0;
+ }
- // FIXME: We're relying on currentIndex == -1 to mean the exact same thing as NoCurrentItemIndex (UINT_MAX) in unsigned form.
- // That seems implicit and fragile and we should find a better way of representing the NoCurrentItemIndex case.
- if (!m_hasCurrentIndex || !CFArrayGetCount(entries.get()))
- currentIndex = -1;
+ // If we have an index and all items before and including the current item were filtered then currentIndex will be -1.
+ // In this case the new current index should point at the first item.
+ // It's also possible that all items were filtered so we should represent not having a current index.
+ if (currentIndex == -1) {
+ if (CFArrayGetCount(entries.get()))
+ currentIndex = 0;
+ else
+ hasCurrentIndex = false;
+ }
- RetainPtr<CFNumberRef> currentIndexNumber(AdoptCF, CFNumberCreate(0, kCFNumberCFIndexType, ¤tIndex));
+ if (hasCurrentIndex) {
+ RetainPtr<CFNumberRef> currentIndexNumber(AdoptCF, CFNumberCreate(0, kCFNumberCFIndexType, ¤tIndex));
+ const void* keys[3] = { SessionHistoryVersionKey(), SessionHistoryCurrentIndexKey(), SessionHistoryEntriesKey() };
+ const void* values[3] = { SessionHistoryCurrentVersion(), currentIndexNumber.get(), entries.get() };
+
+ return CFDictionaryCreate(0, keys, values, 3, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+ }
- const void* keys[2] = { SessionHistoryCurrentIndexKey(), SessionHistoryEntriesKey() };
- const void* values[2] = { currentIndexNumber.get(), entries.get() };
-
- return CFDictionaryCreate(0, keys, values, 2, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+ // We represent having no current index by writing out an empty dictionary (besides the version).
+ return createEmptySessionHistoryDictionary();
}
bool WebBackForwardList::restoreFromCFDictionaryRepresentation(CFDictionaryRef dictionary)
{
+ CFNumberRef cfVersion = (CFNumberRef)CFDictionaryGetValue(dictionary, SessionHistoryVersionKey());
+ if (!cfVersion) {
+ // v0 session history dictionaries did not contain versioning
+ return restoreFromV0CFDictionaryRepresentation(dictionary);
+ }
+
+ if (CFGetTypeID(cfVersion) != CFNumberGetTypeID()) {
+ LOG(SessionState, "WebBackForwardList dictionary representation contains a version that is not a number");
+ return false;
+ }
+
+ CFIndex version;
+ if (!CFNumberGetValue(cfVersion, kCFNumberCFIndexType, &version)) {
+ LOG(SessionState, "WebBackForwardList dictionary representation does not have a correctly typed current version");
+ return false;
+ }
+
+ if (version == 1)
+ return restoreFromV1CFDictionaryRepresentation(dictionary);
+
+ LOG(SessionState, "WebBackForwardList dictionary representation has an invalid current version (%ld)", version);
+ return false;
+}
+
+bool WebBackForwardList::restoreFromV0CFDictionaryRepresentation(CFDictionaryRef dictionary)
+{
CFNumberRef cfIndex = (CFNumberRef)CFDictionaryGetValue(dictionary, SessionHistoryCurrentIndexKey());
if (!cfIndex || CFGetTypeID(cfIndex) != CFNumberGetTypeID()) {
LOG(SessionState, "WebBackForwardList dictionary representation does not have a valid current index");
@@ -115,12 +173,12 @@
CFIndex currentCFIndex;
if (!CFNumberGetValue(cfIndex, kCFNumberCFIndexType, ¤tCFIndex)) {
- LOG(SessionState, "WebBackForwardList dictionary representation does not have a valid integer current index");
+ LOG(SessionState, "WebBackForwardList dictionary representation does not have a correctly typed current index");
return false;
}
if (currentCFIndex < -1) {
- LOG(SessionState, "WebBackForwardList dictionary representation contains a negative current index that is bogus (%ld)", currentCFIndex);
+ LOG(SessionState, "WebBackForwardList dictionary representation contains an unexpected negative current index (%ld)", currentCFIndex);
return false;
}
@@ -131,23 +189,94 @@
}
CFIndex size = CFArrayGetCount(cfEntries);
- if (currentCFIndex < -1 || currentCFIndex >= size ) {
+ if (size < 0 || currentCFIndex >= size) {
LOG(SessionState, "WebBackForwardList dictionary representation contains an invalid current index (%ld) for the number of entries (%ld)", currentCFIndex, size);
return false;
}
- // FIXME: We're relying on currentIndex == -1 to mean the exact same thing as NoCurrentItemIndex (UINT_MAX) in unsigned form.
- // That seems implicit and fragile and we should find a better way of representing the NoCurrentItemIndex case.
- bool hasCurrentIndex = currentCFIndex > -1;
- unsigned currentIndex = hasCurrentIndex ? static_cast<unsigned>(currentCFIndex) : 0;
+ // Version 0 session history relied on currentIndex == -1 to represent the same thing as not having a current index.
+ bool hasCurrentIndex = currentCFIndex != -1;
if (!hasCurrentIndex && size) {
- LOG(SessionState, "WebBackForwardList dictionary representation says there is no current item index, but there is a list of %ld entries - this is bogus", size);
+ LOG(SessionState, "WebBackForwardList dictionary representation says there is no current index, but there is a list of %ld entries", size);
return false;
}
- BackForwardListItemVector newEntries;
- newEntries.reserveCapacity(size);
+ BackForwardListItemVector entries;
+ if (!extractBackForwardListEntriesFromArray(cfEntries, entries)) {
+ // extractBackForwardListEntriesFromArray has already logged the appropriate error message.
+ return false;
+ }
+
+ ASSERT(entries.size() == static_cast<unsigned>(size));
+
+ m_hasCurrentIndex = hasCurrentIndex;
+ m_currentIndex = m_hasCurrentIndex ? static_cast<uint32_t>(currentCFIndex) : 0;
+ m_entries = entries;
+
+ return true;
+}
+
+bool WebBackForwardList::restoreFromV1CFDictionaryRepresentation(CFDictionaryRef dictionary)
+{
+ CFNumberRef cfIndex = (CFNumberRef)CFDictionaryGetValue(dictionary, SessionHistoryCurrentIndexKey());
+ if (!cfIndex) {
+ // No current index means the dictionary represents an empty session.
+ m_hasCurrentIndex = false;
+ m_currentIndex = 0;
+ m_entries.clear();
+
+ return true;
+ }
+
+ if (CFGetTypeID(cfIndex) != CFNumberGetTypeID()) {
+ LOG(SessionState, "WebBackForwardList dictionary representation does not have a valid current index");
+ return false;
+ }
+
+ CFIndex currentCFIndex;
+ if (!CFNumberGetValue(cfIndex, kCFNumberCFIndexType, ¤tCFIndex)) {
+ LOG(SessionState, "WebBackForwardList dictionary representation does not have a correctly typed current index");
+ return false;
+ }
+
+ if (currentCFIndex < 0) {
+ LOG(SessionState, "WebBackForwardList dictionary representation contains an unexpected negative current index (%ld)", currentCFIndex);
+ return false;
+ }
+
+ CFArrayRef cfEntries = (CFArrayRef)CFDictionaryGetValue(dictionary, SessionHistoryEntriesKey());
+ if (!cfEntries || CFGetTypeID(cfEntries) != CFArrayGetTypeID()) {
+ LOG(SessionState, "WebBackForwardList dictionary representation does not have a valid list of entries");
+ return false;
+ }
+
+ CFIndex size = CFArrayGetCount(cfEntries);
+ if (currentCFIndex >= size) {
+ LOG(SessionState, "WebBackForwardList dictionary representation contains an invalid current index (%ld) for the number of entries (%ld)", currentCFIndex, size);
+ return false;
+ }
+
+ BackForwardListItemVector entries;
+ if (!extractBackForwardListEntriesFromArray(cfEntries, entries)) {
+ // extractBackForwardListEntriesFromArray has already logged the appropriate error message.
+ return false;
+ }
+
+ ASSERT(entries.size() == static_cast<unsigned>(size));
+
+ m_hasCurrentIndex = true;
+ m_currentIndex = static_cast<uint32_t>(currentCFIndex);
+ m_entries = entries;
+
+ return true;
+}
+
+static bool extractBackForwardListEntriesFromArray(CFArrayRef cfEntries, BackForwardListItemVector& entries)
+{
+ CFIndex size = CFArrayGetCount(cfEntries);
+
+ entries.reserveCapacity(size);
for (CFIndex i = 0; i < size; ++i) {
CFDictionaryRef entryDictionary = (CFDictionaryRef)CFArrayGetValueAtIndex(cfEntries, i);
if (!entryDictionary || CFGetTypeID(entryDictionary) != CFDictionaryGetTypeID()) {
@@ -179,12 +308,8 @@
return false;
}
- newEntries.append(WebBackForwardListItem::create(originalURL, entryURL, entryTitle, CFDataGetBytePtr(backForwardData), CFDataGetLength(backForwardData), generateWebBackForwardItemID()));
+ entries.append(WebBackForwardListItem::create(originalURL, entryURL, entryTitle, CFDataGetBytePtr(backForwardData), CFDataGetLength(backForwardData), generateWebBackForwardItemID()));
}
-
- m_entries = newEntries;
- m_hasCurrentIndex = hasCurrentIndex;
- m_currentIndex = currentIndex;
return true;
}
