Skip to site navigation (Press enter)

[webkit-changes] [118560] trunk/Source/WebKit2

beidson Fri, 25 May 2012 14:09:47 -0700

Title: [118560] trunk/Source/WebKit2

Revision: 118560
Author: [email protected]
Date: 2012-05-25 14:09:41 -0700 (Fri, 25 May 2012)

Log Message

https://bugs.webkit.org/show_bug.cgi?id=87418
WebBackForwardList should separate "has no current index" from the integer value of the current index


This patch also renames "m_current" to "m_currentIndex" for clarity and symmetry with
other variable names that reference "index",

It also removes the m_closed and m_enabled flags which were never actually used.

Reviewed by Darin Adler.

* UIProcess/WebBackForwardList.cpp:
(WebKit::WebBackForwardList::WebBackForwardList):
(WebKit::WebBackForwardList::addItem):
(WebKit::WebBackForwardList::goToItem):
(WebKit::WebBackForwardList::currentItem):
(WebKit::WebBackForwardList::backItem):
(WebKit::WebBackForwardList::forwardItem):
(WebKit::WebBackForwardList::itemAtIndex):
(WebKit::WebBackForwardList::backListCount):
(WebKit::WebBackForwardList::forwardListCount):
(WebKit::WebBackForwardList::backListAsImmutableArrayWithLimit):
(WebKit::WebBackForwardList::forwardListAsImmutableArrayWithLimit):
(WebKit::WebBackForwardList::clear):

* UIProcess/WebBackForwardList.h:
(WebKit::WebBackForwardList::currentIndex):
(WebBackForwardList):

* UIProcess/cf/WebBackForwardListCF.cpp:
(WebKit::WebBackForwardList::createCFDictionaryRepresentation):
(WebKit::WebBackForwardList::restoreFromCFDictionaryRepresentation):

Modified Paths

trunk/Source/WebKit2/ChangeLog
trunk/Source/WebKit2/UIProcess/WebBackForwardList.cpp
trunk/Source/WebKit2/UIProcess/WebBackForwardList.h
trunk/Source/WebKit2/UIProcess/cf/WebBackForwardListCF.cpp

Diff

Modified: trunk/Source/WebKit2/ChangeLog (118559 => 118560)


--- trunk/Source/WebKit2/ChangeLog	2012-05-25 21:07:28 UTC (rev 118559)
+++ trunk/Source/WebKit2/ChangeLog	2012-05-25 21:09:41 UTC (rev 118560)
@@ -1,3 +1,37 @@
+2012-05-25  Brady Eidson  <[email protected]>
+
+        https://bugs.webkit.org/show_bug.cgi?id=87418
+        WebBackForwardList should separate "has no current index" from the integer value of the current index
+
+        This patch also renames "m_current" to "m_currentIndex" for clarity and symmetry with
+        other variable names that reference "index",
+
+        It also removes the m_closed and m_enabled flags which were never actually used.
+
+        Reviewed by Darin Adler.
+
+        * UIProcess/WebBackForwardList.cpp:
+        (WebKit::WebBackForwardList::WebBackForwardList):
+        (WebKit::WebBackForwardList::addItem):
+        (WebKit::WebBackForwardList::goToItem):
+        (WebKit::WebBackForwardList::currentItem):
+        (WebKit::WebBackForwardList::backItem):
+        (WebKit::WebBackForwardList::forwardItem):
+        (WebKit::WebBackForwardList::itemAtIndex):
+        (WebKit::WebBackForwardList::backListCount):
+        (WebKit::WebBackForwardList::forwardListCount):
+        (WebKit::WebBackForwardList::backListAsImmutableArrayWithLimit):
+        (WebKit::WebBackForwardList::forwardListAsImmutableArrayWithLimit):
+        (WebKit::WebBackForwardList::clear):
+
+        * UIProcess/WebBackForwardList.h:
+        (WebKit::WebBackForwardList::currentIndex):
+        (WebBackForwardList):
+
+        * UIProcess/cf/WebBackForwardListCF.cpp:
+        (WebKit::WebBackForwardList::createCFDictionaryRepresentation):
+        (WebKit::WebBackForwardList::restoreFromCFDictionaryRepresentation):
+
 2012-05-25  Beth Dakin  <[email protected]>
 
         https://bugs.webkit.org/show_bug.cgi?id=87529

Modified: trunk/Source/WebKit2/UIProcess/WebBackForwardList.cpp (118559 => 118560)


--- trunk/Source/WebKit2/UIProcess/WebBackForwardList.cpp	2012-05-25 21:07:28 UTC (rev 118559)
+++ trunk/Source/WebKit2/UIProcess/WebBackForwardList.cpp	2012-05-25 21:09:41 UTC (rev 118560)
@@ -34,10 +34,9 @@
 
 WebBackForwardList::WebBackForwardList(WebPageProxy* page)
     : m_page(page)
-    , m_current(NoCurrentItemIndex)
+    , m_hasCurrentIndex(false)
+    , m_currentIndex(0)
     , m_capacity(DefaultCapacity)
-    , m_closed(true)
-    , m_enabled(true)
 {
     ASSERT(m_page);
 }
@@ -59,16 +58,16 @@
 
 void WebBackForwardList::addItem(WebBackForwardListItem* newItem)
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
-    if (!m_capacity || !m_enabled || !newItem || !m_page)
+    if (!m_capacity || !newItem || !m_page)
         return;
 
     Vector<RefPtr<APIObject> > removedItems;
     
-    if (m_current != NoCurrentItemIndex) {
+    if (m_hasCurrentIndex) {
         // Toss everything in the forward list.
-        unsigned targetSize = m_current + 1;
+        unsigned targetSize = m_currentIndex + 1;
         removedItems.reserveCapacity(m_entries.size() - targetSize);
         while (m_entries.size() > targetSize) {
             m_page->backForwardRemovedItem(m_entries.last()->itemID());
@@ -78,11 +77,14 @@
 
         // Toss the first item if the list is getting too big, as long as we're not using it
         // (or even if we are, if we only want 1 entry).
-        if (m_entries.size() == m_capacity && (m_current || m_capacity == 1)) {
+        if (m_entries.size() == m_capacity && (m_currentIndex || m_capacity == 1)) {
             m_page->backForwardRemovedItem(m_entries[0]->itemID());
             removedItems.append(m_entries[0].release());
             m_entries.remove(0);
-            m_current--;
+            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.
@@ -94,23 +96,24 @@
         m_entries.clear();
     }
     
-    if (m_current == NoCurrentItemIndex)
-        m_current = 0;
-    else
-        m_current++;
+    if (!m_hasCurrentIndex) {
+        m_currentIndex = 0;
+        m_hasCurrentIndex = true;
+    } else
+        m_currentIndex++;
 
     // m_current never be pointing more than 1 past the end of the entries Vector.
     // If it is, something has gone wrong and we should not try to insert the new item.
-    ASSERT(m_current <= m_entries.size());
-    if (m_current <= m_entries.size())
-        m_entries.insert(m_current, newItem);
+    ASSERT(m_currentIndex <= m_entries.size());
+    if (m_currentIndex <= m_entries.size())
+        m_entries.insert(m_currentIndex, newItem);
 
     m_page->didChangeBackForwardList(newItem, &removedItems);
 }
 
 void WebBackForwardList::goToItem(WebBackForwardListItem* item)
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
     if (!m_entries.size() || !item)
         return;
@@ -121,7 +124,7 @@
             break;
     }
     if (index < m_entries.size()) {
-        m_current = index;
+        m_currentIndex = index;
         if (m_page)
             m_page->didChangeBackForwardList(0, 0);
     }
@@ -129,36 +132,30 @@
 
 WebBackForwardListItem* WebBackForwardList::currentItem()
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
-    if (m_current != NoCurrentItemIndex)
-        return m_entries[m_current].get();
-    return 0;
+    return m_hasCurrentIndex ? m_entries[m_currentIndex].get() : 0;
 }
 
 WebBackForwardListItem* WebBackForwardList::backItem()
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
-    if (m_current && m_current != NoCurrentItemIndex)
-        return m_entries[m_current - 1].get();
-    return 0;
+    return m_hasCurrentIndex && m_currentIndex ? m_entries[m_currentIndex - 1].get() : 0;
 }
 
 WebBackForwardListItem* WebBackForwardList::forwardItem()
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
-    if (m_entries.size() && m_current < m_entries.size() - 1)
-        return m_entries[m_current + 1].get();
-    return 0;
+    return 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_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
-    if (m_current == NoCurrentItemIndex)
+    if (!m_hasCurrentIndex)
         return 0;
     
     // Do range checks without doing math on index to avoid overflow.
@@ -168,26 +165,26 @@
     if (index > forwardListCount())
         return 0;
         
-    return m_entries[index + m_current].get();
+    return m_entries[index + m_currentIndex].get();
 }
 
 int WebBackForwardList::backListCount()
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
-    return m_current == NoCurrentItemIndex ? 0 : m_current;
+    return !m_hasCurrentIndex ? 0 : m_currentIndex;
 }
 
 int WebBackForwardList::forwardListCount()
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
-    return m_current == NoCurrentItemIndex ? 0 : static_cast<int>(m_entries.size()) - (m_current + 1);
+    return !m_hasCurrentIndex ? 0 : static_cast<int>(m_entries.size()) - (m_currentIndex + 1);
 }
 
 PassRefPtr<ImmutableArray> WebBackForwardList::backListAsImmutableArrayWithLimit(unsigned limit)
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
     unsigned backListSize = static_cast<unsigned>(backListCount());
     unsigned size = std::min(backListSize, limit);
@@ -206,7 +203,7 @@
 
 PassRefPtr<ImmutableArray> WebBackForwardList::forwardListAsImmutableArrayWithLimit(unsigned limit)
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
     unsigned size = std::min(static_cast<unsigned>(forwardListCount()), limit);
     if (!size)
@@ -215,9 +212,9 @@
     Vector<RefPtr<APIObject> > vector;
     vector.reserveInitialCapacity(size);
 
-    unsigned last = m_current + size;
+    unsigned last = m_currentIndex + size;
     ASSERT(last < m_entries.size());
-    for (unsigned i = m_current + 1; i <= last; ++i)
+    for (unsigned i = m_currentIndex + 1; i <= last; ++i)
         vector.uncheckedAppend(m_entries[i].get());
 
     return ImmutableArray::adopt(vector);
@@ -225,7 +222,7 @@
 
 void WebBackForwardList::clear()
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
     size_t size = m_entries.size();
     if (size <= 1)
@@ -243,18 +240,18 @@
     Vector<RefPtr<APIObject> > removedItems;
     removedItems.reserveCapacity(m_entries.size() - 1);
     for (size_t i = 0; i < m_entries.size(); ++i) {
-        if (i != m_current)
+        if (i != m_currentIndex)
             removedItems.append(m_entries[i].release());
     }
 
-    m_current = 0;
+    m_currentIndex = 0;
 
     if (currentItem) {
         m_entries.shrink(1);
         m_entries[0] = currentItem.release();
     } else {
         m_entries.clear();
-        m_current = NoCurrentItemIndex;
+        m_hasCurrentIndex = false;
     }
 
     if (m_page)

Modified: trunk/Source/WebKit2/UIProcess/WebBackForwardList.h (118559 => 118560)


--- trunk/Source/WebKit2/UIProcess/WebBackForwardList.h	2012-05-25 21:07:28 UTC (rev 118559)
+++ trunk/Source/WebKit2/UIProcess/WebBackForwardList.h	2012-05-25 21:09:41 UTC (rev 118560)
@@ -70,7 +70,7 @@
     
     const BackForwardListItemVector& entries() const { return m_entries; }
 
-    uint32_t currentIndex() { return m_current; }
+    uint32_t currentIndex() { return m_currentIndex; }
     int backListCount();
     int forwardListCount();
 
@@ -83,18 +83,16 @@
 #endif
 
 private:
-    static const unsigned NoCurrentItemIndex = UINT_MAX;
-
     WebBackForwardList(WebPageProxy*);
 
     virtual Type type() const { return APIType; }
 
     WebPageProxy* m_page;
     BackForwardListItemVector m_entries;
-    uint32_t m_current;
-    uint32_t m_capacity;
-    bool m_closed;
-    bool m_enabled;
+    
+    bool m_hasCurrentIndex;
+    unsigned m_currentIndex;
+    unsigned m_capacity;
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebKit2/UIProcess/cf/WebBackForwardListCF.cpp (118559 => 118560)


--- trunk/Source/WebKit2/UIProcess/cf/WebBackForwardListCF.cpp	2012-05-25 21:07:28 UTC (rev 118559)
+++ trunk/Source/WebKit2/UIProcess/cf/WebBackForwardListCF.cpp	2012-05-25 21:09:41 UTC (rev 118560)
@@ -53,12 +53,12 @@
 
 CFDictionaryRef WebBackForwardList::createCFDictionaryRepresentation(WebPageProxy::WebPageProxySessionStateFilterCallback filter, void* context) const
 {
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
+    ASSERT(!m_hasCurrentIndex || m_currentIndex < m_entries.size());
 
     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_current;
+    CFIndex currentIndex = m_currentIndex;
 
     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.
@@ -67,7 +67,7 @@
             return 0;
 
         if (filter && !filter(toAPI(m_page), WKPageGetSessionHistoryURLValueType(), toURLRef(m_entries[i]->originalURL().impl()), context)) {
-            if (i <= static_cast<size_t>(m_current))
+            if (i <= static_cast<size_t>(m_currentIndex))
                 currentIndex--;
             continue;
         }
@@ -89,12 +89,12 @@
         
     // 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_current != NoCurrentItemIndex && CFArrayGetCount(entries.get()))
+    if (currentIndex == -1 && m_hasCurrentIndex && CFArrayGetCount(entries.get()))
         currentIndex = 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_current == NoCurrentItemIndex || !CFArrayGetCount(entries.get()))
+    if (!m_hasCurrentIndex || !CFArrayGetCount(entries.get()))
         currentIndex = -1;
 
     RetainPtr<CFNumberRef> currentIndexNumber(AdoptCF, CFNumberCreate(0, kCFNumberCFIndexType, &currentIndex));
@@ -138,9 +138,10 @@
 
     // 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.
-    uint32_t currentIndex = currentCFIndex == -1 ? NoCurrentItemIndex : static_cast<uint32_t>(currentCFIndex);
+    bool hasCurrentIndex = currentCFIndex > -1;
+    unsigned currentIndex = hasCurrentIndex ? static_cast<unsigned>(currentCFIndex) : 0;
 
-    if (currentIndex == NoCurrentItemIndex && size) {
+    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);
         return false;
     }
@@ -182,10 +183,8 @@
     }
     
     m_entries = newEntries;
-    m_current = currentIndex;
-    // Perform a sanity check: in case we're out of range, we reset.
-    if (m_current != NoCurrentItemIndex && m_current >= newEntries.size())
-        m_current = NoCurrentItemIndex;
+    m_hasCurrentIndex = hasCurrentIndex;
+    m_currentIndex = currentIndex;
 
     return true;
 }

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