Use more references in HistoryItem
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Feb 2015 00:49:36 +0000 (00:49 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Feb 2015 00:49:36 +0000 (00:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141133

Reviewed by Andreas Kling.

Use more references in HistoryItem instead of pointers.
Source/WebKit2:

* WebProcess/InjectedBundle/InjectedBundleBackForwardListItem.cpp:
(WebKit::InjectedBundleBackForwardListItem::children):
* WebProcess/WebCoreSupport/SessionStateConversion.cpp:
(WebKit::toFrameState):
(WebKit::applyFrameState):
* WebProcess/WebPage/WebBackForwardListProxy.cpp:
(WebKit::WebBackForwardListProxy::addItem):
* WebProcess/WebPage/WebBackForwardListProxy.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@179472 268f45cc-cd09-0410-ab3c-d52691b4dbfc

26 files changed:
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/history/BackForwardClient.h
Source/WebCore/history/BackForwardController.cpp
Source/WebCore/history/BackForwardController.h
Source/WebCore/history/BackForwardList.cpp
Source/WebCore/history/BackForwardList.h
Source/WebCore/history/HistoryItem.cpp
Source/WebCore/history/HistoryItem.h
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/HistoryController.cpp
Source/WebCore/loader/HistoryController.h
Source/WebCore/page/Page.cpp
Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/History/WebBackForwardList.mm
Source/WebKit/mac/History/WebHistoryItem.mm
Source/WebKit/mac/WebView/WebView.mm
Source/WebKit/win/WebBackForwardList.cpp
Source/WebKit/win/WebHistoryItem.cpp
Source/WebKit/win/WebView.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/InjectedBundle/InjectedBundleBackForwardListItem.cpp
Source/WebKit2/WebProcess/WebCoreSupport/SessionStateConversion.cpp
Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.cpp
Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.h

index 5390033..0c389bc 100644 (file)
@@ -1,3 +1,12 @@
+2015-02-01  Chris Dumez  <cdumez@apple.com>
+
+        Use more references in HistoryItem
+        https://bugs.webkit.org/show_bug.cgi?id=141133
+
+        Reviewed by Andreas Kling.
+
+        Use more references in HistoryItem instead of pointers.
+
 2015-02-01  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r179467 and r179470.
index cf49ff8..a8ac87b 100644 (file)
@@ -163,7 +163,7 @@ __ZN7WebCore11FrameLoader9setOpenerEPNS_5FrameE
 __ZN7WebCore11HistoryItem10targetItemEv
 __ZN7WebCore11HistoryItem11setFormDataEN3WTF10PassRefPtrINS_8FormDataEEE
 __ZN7WebCore11HistoryItem11setReferrerERKN3WTF6StringE
-__ZN7WebCore11HistoryItem12addChildItemEN3WTF10PassRefPtrIS0_EE
+__ZN7WebCore11HistoryItem12addChildItemEON3WTF3RefIS0_EE
 __ZN7WebCore11HistoryItem12setURLStringERKN3WTF6StringE
 __ZN7WebCore11HistoryItem12setViewStateEP11objc_object
 __ZN7WebCore11HistoryItem14addRedirectURLERKN3WTF6StringE
@@ -175,6 +175,7 @@ __ZN7WebCore11HistoryItem16setDocumentStateERKN3WTF6VectorINS1_6StringELm0ENS1_1
 __ZN7WebCore11HistoryItem17setAlternateTitleERKN3WTF6StringE
 __ZN7WebCore11HistoryItem18setFormContentTypeERKN3WTF6StringE
 __ZN7WebCore11HistoryItem18setPageScaleFactorEf
+__ZN7WebCore11HistoryItem19childItemWithTargetERKN3WTF6StringE
 __ZN7WebCore11HistoryItem20setOriginalURLStringERKN3WTF6StringE
 __ZN7WebCore11HistoryItem20setTransientPropertyERKN3WTF6StringEP11objc_object
 __ZN7WebCore11HistoryItem8formDataEv
@@ -498,8 +499,8 @@ __ZN7WebCore15BackForwardList11currentItemEv
 __ZN7WebCore15BackForwardList11forwardItemEv
 __ZN7WebCore15BackForwardList11setCapacityEi
 __ZN7WebCore15BackForwardList12containsItemEPNS_11HistoryItemE
-__ZN7WebCore15BackForwardList17backListWithLimitEiRN3WTF6VectorINS1_6RefPtrINS_11HistoryItemEEELm0ENS1_15CrashOnOverflowEEE
-__ZN7WebCore15BackForwardList20forwardListWithLimitEiRN3WTF6VectorINS1_6RefPtrINS_11HistoryItemEEELm0ENS1_15CrashOnOverflowEEE
+__ZN7WebCore15BackForwardList17backListWithLimitEiRN3WTF6VectorINS1_3RefINS_11HistoryItemEEELm0ENS1_15CrashOnOverflowEEE
+__ZN7WebCore15BackForwardList20forwardListWithLimitEiRN3WTF6VectorINS1_3RefINS_11HistoryItemEEELm0ENS1_15CrashOnOverflowEEE
 __ZN7WebCore15BackForwardList6closedEv
 __ZN7WebCore15BackForwardList6goBackEv
 __ZN7WebCore15BackForwardList7enabledEv
@@ -1692,7 +1693,6 @@ __ZNK7WebCore11HistoryItem14alternateTitleEv
 __ZNK7WebCore11HistoryItem15formContentTypeEv
 __ZNK7WebCore11HistoryItem15pageScaleFactorEv
 __ZNK7WebCore11HistoryItem17originalURLStringEv
-__ZNK7WebCore11HistoryItem19childItemWithTargetERKN3WTF6StringE
 __ZNK7WebCore11HistoryItem20getTransientPropertyERKN3WTF6StringE
 __ZNK7WebCore11HistoryItem20hasCachedPageExpiredEv
 __ZNK7WebCore11HistoryItem3urlEv
index 4718736..0820415 100644 (file)
@@ -41,7 +41,7 @@ public:
     {
     }
 
-    virtual void addItem(PassRefPtr<HistoryItem>) = 0;
+    virtual void addItem(Ref<HistoryItem>&&) = 0;
 
     virtual void goToItem(HistoryItem*) = 0;
         
index 55caf1c..1aeb63c 100644 (file)
@@ -97,9 +97,9 @@ bool BackForwardController::goForward()
     return true;
 }
 
-void BackForwardController::addItem(PassRefPtr<HistoryItem> item)
+void BackForwardController::addItem(Ref<HistoryItem>&& item)
 {
-    m_client->addItem(item);
+    m_client->addItem(WTF::move(item));
 }
 
 void BackForwardController::setCurrentItem(HistoryItem* item)
index f23b7e4..589ee5e 100644 (file)
@@ -50,7 +50,7 @@ public:
     WEBCORE_EXPORT bool goBack();
     WEBCORE_EXPORT bool goForward();
 
-    void addItem(PassRefPtr<HistoryItem>);
+    void addItem(Ref<HistoryItem>&&);
     void setCurrentItem(HistoryItem*);
         
     int count() const;
index eae62bc..0104d26 100644 (file)
@@ -55,36 +55,34 @@ BackForwardList::~BackForwardList()
     ASSERT(m_closed);
 }
 
-void BackForwardList::addItem(PassRefPtr<HistoryItem> prpItem)
+void BackForwardList::addItem(Ref<HistoryItem>&& newItem)
 {
-    ASSERT(prpItem);
-    if (m_capacity == 0 || !m_enabled)
+    if (!m_capacity || !m_enabled)
         return;
     
     // Toss anything in the forward list    
     if (m_current != NoCurrentItemIndex) {
         unsigned targetSize = m_current + 1;
         while (m_entries.size() > targetSize) {
-            RefPtr<HistoryItem> item = m_entries.last();
-            m_entries.removeLast();
-            m_entryHash.remove(item);
-            PageCache::singleton().remove(*item);
+            Ref<HistoryItem> item = m_entries.takeLast();
+            m_entryHash.remove(item.ptr());
+            PageCache::singleton().remove(item);
         }
     }
 
     // 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 != 0 || m_capacity == 1)) {
-        RefPtr<HistoryItem> item = m_entries[0];
+    if (m_entries.size() == m_capacity && (m_current || m_capacity == 1)) {
+        Ref<HistoryItem> item = WTF::move(m_entries[0]);
         m_entries.remove(0);
-        m_entryHash.remove(item);
-        PageCache::singleton().remove(*item);
-        m_current--;
+        m_entryHash.remove(item.ptr());
+        PageCache::singleton().remove(item);
+        --m_current;
     }
 
-    m_entryHash.add(prpItem.get());
-    m_entries.insert(m_current + 1, prpItem);
-    m_current++;
+    m_entryHash.add(newItem.ptr());
+    m_entries.insert(m_current + 1, WTF::move(newItem));
+    ++m_current;
 }
 
 void BackForwardList::goBack()
@@ -110,7 +108,7 @@ void BackForwardList::goToItem(HistoryItem* item)
         
     unsigned int index = 0;
     for (; index < m_entries.size(); ++index)
-        if (m_entries[index] == item)
+        if (m_entries[index].ptr() == item)
             break;
     if (index < m_entries.size()) {
         m_current = index;
@@ -120,31 +118,31 @@ void BackForwardList::goToItem(HistoryItem* item)
 HistoryItem* BackForwardList::backItem()
 {
     if (m_current && m_current != NoCurrentItemIndex)
-        return m_entries[m_current - 1].get();
-    return 0;
+        return m_entries[m_current - 1].ptr();
+    return nullptr;
 }
 
 HistoryItem* BackForwardList::currentItem()
 {
     if (m_current != NoCurrentItemIndex)
-        return m_entries[m_current].get();
-    return 0;
+        return m_entries[m_current].ptr();
+    return nullptr;
 }
 
 HistoryItem* BackForwardList::forwardItem()
 {
     if (m_entries.size() && m_current < m_entries.size() - 1)
-        return m_entries[m_current + 1].get();
-    return 0;
+        return m_entries[m_current + 1].ptr();
+    return nullptr;
 }
 
 void BackForwardList::backListWithLimit(int limit, HistoryItemVector& list)
 {
     list.clear();
     if (m_current != NoCurrentItemIndex) {
-        unsigned first = std::max((int)m_current - limit, 0);
+        unsigned first = std::max(static_cast<int>(m_current) - limit, 0);
         for (; first < m_current; ++first)
-            list.append(m_entries[first]);
+            list.append(m_entries[first].get());
     }
 }
 
@@ -160,7 +158,7 @@ void BackForwardList::forwardListWithLimit(int limit, HistoryItemVector& list)
         int last = std::min(m_current + limit, lastEntry);
         limit = m_current + 1;
         for (; limit <= last; ++limit)
-            list.append(m_entries[limit]);
+            list.append(m_entries[limit].get());
     }
 }
 
@@ -171,11 +169,10 @@ int BackForwardList::capacity()
 
 void BackForwardList::setCapacity(int size)
 {    
-    while (size < (int)m_entries.size()) {
-        RefPtr<HistoryItem> item = m_entries.last();
-        m_entries.removeLast();
-        m_entryHash.remove(item);
-        PageCache::singleton().remove(*item);
+    while (size < static_cast<int>(m_entries.size())) {
+        Ref<HistoryItem> item = m_entries.takeLast();
+        m_entryHash.remove(item.ptr());
+        PageCache::singleton().remove(item);
     }
 
     if (!size)
@@ -214,13 +211,13 @@ int BackForwardList::forwardListCount()
 HistoryItem* BackForwardList::itemAtIndex(int index)
 {
     // Do range checks without doing math on index to avoid overflow.
-    if (index < -(int)m_current)
-        return 0;
+    if (index < -static_cast<int>(m_current))
+        return nullptr;
     
     if (index > forwardListCount())
-        return 0;
+        return nullptr;
         
-    return m_entries[index + m_current].get();
+    return m_entries[index + m_current].ptr();
 }
 
 HistoryItemVector& BackForwardList::entries()
@@ -245,7 +242,7 @@ bool BackForwardList::clearAllPageCaches()
     for (auto& item : m_entries) {
         if (item->isInPageCache()) {
             didRemoveAtLeastOneItem = true;
-            PageCache::singleton().remove(*item);
+            PageCache::singleton().remove(item);
         }
     }
     return didRemoveAtLeastOneItem;
@@ -255,7 +252,7 @@ bool BackForwardList::clearAllPageCaches()
 void BackForwardList::close()
 {
     for (auto& item : m_entries)
-        PageCache::singleton().remove(*item);
+        PageCache::singleton().remove(item);
     m_entries.clear();
     m_entryHash.clear();
     m_page = nullptr;
@@ -272,8 +269,8 @@ void BackForwardList::removeItem(HistoryItem* item)
     if (!item)
         return;
     
-    for (unsigned i = 0; i < m_entries.size(); ++i)
-        if (m_entries[i] == item) {
+    for (unsigned i = 0; i < m_entries.size(); ++i) {
+        if (m_entries[i].ptr() == item) {
             m_entries.remove(i);
             m_entryHash.remove(item);
             if (m_current == NoCurrentItemIndex || m_current < i)
@@ -287,6 +284,7 @@ void BackForwardList::removeItem(HistoryItem* item)
             }
             break;
         }
+    }
 }
 
 bool BackForwardList::containsItem(HistoryItem* entry)
index a33c8a6..5929276 100644 (file)
@@ -36,7 +36,7 @@ namespace WebCore {
 
 class Page;
 
-typedef Vector<RefPtr<HistoryItem>> HistoryItemVector;
+typedef Vector<Ref<HistoryItem>> HistoryItemVector;
 typedef HashSet<RefPtr<HistoryItem>> HistoryItemHashSet;
 
 class BackForwardList : public BackForwardClient {
@@ -46,7 +46,7 @@ public:
 
     Page* page() { return m_page; }
 
-    virtual void addItem(PassRefPtr<HistoryItem>) override;
+    virtual void addItem(Ref<HistoryItem>&&) override;
     WEBCORE_EXPORT void goBack();
     WEBCORE_EXPORT void goForward();
     virtual void goToItem(HistoryItem*) override;
index 3c52fd6..229bab8 100644 (file)
@@ -172,9 +172,9 @@ inline HistoryItem::HistoryItem(const HistoryItem& item)
         m_redirectURLs = std::make_unique<Vector<String>>(*item.m_redirectURLs);
 }
 
-PassRefPtr<HistoryItem> HistoryItem::copy() const
+Ref<HistoryItem> HistoryItem::copy() const
 {
-    return adoptRef(new HistoryItem(*this));
+    return adoptRef(*new HistoryItem(*this));
 }
 
 void HistoryItem::reset()
@@ -366,44 +366,44 @@ void HistoryItem::setStateObject(PassRefPtr<SerializedScriptValue> object)
     m_stateObject = object;
 }
 
-void HistoryItem::addChildItem(PassRefPtr<HistoryItem> child)
+void HistoryItem::addChildItem(Ref<HistoryItem>&& child)
 {
     ASSERT(!childItemWithTarget(child->target()));
-    m_children.append(child);
+    m_children.append(WTF::move(child));
 }
 
-void HistoryItem::setChildItem(PassRefPtr<HistoryItem> child)
+void HistoryItem::setChildItem(Ref<HistoryItem>&& child)
 {
     ASSERT(!child->isTargetItem());
     unsigned size = m_children.size();
     for (unsigned i = 0; i < size; ++i)  {
         if (m_children[i]->target() == child->target()) {
             child->setIsTargetItem(m_children[i]->isTargetItem());
-            m_children[i] = child;
+            m_children[i] = WTF::move(child);
             return;
         }
     }
-    m_children.append(child);
+    m_children.append(WTF::move(child));
 }
 
-HistoryItem* HistoryItem::childItemWithTarget(const String& target) const
+HistoryItem* HistoryItem::childItemWithTarget(const String& target)
 {
     unsigned size = m_children.size();
     for (unsigned i = 0; i < size; ++i) {
         if (m_children[i]->target() == target)
-            return m_children[i].get();
+            return m_children[i].ptr();
     }
-    return 0;
+    return nullptr;
 }
 
-HistoryItem* HistoryItem::childItemWithDocumentSequenceNumber(long long number) const
+HistoryItem* HistoryItem::childItemWithDocumentSequenceNumber(long long number)
 {
     unsigned size = m_children.size();
     for (unsigned i = 0; i < size; ++i) {
         if (m_children[i]->documentSequenceNumber() == number)
-            return m_children[i].get();
+            return m_children[i].ptr();
     }
-    return 0;
+    return nullptr;
 }
 
 // <rdar://problem/4895849> HistoryItem::findTargetItem() should be replaced with a non-recursive method.
@@ -413,15 +413,17 @@ HistoryItem* HistoryItem::findTargetItem()
         return this;
     unsigned size = m_children.size();
     for (unsigned i = 0; i < size; ++i) {
+        // FIXME: targetItem() cannot return null currently, which is wrong.
         if (HistoryItem* match = m_children[i]->targetItem())
             return match;
     }
-    return 0;
+    return nullptr;
 }
 
 HistoryItem* HistoryItem::targetItem()
 {
     HistoryItem* foundItem = findTargetItem();
+    // FIXME: This should probably not fall back to |this|.
     return foundItem ? foundItem : this;
 }
 
@@ -440,13 +442,13 @@ void HistoryItem::clearChildren()
     m_children.clear();
 }
 
-bool HistoryItem::isAncestorOf(const HistoryItem* item) const
+bool HistoryItem::isAncestorOf(const HistoryItem& item) const
 {
     for (size_t i = 0; i < m_children.size(); ++i) {
-        HistoryItem* child = m_children[i].get();
-        if (child == item)
+        auto& child = m_children[i].get();
+        if (&child == &item)
             return true;
-        if (child->isAncestorOf(item))
+        if (child.isAncestorOf(item))
             return true;
     }
     return false;
@@ -455,34 +457,34 @@ bool HistoryItem::isAncestorOf(const HistoryItem* item) const
 // We do same-document navigation if going to a different item and if either of the following is true:
 // - The other item corresponds to the same document (for history entries created via pushState or fragment changes).
 // - The other item corresponds to the same set of documents, including frames (for history entries created via regular navigation)
-bool HistoryItem::shouldDoSameDocumentNavigationTo(HistoryItem* otherItem) const
+bool HistoryItem::shouldDoSameDocumentNavigationTo(HistoryItem& otherItem) const
 {
-    if (this == otherItem)
+    if (this == &otherItem)
         return false;
 
-    if (stateObject() || otherItem->stateObject())
-        return documentSequenceNumber() == otherItem->documentSequenceNumber();
+    if (stateObject() || otherItem.stateObject())
+        return documentSequenceNumber() == otherItem.documentSequenceNumber();
     
-    if ((url().hasFragmentIdentifier() || otherItem->url().hasFragmentIdentifier()) && equalIgnoringFragmentIdentifier(url(), otherItem->url()))
-        return documentSequenceNumber() == otherItem->documentSequenceNumber();        
+    if ((url().hasFragmentIdentifier() || otherItem.url().hasFragmentIdentifier()) && equalIgnoringFragmentIdentifier(url(), otherItem.url()))
+        return documentSequenceNumber() == otherItem.documentSequenceNumber();
     
     return hasSameDocumentTree(otherItem);
 }
 
 // Does a recursive check that this item and its descendants have the same
 // document sequence numbers as the other item.
-bool HistoryItem::hasSameDocumentTree(HistoryItem* otherItem) const
+bool HistoryItem::hasSameDocumentTree(HistoryItem& otherItem) const
 {
-    if (documentSequenceNumber() != otherItem->documentSequenceNumber())
+    if (documentSequenceNumber() != otherItem.documentSequenceNumber())
         return false;
         
-    if (children().size() != otherItem->children().size())
+    if (children().size() != otherItem.children().size())
         return false;
 
     for (size_t i = 0; i < children().size(); i++) {
-        HistoryItem* child = children()[i].get();
-        HistoryItem* otherChild = otherItem->childItemWithDocumentSequenceNumber(child->documentSequenceNumber());
-        if (!otherChild || !child->hasSameDocumentTree(otherChild))
+        auto& child = children()[i].get();
+        auto* otherChild = otherItem.childItemWithDocumentSequenceNumber(child.documentSequenceNumber());
+        if (!otherChild || !child.hasSameDocumentTree(*otherChild))
             return false;
     }
 
@@ -491,16 +493,16 @@ bool HistoryItem::hasSameDocumentTree(HistoryItem* otherItem) const
 
 // Does a non-recursive check that this item and its immediate children have the
 // same frames as the other item.
-bool HistoryItem::hasSameFrames(HistoryItem* otherItem) const
+bool HistoryItem::hasSameFrames(HistoryItem& otherItem) const
 {
-    if (target() != otherItem->target())
+    if (target() != otherItem.target())
         return false;
         
-    if (children().size() != otherItem->children().size())
+    if (children().size() != otherItem.children().size())
         return false;
 
     for (size_t i = 0; i < children().size(); i++) {
-        if (!otherItem->childItemWithTarget(children()[i]->target()))
+        if (!otherItem.childItemWithTarget(children()[i]->target()))
             return false;
     }
 
@@ -542,10 +544,10 @@ FormData* HistoryItem::formData()
     return m_formData.get();
 }
 
-bool HistoryItem::isCurrentDocument(Document* doc) const
+bool HistoryItem::isCurrentDocument(Document& document) const
 {
     // FIXME: We should find a better way to check if this is the current document.
-    return equalIgnoringFragmentIdentifier(url(), doc->url());
+    return equalIgnoringFragmentIdentifier(url(), document.url());
 }
 
 void HistoryItem::addRedirectURL(const String& url)
index 64c04be..4d843c4 100644 (file)
@@ -55,7 +55,7 @@ class ResourceRequest;
 class URL;
 enum class PruningReason;
 
-typedef Vector<RefPtr<HistoryItem>> HistoryItemVector;
+typedef Vector<Ref<HistoryItem>> HistoryItemVector;
 
 extern void (*notifyHistoryItemChanged)(HistoryItem*);
 
@@ -79,7 +79,7 @@ public:
     
     WEBCORE_EXPORT ~HistoryItem();
 
-    WEBCORE_EXPORT PassRefPtr<HistoryItem> copy() const;
+    WEBCORE_EXPORT Ref<HistoryItem> copy() const;
 
     // Resets the HistoryItem to its initial state, as returned by create().
     void reset();
@@ -141,24 +141,24 @@ public:
 
     void setLastVisitWasFailure(bool wasFailure) { m_lastVisitWasFailure = wasFailure; }
 
-    WEBCORE_EXPORT void addChildItem(PassRefPtr<HistoryItem>);
-    void setChildItem(PassRefPtr<HistoryItem>);
-    WEBCORE_EXPORT HistoryItem* childItemWithTarget(const String&) const;
-    HistoryItem* childItemWithDocumentSequenceNumber(long long number) const;
+    WEBCORE_EXPORT void addChildItem(Ref<HistoryItem>&&);
+    void setChildItem(Ref<HistoryItem>&&);
+    WEBCORE_EXPORT HistoryItem* childItemWithTarget(const String&);
+    HistoryItem* childItemWithDocumentSequenceNumber(long long number);
     WEBCORE_EXPORT HistoryItem* targetItem();
     WEBCORE_EXPORT const HistoryItemVector& children() const;
     WEBCORE_EXPORT bool hasChildren() const;
     void clearChildren();
-    bool isAncestorOf(const HistoryItem*) const;
+    bool isAncestorOf(const HistoryItem&) const;
     
-    bool shouldDoSameDocumentNavigationTo(HistoryItem* otherItem) const;
-    bool hasSameFrames(HistoryItem* otherItem) const;
+    bool shouldDoSameDocumentNavigationTo(HistoryItem& otherItem) const;
+    bool hasSameFrames(HistoryItem& otherItem) const;
 
     WEBCORE_EXPORT void addRedirectURL(const String&);
     WEBCORE_EXPORT Vector<String>* redirectURLs() const;
     WEBCORE_EXPORT void setRedirectURLs(std::unique_ptr<Vector<String>>);
 
-    bool isCurrentDocument(Document*) const;
+    bool isCurrentDocument(Document&) const;
     
 #if PLATFORM(COCOA)
     WEBCORE_EXPORT id viewState() const;
@@ -212,9 +212,9 @@ private:
     WEBCORE_EXPORT HistoryItem(const String& urlString, const String& title, const String& alternateTitle);
     WEBCORE_EXPORT HistoryItem(const URL&, const String& frameName, const String& parent, const String& title);
 
-    explicit HistoryItem(const HistoryItem&);
+    HistoryItem(const HistoryItem&);
 
-    bool hasSameDocumentTree(HistoryItem* otherItem) const;
+    bool hasSameDocumentTree(HistoryItem& otherItem) const;
 
     HistoryItem* findTargetItem();
 
index 059f0ff..ff1ecc4 100644 (file)
@@ -3143,7 +3143,7 @@ void FrameLoader::loadSameDocumentItem(HistoryItem& item)
     if (FrameView* view = m_frame.view())
         view->setWasScrolledByUser(false);
 
-    history().setCurrentItem(&item);
+    history().setCurrentItem(item);
         
     // loadInSameDocument() actually changes the URL and notifies load delegates of a "fake" load
     loadInSameDocument(item.url(), item.stateObject(), false);
@@ -3247,7 +3247,7 @@ void FrameLoader::loadItem(HistoryItem& item, FrameLoadType loadType)
 {
     m_requestedHistoryItem = &item;
     HistoryItem* currentItem = history().currentItem();
-    bool sameDocumentNavigation = currentItem && item.shouldDoSameDocumentNavigationTo(currentItem);
+    bool sameDocumentNavigation = currentItem && item.shouldDoSameDocumentNavigationTo(*currentItem);
 
     if (sameDocumentNavigation)
         loadSameDocumentItem(item);
index acf9090..d4610ca 100644 (file)
@@ -185,12 +185,11 @@ void HistoryController::saveDocumentState()
     if (!item)
         return;
 
-    Document* document = m_frame.document();
-    ASSERT(document);
-    
-    if (item->isCurrentDocument(document) && document->hasLivingRenderTree()) {
+    ASSERT(m_frame.document());
+    Document& document = *m_frame.document();
+    if (item->isCurrentDocument(document) && document.hasLivingRenderTree()) {
         LOG(Loading, "WebCoreLoading %s: saving form state to %p", m_frame.tree().uniqueName().string().utf8().data(), item);
-        item->setDocumentState(document->formElementsState());
+        item->setDocumentState(document.formElementsState());
     }
 }
 
@@ -253,7 +252,7 @@ void HistoryController::invalidateCurrentItemCachedPage()
     }
 }
 
-bool HistoryController::shouldStopLoadingForHistoryItem(HistoryItem* targetItem) const
+bool HistoryController::shouldStopLoadingForHistoryItem(HistoryItem& targetItem) const
 {
     if (!m_currentItem)
         return false;
@@ -469,8 +468,8 @@ void HistoryController::updateForCommit()
         // Note previousItem must be set before we close the URL, which will
         // happen when the data source is made non-provisional below
         ASSERT(m_provisionalItem);
-        setCurrentItem(m_provisionalItem.get());
-        m_provisionalItem = 0;
+        setCurrentItem(*m_provisionalItem);
+        m_provisionalItem = nullptr;
 
         // Tell all other frames in the tree to commit their provisional items and
         // restore their scroll position.  We'll avoid this frame (which has already
@@ -510,8 +509,8 @@ void HistoryController::recursiveUpdateForCommit()
             view->setWasScrolledByUser(false);
 
         // Now commit the provisional item
-        setCurrentItem(m_provisionalItem.get());
-        m_provisionalItem = 0;
+        setCurrentItem(*m_provisionalItem);
+        m_provisionalItem = nullptr;
 
         // Restore form state (works from currentItem)
         restoreDocumentState();
@@ -555,12 +554,12 @@ void HistoryController::recursiveUpdateForSameDocumentNavigation()
 
     // The provisional item may represent a different pending navigation.
     // Don't commit it if it isn't a same document navigation.
-    if (m_currentItem && !m_currentItem->shouldDoSameDocumentNavigationTo(m_provisionalItem.get()))
+    if (m_currentItem && !m_currentItem->shouldDoSameDocumentNavigationTo(*m_provisionalItem))
         return;
 
     // Commit the provisional item.
-    setCurrentItem(m_provisionalItem.get());
-    m_provisionalItem = 0;
+    setCurrentItem(*m_provisionalItem);
+    m_provisionalItem = nullptr;
 
     // Iterate over the rest of the tree.
     for (Frame* child = m_frame.tree().firstChild(); child; child = child->tree().nextSibling())
@@ -575,11 +574,11 @@ void HistoryController::updateForFrameLoadCompleted()
     m_frameLoadComplete = true;
 }
 
-void HistoryController::setCurrentItem(HistoryItem* item)
+void HistoryController::setCurrentItem(HistoryItem& item)
 {
     m_frameLoadComplete = false;
     m_previousItem = m_currentItem;
-    m_currentItem = item;
+    m_currentItem = &item;
 }
 
 void HistoryController::setCurrentItemTitle(const StringWithDirection& title)
@@ -610,7 +609,7 @@ void HistoryController::setProvisionalItem(HistoryItem* item)
     m_provisionalItem = item;
 }
 
-void HistoryController::initializeItem(HistoryItem* item)
+void HistoryController::initializeItem(HistoryItem& item)
 {
     DocumentLoader* documentLoader = m_frame.loader().documentLoader();
     ASSERT(documentLoader);
@@ -642,34 +641,34 @@ void HistoryController::initializeItem(HistoryItem* item)
     String parent = parentFrame ? parentFrame->tree().uniqueName() : "";
     StringWithDirection title = documentLoader->title();
 
-    item->setURL(url);
-    item->setTarget(m_frame.tree().uniqueName());
-    item->setParent(parent);
+    item.setURL(url);
+    item.setTarget(m_frame.tree().uniqueName());
+    item.setParent(parent);
     // FIXME: should store title directionality in history as well.
-    item->setTitle(title.string());
-    item->setOriginalURLString(originalURL.string());
+    item.setTitle(title.string());
+    item.setOriginalURLString(originalURL.string());
 
     if (!unreachableURL.isEmpty() || documentLoader->response().httpStatusCode() >= 400)
-        item->setLastVisitWasFailure(true);
+        item.setLastVisitWasFailure(true);
 
     // Save form state if this is a POST
-    item->setFormInfoFromRequest(documentLoader->request());
+    item.setFormInfoFromRequest(documentLoader->request());
 }
 
-PassRefPtr<HistoryItem> HistoryController::createItem()
+Ref<HistoryItem> HistoryController::createItem()
 {
-    RefPtr<HistoryItem> item = HistoryItem::create();
-    initializeItem(item.get());
+    Ref<HistoryItem> item = HistoryItem::create();
+    initializeItem(item);
     
     // Set the item for which we will save document state
-    setCurrentItem(item.get());
+    setCurrentItem(item);
     
-    return item.release();
+    return item;
 }
 
-PassRefPtr<HistoryItem> HistoryController::createItemTree(Frame& targetFrame, bool clipAtTarget)
+Ref<HistoryItem> HistoryController::createItemTree(Frame& targetFrame, bool clipAtTarget)
 {
-    RefPtr<HistoryItem> bfItem = createItem();
+    Ref<HistoryItem> bfItem = createItem();
     if (!m_frameLoadComplete)
         saveScrollPositionAndViewStateToItem(m_previousItem.get());
 
@@ -717,7 +716,7 @@ void HistoryController::recursiveSetProvisionalItem(HistoryItem& item, HistoryIt
     // Set provisional item, which will be committed in recursiveUpdateForCommit.
     m_provisionalItem = &item;
 
-    for (const auto& childItem : item.children()) {
+    for (auto& childItem : item.children()) {
         const String& childFrameName = childItem->target();
 
         HistoryItem* fromChildItem = fromItem->childItemWithTarget(childFrameName);
@@ -725,7 +724,7 @@ void HistoryController::recursiveSetProvisionalItem(HistoryItem& item, HistoryIt
         Frame* childFrame = m_frame.tree().child(childFrameName);
         ASSERT(childFrame);
 
-        childFrame->loader().history().recursiveSetProvisionalItem(*childItem, fromChildItem);
+        childFrame->loader().history().recursiveSetProvisionalItem(const_cast<HistoryItem&>(childItem.get()), fromChildItem);
     }
 }
 
@@ -739,14 +738,14 @@ void HistoryController::recursiveGoToItem(HistoryItem& item, HistoryItem* fromIt
     }
 
     // Just iterate over the rest, looking for frames to navigate.
-    for (const auto& childItem : item.children()) {
+    for (auto& childItem : item.children()) {
         const String& childFrameName = childItem->target();
 
         HistoryItem* fromChildItem = fromItem->childItemWithTarget(childFrameName);
         ASSERT(fromChildItem);
         Frame* childFrame = m_frame.tree().child(childFrameName);
         ASSERT(childFrame);
-        childFrame->loader().history().recursiveGoToItem(*childItem, fromChildItem, type);
+        childFrame->loader().history().recursiveGoToItem(const_cast<HistoryItem&>(childItem.get()), fromChildItem, type);
     }
 }
 
@@ -763,7 +762,7 @@ bool HistoryController::itemsAreClones(HistoryItem& item1, HistoryItem* item2) c
         && &item1 != item2
         && item1.itemSequenceNumber() == item2->itemSequenceNumber()
         && currentFramesMatchItem(&item1)
-        && item2->hasSameFrames(&item1);
+        && item2->hasSameFrames(item1);
 }
 
 // Helper method that determines whether the current frame tree matches given history item's.
@@ -801,9 +800,9 @@ void HistoryController::updateBackForwardListClippedAtTarget(bool doClip)
 
     FrameLoader& frameLoader = m_frame.mainFrame().loader();
 
-    RefPtr<HistoryItem> topItem = frameLoader.history().createItemTree(m_frame, doClip);
-    LOG(BackForward, "WebCoreBackForward - Adding backforward item %p for frame %s", topItem.get(), m_frame.loader().documentLoader()->url().string().ascii().data());
-    page->backForward().addItem(topItem.release());
+    Ref<HistoryItem> topItem = frameLoader.history().createItemTree(m_frame, doClip);
+    LOG(BackForward, "WebCoreBackForward - Adding backforward item %p for frame %s", topItem.ptr(), m_frame.loader().documentLoader()->url().string().ascii().data());
+    page->backForward().addItem(WTF::move(topItem));
 }
 
 void HistoryController::updateCurrentItem()
@@ -823,7 +822,7 @@ void HistoryController::updateCurrentItem()
         // dependent on the document.
         bool isTargetItem = m_currentItem->isTargetItem();
         m_currentItem->reset();
-        initializeItem(m_currentItem.get());
+        initializeItem(*m_currentItem);
         m_currentItem->setIsTargetItem(isTargetItem);
     } else {
         // Even if the final URL didn't change, the form data may have changed.
@@ -840,7 +839,7 @@ void HistoryController::pushState(PassRefPtr<SerializedScriptValue> stateObject,
     ASSERT(page);
 
     // Get a HistoryItem tree for the current frame tree.
-    RefPtr<HistoryItem> topItem = m_frame.mainFrame().loader().history().createItemTree(m_frame, false);
+    Ref<HistoryItem> topItem = m_frame.mainFrame().loader().history().createItemTree(m_frame, false);
     
     // Override data in the current item (created by createItemTree) to reflect
     // the pushState() arguments.
@@ -848,7 +847,7 @@ void HistoryController::pushState(PassRefPtr<SerializedScriptValue> stateObject,
     m_currentItem->setStateObject(stateObject);
     m_currentItem->setURLString(urlString);
 
-    page->backForward().addItem(topItem.release());
+    page->backForward().addItem(WTF::move(topItem));
 
     if (m_frame.page()->usesEphemeralSession())
         return;
index 1b40a80..e3952c6 100644 (file)
@@ -74,7 +74,7 @@ public:
     void updateForFrameLoadCompleted();
 
     HistoryItem* currentItem() const { return m_currentItem.get(); }
-    void setCurrentItem(HistoryItem*);
+    void setCurrentItem(HistoryItem&);
     void setCurrentItemTitle(const StringWithDirection&);
     bool currentItemShouldBeReplaced() const;
     WEBCORE_EXPORT void replaceCurrentItem(HistoryItem*);
@@ -92,12 +92,12 @@ public:
 
 private:
     friend class Page;
-    bool shouldStopLoadingForHistoryItem(HistoryItem*) const;
+    bool shouldStopLoadingForHistoryItem(HistoryItem&) const;
     void goToItem(HistoryItem&, FrameLoadType);
 
-    void initializeItem(HistoryItem*);
-    PassRefPtr<HistoryItem> createItem();
-    PassRefPtr<HistoryItem> createItemTree(Frame& targetFrame, bool clipAtTarget);
+    void initializeItem(HistoryItem&);
+    Ref<HistoryItem> createItem();
+    Ref<HistoryItem> createItemTree(Frame& targetFrame, bool clipAtTarget);
 
     void recursiveSetProvisionalItem(HistoryItem&, HistoryItem*);
     void recursiveGoToItem(HistoryItem&, HistoryItem*, FrameLoadType);
index 27c04b5..06b91ba 100644 (file)
@@ -436,7 +436,7 @@ void Page::goToItem(HistoryItem& item, FrameLoadType type)
     // being deref()-ed. Make sure we can still use it with HistoryController::goToItem later.
     Ref<HistoryItem> protector(item);
 
-    if (m_mainFrame->loader().history().shouldStopLoadingForHistoryItem(&item))
+    if (m_mainFrame->loader().history().shouldStopLoadingForHistoryItem(item))
         m_mainFrame->loader().stopAllLoaders();
 
     m_mainFrame->loader().history().goToItem(item, type);
index b07c44b..0160606 100644 (file)
@@ -169,7 +169,7 @@ EXPORTS
         symbolWithPointer(?absoluteBoundingBoxRectIgnoringTransforms@RenderObject@WebCore@@QBE?AVIntRect@2@XZ, ?absoluteBoundingBoxRectIgnoringTransforms@RenderObject@WebCore@@QEBA?AVIntRect@2@XZ)
         symbolWithPointer(?description@DocumentMarker@WebCore@@QBEABVString@WTF@@XZ, ?description@DocumentMarker@WebCore@@QEBAAEBVString@WTF@@XZ)
         symbolWithPointer(?cacheDOMStructure@WebCore@@YAPAVStructure@JSC@@PAVJSDOMGlobalObject@1@PAV23@PBUClassInfo@3@@Z, ?cacheDOMStructure@WebCore@@YAPEAVStructure@JSC@@PEAVJSDOMGlobalObject@1@PEAV23@PEBUClassInfo@3@@Z)
-        symbolWithPointer(?childItemWithTarget@HistoryItem@WebCore@@QBEPAV12@ABVString@WTF@@@Z, ?childItemWithTarget@HistoryItem@WebCore@@QEBAPEAV12@AEBVString@WTF@@@Z)
+        symbolWithPointer(?childItemWithTarget@HistoryItem@WebCore@@QAEPAV12@ABVString@WTF@@@Z, ?childItemWithTarget@HistoryItem@WebCore@@QEAEPEAV12@AEBVString@WTF@@@Z)
         symbolWithPointer(?create@Range@WebCore@@SA?AV?$Ref@VRange@WebCore@@@WTF@@AAVDocument@2@V?$PassRefPtr@VNode@WebCore@@@4@H1H@Z, ?create@Range@WebCore@@SA?AV?$Ref@VRange@WebCore@@@WTF@@AEAVDocument@2@V?$PassRefPtr@VNode@WebCore@@@4@H1H@Z)
         symbolWithPointer(?commonVM@JSDOMWindowBase@WebCore@@SAAAVVM@JSC@@XZ, ?commonVM@JSDOMWindowBase@WebCore@@SAAEAVVM@JSC@@XZ)
                symbolWithPointer(?contentDocument@HTMLFrameOwnerElement@WebCore@@QBEPAVDocument@2@XZ, ?contentDocument@HTMLFrameOwnerElement@WebCore@@QEBAPEAVDocument@2@XZ)
index c2b683c..528a717 100644 (file)
@@ -1,3 +1,12 @@
+2015-02-01  Chris Dumez  <cdumez@apple.com>
+
+        Use more references in HistoryItem
+        https://bugs.webkit.org/show_bug.cgi?id=141133
+
+        Reviewed by Andreas Kling.
+
+        Use more references in HistoryItem instead of pointers.
+
 2015-01-31  Sam Weinig  <sam@webkit.org>
 
         Remove even more Mountain Lion support
index ebfbd4d..e970524 100644 (file)
@@ -153,7 +153,8 @@ WebBackForwardList *kit(BackForwardList* backForwardList)
 
 - (void)addItem:(WebHistoryItem *)entry
 {
-    core(self)->addItem(core(entry));
+    ASSERT(entry);
+    core(self)->addItem(*core(entry));
     
     // Since the assumed contract with WebBackForwardList is that it retains its WebHistoryItems,
     // the following line prevents a whole class of problems where a history item will be created in
@@ -178,11 +179,11 @@ WebBackForwardList *kit(BackForwardList* backForwardList)
 {
     BackForwardList *coreBFList = core(self);
     
-    HistoryItemVector historyItems = coreBFList->entries();
+    HistoryItemVector& historyItems = coreBFList->entries();
     unsigned size = historyItems.size();
     NSMutableArray *entriesArray = [[NSMutableArray alloc] initWithCapacity:size];
     for (unsigned i = 0; i < size; ++i)
-        [entriesArray addObject:[kit(historyItems[i].get()) dictionaryRepresentationIncludingChildren:NO]];
+        [entriesArray addObject:[kit(historyItems[i].ptr()) dictionaryRepresentationIncludingChildren:NO]];
     
     NSDictionary *dictionary = [NSDictionary dictionaryWithObjectsAndKeys:
         entriesArray, WebBackForwardListDictionaryEntriesKey,
@@ -203,7 +204,7 @@ WebBackForwardList *kit(BackForwardList* backForwardList)
     
     for (NSDictionary *itemDictionary in [dictionary objectForKey:WebBackForwardListDictionaryEntriesKey]) {
         WebHistoryItem *item = [[WebHistoryItem alloc] initFromDictionaryRepresentation:itemDictionary];
-        coreBFList->addItem(core(item));
+        coreBFList->addItem(*core(item));
         [item release];
     }
 
@@ -255,7 +256,7 @@ static NSArray* vectorToNSArray(HistoryItemVector& list)
     unsigned size = list.size();
     NSMutableArray *result = [[[NSMutableArray alloc] initWithCapacity:size] autorelease];
     for (unsigned i = 0; i < size; ++i)
-        [result addObject:kit(list[i].get())];
+        [result addObject:kit(list[i].ptr())];
 
     return result;
 }
@@ -327,14 +328,14 @@ static bool bumperCarBackForwardHackNeeded()
     
     unsigned size = entries.size();
     for (unsigned i = 0; i < size; ++i) {
-        if (entries[i] == backForwardList->currentItem()) {
+        if (entries[i].ptr() == backForwardList->currentItem()) {
             [result appendString:@" >>>"]; 
         } else {
             [result appendString:@"    "]; 
         }   
         [result appendFormat:@"%2d) ", i];
         int currPos = [result length];
-        [result appendString:[kit(entries[i].get()) description]];
+        [result appendString:[kit(entries[i].ptr()) description]];
 
         // shift all the contents over.  a bit slow, but this is for debugging
         NSRange replRange = { static_cast<NSUInteger>(currPos), [result length] - currPos };
index e677b01..ae7601f 100644 (file)
@@ -258,7 +258,7 @@ void WKNotifyHistoryItemChanged(HistoryItem*)
         int currPos = [result length];
         unsigned size = children.size();        
         for (unsigned i = 0; i < size; ++i) {
-            WebHistoryItem *child = kit(children[i].get());
+            WebHistoryItem *child = kit(const_cast<HistoryItem*>(children[i].ptr()));
             [result appendString:@"\n"];
             [result appendString:[child description]];
         }
@@ -380,7 +380,7 @@ WebHistoryItem *kit(HistoryItem* item)
     if (childDicts) {
         for (int i = [childDicts count] - 1; i >= 0; i--) {
             WebHistoryItem *child = [[WebHistoryItem alloc] initFromDictionaryRepresentation:[childDicts objectAtIndex:i]];
-            core(_private)->addChildItem(core(child->_private));
+            core(_private)->addChildItem(*core(child->_private));
             [child release];
         }
     }
@@ -481,7 +481,7 @@ WebHistoryItem *kit(HistoryItem* item)
         NSMutableArray *childDicts = [NSMutableArray arrayWithCapacity:children.size()];
         
         for (int i = children.size() - 1; i >= 0; i--)
-            [childDicts addObject:[kit(children[i].get()) dictionaryRepresentation]];
+            [childDicts addObject:[kit(const_cast<HistoryItem*>(children[i].ptr())) dictionaryRepresentation]];
         [dict setObject: childDicts forKey:childrenKey];
     }
 
@@ -541,7 +541,7 @@ WebHistoryItem *kit(HistoryItem* item)
     NSMutableArray *result = [[[NSMutableArray alloc] initWithCapacity:size] autorelease];
     
     for (unsigned i = 0; i < size; ++i)
-        [result addObject:kit(children[i].get())];
+        [result addObject:kit(const_cast<HistoryItem*>(children[i].ptr()))];
     
     return result;
 }
index 74ad55a..483ca0e 100644 (file)
@@ -2002,10 +2002,10 @@ static bool fastDocumentTeardownEnabled()
             // until we leave that page.
             otherView->_private->page->mainFrame().loader().history().saveDocumentAndScrollState();
         }
-        RefPtr<HistoryItem> newItem = otherBackForwardClient->itemAtIndex(i)->copy();
+        Ref<HistoryItem> newItem = otherBackForwardClient->itemAtIndex(i)->copy();
         if (i == 0) 
-            newItemToGoTo = newItem.get();
-        backForwardClient->addItem(newItem.release());
+            newItemToGoTo = newItem.ptr();
+        backForwardClient->addItem(WTF::move(newItem));
     }
     
     ASSERT(newItemToGoTo);
index a139584..94b5a1b 100644 (file)
@@ -124,7 +124,7 @@ HRESULT STDMETHODCALLTYPE WebBackForwardList::addItem(
     if (!item || FAILED(item->QueryInterface(&webHistoryItem)))
         return E_FAIL;
  
-    m_backForwardList->addItem(webHistoryItem->historyItem());
+    m_backForwardList->addItem(*webHistoryItem->historyItem());
     return S_OK;
 }
 
@@ -209,7 +209,7 @@ HRESULT STDMETHODCALLTYPE WebBackForwardList::backListWithLimit(
 
     if (list)
         for (unsigned i = 0; i < historyItemVector.size(); i++)
-            list[i] = WebHistoryItem::createInstance(historyItemVector[i].get());
+            list[i] = WebHistoryItem::createInstance(historyItemVector[i].ptr());
 
     return S_OK;
 }
@@ -226,7 +226,7 @@ HRESULT STDMETHODCALLTYPE WebBackForwardList::forwardListWithLimit(
 
     if (list)
         for (unsigned i = 0; i < historyItemVector.size(); i++)
-            list[i] = WebHistoryItem::createInstance(historyItemVector[i].get());
+            list[i] = WebHistoryItem::createInstance(historyItemVector[i].ptr());
 
     return S_OK;
 }
index 68f5138..e247784 100644 (file)
@@ -286,7 +286,7 @@ HRESULT STDMETHODCALLTYPE WebHistoryItem::children(unsigned* outChildCount, SAFE
         return E_OUTOFMEMORY;
 
     for (unsigned i = 0; i < childCount; ++i) {
-        COMPtr<WebHistoryItem> item(AdoptCOM, WebHistoryItem::createInstance(coreChildren[i]));
+        COMPtr<WebHistoryItem> item(AdoptCOM, WebHistoryItem::createInstance(const_cast<HistoryItem*>(coreChildren[i].ptr())));
         if (!item) {
             SafeArrayDestroy(children);
             return E_OUTOFMEMORY;
index 99d353c..7ae02c0 100644 (file)
@@ -5489,10 +5489,10 @@ HRESULT STDMETHODCALLTYPE WebView::loadBackForwardListFromOtherView(
             // until we leave that page.
             otherWebView->m_page->mainFrame().loader().history().saveDocumentAndScrollState();
         }
-        RefPtr<HistoryItem> newItem = otherBackForwardClient->itemAtIndex(i)->copy();
+        Ref<HistoryItem> newItem = otherBackForwardClient->itemAtIndex(i)->copy();
         if (!i) 
-            newItemToGoTo = newItem.get();
-        backForwardClient->addItem(newItem.release());
+            newItemToGoTo = newItem.ptr();
+        backForwardClient->addItem(WTF::move(newItem));
     }
     
     ASSERT(newItemToGoTo);
index 2311a57..021e788 100644 (file)
@@ -1,3 +1,21 @@
+2015-02-01  Chris Dumez  <cdumez@apple.com>
+
+        Use more references in HistoryItem
+        https://bugs.webkit.org/show_bug.cgi?id=141133
+
+        Reviewed by Andreas Kling.
+
+        Use more references in HistoryItem instead of pointers.
+
+        * WebProcess/InjectedBundle/InjectedBundleBackForwardListItem.cpp:
+        (WebKit::InjectedBundleBackForwardListItem::children):
+        * WebProcess/WebCoreSupport/SessionStateConversion.cpp:
+        (WebKit::toFrameState):
+        (WebKit::applyFrameState):
+        * WebProcess/WebPage/WebBackForwardListProxy.cpp:
+        (WebKit::WebBackForwardListProxy::addItem):
+        * WebProcess/WebPage/WebBackForwardListProxy.h:
+
 2015-02-01  Dan Bernstein  <mitz@apple.com>
 
         Remove ViewGestureController tracing
index 2891935..aae6a46 100644 (file)
@@ -38,7 +38,7 @@ PassRefPtr<API::Array> InjectedBundleBackForwardListItem::children() const
     children.reserveInitialCapacity(m_item->children().size());
 
     for (const auto& child : m_item->children())
-        children.uncheckedAppend(InjectedBundleBackForwardListItem::create(child));
+        children.uncheckedAppend(InjectedBundleBackForwardListItem::create(const_cast<HistoryItem*>(child.ptr())));
 
     return API::Array::create(WTF::move(children));
 }
index b094099..fd0a98e 100644 (file)
@@ -103,7 +103,7 @@ static FrameState toFrameState(const HistoryItem& historyItem)
 #endif
 
     for (auto& childHistoryItem : historyItem.children()) {
-        FrameState childFrameState = toFrameState(*childHistoryItem);
+        FrameState childFrameState = toFrameState(childHistoryItem);
         frameState.children.append(WTF::move(childFrameState));
     }
 
@@ -178,10 +178,10 @@ static void applyFrameState(HistoryItem& historyItem, const FrameState& frameSta
 #endif
 
     for (const auto& childFrameState : frameState.children) {
-        RefPtr<HistoryItem> childHistoryItem = HistoryItem::create(childFrameState.urlString, String());
-        applyFrameState(*childHistoryItem, childFrameState);
+        Ref<HistoryItem> childHistoryItem = HistoryItem::create(childFrameState.urlString, String());
+        applyFrameState(childHistoryItem, childFrameState);
 
-        historyItem.addChildItem(childHistoryItem.release());
+        historyItem.addChildItem(WTF::move(childHistoryItem));
     }
 }
 
index a9c6779..9dc2a61 100644 (file)
@@ -143,11 +143,9 @@ WebBackForwardListProxy::WebBackForwardListProxy(WebPage* page)
     WebCore::notifyHistoryItemChanged = WK2NotifyHistoryItemChanged;
 }
 
-void WebBackForwardListProxy::addItem(PassRefPtr<HistoryItem> prpItem)
+void WebBackForwardListProxy::addItem(Ref<HistoryItem>&& item)
 {
-    RefPtr<HistoryItem> item = prpItem;
-
-    ASSERT(!historyItemToIDMap().contains(item));
+    ASSERT(!historyItemToIDMap().contains(item.ptr()));
 
     if (!m_page)
         return;
@@ -158,10 +156,10 @@ void WebBackForwardListProxy::addItem(PassRefPtr<HistoryItem> prpItem)
 
     m_associatedItemIDs.add(itemID);
 
-    historyItemToIDMap().set<ItemAndPageID>(item, { .itemID = itemID, .pageID = m_page->pageID() });
-    idToHistoryItemMap().set(itemID, item);
+    historyItemToIDMap().set<ItemAndPageID>(item.ptr(), { .itemID = itemID, .pageID = m_page->pageID() });
+    idToHistoryItemMap().set(itemID, item.ptr());
 
-    updateBackForwardItem(itemID, m_page->pageID(), item.get());
+    updateBackForwardItem(itemID, m_page->pageID(), item.ptr());
     m_page->send(Messages::WebPageProxy::BackForwardAddItem(itemID));
 }
 
index aa147e1..2cbded2 100644 (file)
@@ -50,7 +50,7 @@ public:
 private:
     WebBackForwardListProxy(WebPage*);
 
-    virtual void addItem(PassRefPtr<WebCore::HistoryItem>) override;
+    virtual void addItem(Ref<WebCore::HistoryItem>&&) override;
 
     virtual void goToItem(WebCore::HistoryItem*) override;