Title: [156051] trunk/Source/WebKit/win
Revision
156051
Author
par...@webkit.org
Date
2013-09-18 10:32:29 -0700 (Wed, 18 Sep 2013)

Log Message

Make WebHistory more type safe
https://bugs.webkit.org/show_bug.cgi?id=119389

Reviewed by Brent Fulgham.

Use a WTF::HashMap instead of a CFMutableDictionary to avoid
casting from void* all the time when accessing the entries.
This reduces the dependencies on CoreFoundation too.

* WebHistory.cpp:
(WebHistory::WebHistory):
(WebHistory::removeAllItems):
(WebHistory::allItems):
(WebHistory::removeItem):
(WebHistory::addItem):
(WebHistory::visitedURL):
(WebHistory::itemForURL):
(WebHistory::removeItemForURLString):
(WebHistory::itemForURLString):
(WebHistory::addVisitedLinksToPageGroup):
* WebHistory.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit/win/ChangeLog (156050 => 156051)


--- trunk/Source/WebKit/win/ChangeLog	2013-09-18 17:27:08 UTC (rev 156050)
+++ trunk/Source/WebKit/win/ChangeLog	2013-09-18 17:32:29 UTC (rev 156051)
@@ -1,3 +1,27 @@
+2013-09-18  Patrick Gansterer  <par...@webkit.org>
+
+        Make WebHistory more type safe
+        https://bugs.webkit.org/show_bug.cgi?id=119389
+
+        Reviewed by Brent Fulgham.
+
+        Use a WTF::HashMap instead of a CFMutableDictionary to avoid
+        casting from void* all the time when accessing the entries.
+        This reduces the dependencies on CoreFoundation too.
+
+        * WebHistory.cpp:
+        (WebHistory::WebHistory):
+        (WebHistory::removeAllItems):
+        (WebHistory::allItems):
+        (WebHistory::removeItem):
+        (WebHistory::addItem):
+        (WebHistory::visitedURL):
+        (WebHistory::itemForURL):
+        (WebHistory::removeItemForURLString):
+        (WebHistory::itemForURLString):
+        (WebHistory::addVisitedLinksToPageGroup):
+        * WebHistory.h:
+
 2013-09-17  Brent Fulgham  <bfulg...@apple.com>
 
         [Windows] Speculative build fix after r155963

Modified: trunk/Source/WebKit/win/WebHistory.cpp (156050 => 156051)


--- trunk/Source/WebKit/win/WebHistory.cpp	2013-09-18 17:27:08 UTC (rev 156050)
+++ trunk/Source/WebKit/win/WebHistory.cpp	2013-09-18 17:32:29 UTC (rev 156051)
@@ -84,8 +84,6 @@
     gClassCount++;
     gClassNameCount.add("WebHistory");
 
-    m_entriesByURL = adoptCF(CFDictionaryCreateMutable(0, 0, &kCFTypeDictionaryKeyCallBacks, &MarshallingHelpers::kIUnknownDictionaryValueCallBacks));
-
     m_preferences = WebPreferences::sharedStandardPreferences();
 }
 
@@ -244,12 +242,12 @@
     m_entriesByDate.clear();
     m_orderedLastVisitedDays.clear();
 
-    CFIndex itemCount = CFDictionaryGetCount(m_entriesByURL.get());
-    Vector<IWebHistoryItem*> itemsVector(itemCount);
-    CFDictionaryGetKeysAndValues(m_entriesByURL.get(), 0, (const void**)itemsVector.data());
-    RetainPtr<CFArrayRef> allItems = adoptCF(CFArrayCreate(kCFAllocatorDefault, (const void**)itemsVector.data(), itemCount, &MarshallingHelpers::kIUnknownArrayCallBacks));
+    Vector<IWebHistoryItem*> itemsVector(m_entriesByURL.size());
+    for (auto it = m_entriesByURL.begin(); it != m_entriesByURL.end(); ++it)
+        itemsVector.append(it->value.get());
+    RetainPtr<CFArrayRef> allItems = adoptCF(CFArrayCreate(kCFAllocatorDefault, (const void**)itemsVector.data(), itemsVector.size(), &MarshallingHelpers::kIUnknownArrayCallBacks));
 
-    CFDictionaryRemoveAllValues(m_entriesByURL.get());
+    m_entriesByURL.clear();
 
     PageGroup::removeAllVisitedLinks();
 
@@ -330,7 +328,7 @@
     /* [out][in] */ int* count,
     /* [out][retval] */ IWebHistoryItem** items)
 {
-    int entriesByURLCount = CFDictionaryGetCount(m_entriesByURL.get());
+    int entriesByURLCount = m_entriesByURL.size();
 
     if (!items) {
         *count = entriesByURLCount;
@@ -343,9 +341,11 @@
     }
 
     *count = entriesByURLCount;
-    CFDictionaryGetKeysAndValues(m_entriesByURL.get(), 0, (const void**)items);
-    for (int i = 0; i < entriesByURLCount; i++)
+    int i = 0;
+    for (auto it = m_entriesByURL.begin(); it != m_entriesByURL.end(); ++i, ++it) {
+        items[i] = it->value.get();
         items[i]->AddRef();
+    }
 
     return S_OK;
 }
@@ -403,16 +403,19 @@
     if (FAILED(hr))
         return hr;
 
-    RetainPtr<CFStringRef> urlString = adoptCF(MarshallingHelpers::BSTRToCFStringRef(urlBStr));
+    String urlString(urlBStr, SysStringLen(urlBStr));
 
+    auto it = m_entriesByURL.find(urlString);
+    if (it == m_entriesByURL.end())
+        return E_FAIL;
+
     // If this exact object isn't stored, then make no change.
     // FIXME: Is this the right behavior if this entry isn't present, but another entry for the same URL is?
     // Maybe need to change the API to make something like removeEntryForURLString public instead.
-    IWebHistoryItem *matchingEntry = (IWebHistoryItem*)CFDictionaryGetValue(m_entriesByURL.get(), urlString.get());
-    if (matchingEntry != entry)
+    if (it->value.get() != entry)
         return E_FAIL;
 
-    hr = removeItemForURLString(urlString.get());
+    hr = removeItemForURLString(urlString);
     if (FAILED(hr))
         return hr;
 
@@ -435,20 +438,19 @@
     if (FAILED(hr))
         return hr;
 
-    RetainPtr<CFStringRef> urlString = adoptCF(MarshallingHelpers::BSTRToCFStringRef(urlBStr));
+    String urlString(urlBStr, SysStringLen(urlBStr));
 
-    COMPtr<IWebHistoryItem> oldEntry((IWebHistoryItem*) CFDictionaryGetValue(
-        m_entriesByURL.get(), urlString.get()));
-    
+    COMPtr<IWebHistoryItem> oldEntry(m_entriesByURL.get(urlString));
+
     if (oldEntry) {
         if (discardDuplicate) {
             if (added)
                 *added = false;
             return S_OK;
         }
-        
-        removeItemForURLString(urlString.get());
 
+        removeItemForURLString(urlString);
+
         // If we already have an item with this URL, we need to merge info that drives the
         // URL autocomplete heuristics from that item into the new one.
         IWebHistoryItemPrivate* entryPriv;
@@ -463,7 +465,7 @@
     if (FAILED(hr))
         return hr;
 
-    CFDictionarySetValue(m_entriesByURL.get(), urlString.get(), entry);
+    m_entriesByURL.set(urlString, entry);
 
     COMPtr<CFDictionaryPropertyBag> userInfo = createUserInfoFromHistoryItem(
         getNotificationString(kWebHistoryItemsAddedNotification), entry);
@@ -477,9 +479,7 @@
 
 void WebHistory::visitedURL(const KURL& url, const String& title, const String& httpMethod, bool wasFailure, bool increaseVisitCount)
 {
-    RetainPtr<CFStringRef> urlString = url.string().createCFString();
-
-    IWebHistoryItem* entry = (IWebHistoryItem*)CFDictionaryGetValue(m_entriesByURL.get(), urlString.get());
+    IWebHistoryItem* entry = m_entriesByURL.get(url.string()).get();
     if (entry) {
         COMPtr<IWebHistoryItemPrivate> entryPrivate(Query, entry);
         if (!entryPrivate)
@@ -507,7 +507,7 @@
         
         item->recordInitialVisit();
 
-        CFDictionarySetValue(m_entriesByURL.get(), urlString.get(), entry);
+        m_entriesByURL.set(url.string(), entry);
     }
 
     addItemToDateCaches(entry);
@@ -528,41 +528,30 @@
     postNotification(kWebHistoryItemsAddedNotification, userInfo.get());
 }
 
-HRESULT WebHistory::itemForURLString(
-    /* [in] */ CFStringRef urlString,
-    /* [retval][out] */ IWebHistoryItem** item) const
+HRESULT WebHistory::itemForURL(BSTR url, IWebHistoryItem** item)
 {
     if (!item)
         return E_FAIL;
     *item = 0;
 
-    IWebHistoryItem* foundItem = (IWebHistoryItem*) CFDictionaryGetValue(m_entriesByURL.get(), urlString);
-    if (!foundItem)
+    auto it = m_entriesByURL.find(url);
+    if (it == m_entriesByURL.end())
         return E_FAIL;
 
-    foundItem->AddRef();
-    *item = foundItem;
+    it->value.copyRefTo(item);
     return S_OK;
 }
 
-HRESULT STDMETHODCALLTYPE WebHistory::itemForURL( 
-    /* [in] */ BSTR url,
-    /* [retval][out] */ IWebHistoryItem** item)
+HRESULT WebHistory::removeItemForURLString(const WTF::String& urlString)
 {
-    RetainPtr<CFStringRef> urlString = adoptCF(MarshallingHelpers::BSTRToCFStringRef(url));
-    return itemForURLString(urlString.get(), item);
-}
-
-HRESULT WebHistory::removeItemForURLString(CFStringRef urlString)
-{
-    IWebHistoryItem* entry = (IWebHistoryItem*) CFDictionaryGetValue(m_entriesByURL.get(), urlString);
-    if (!entry) 
+    auto it = m_entriesByURL.find(urlString);
+    if (it == m_entriesByURL.end())
         return E_FAIL;
 
-    HRESULT hr = removeItemFromDateCaches(entry);
-    CFDictionaryRemoveValue(m_entriesByURL.get(), urlString);
+    HRESULT hr = removeItemFromDateCaches(it->value.get());
+    m_entriesByURL.remove(it);
 
-    if (!CFDictionaryGetCount(m_entriesByURL.get()))
+    if (!m_entriesByURL.size())
         PageGroup::removeAllVisitedLinks();
 
     return hr;
@@ -572,10 +561,7 @@
 {
     if (!urlString)
         return 0;
-    COMPtr<IWebHistoryItem> item;
-    if (FAILED(itemForURLString(urlString.createCFString().get(), &item)))
-        return 0;
-    return item;
+    return m_entriesByURL.get(urlString);
 }
 
 HRESULT WebHistory::addItemToDateCaches(IWebHistoryItem* entry)
@@ -764,23 +750,10 @@
     return S_OK;
 }
 
-static void addVisitedLinkToPageGroup(const void* key, const void*, void* context)
+void WebHistory::addVisitedLinksToPageGroup(PageGroup& group)
 {
-    CFStringRef url = ""
-    PageGroup* group = static_cast<PageGroup*>(context);
-
-    CFIndex length = CFStringGetLength(url);
-    const UChar* characters = reinterpret_cast<const UChar*>(CFStringGetCharactersPtr(url));
-    if (characters)
-        group->addVisitedLink(characters, length);
-    else {
-        Vector<UChar, 512> buffer(length);
-        CFStringGetCharacters(url, CFRangeMake(0, length), reinterpret_cast<UniChar*>(buffer.data()));
-        group->addVisitedLink(buffer.data(), length);
+    for (auto it = m_entriesByURL.begin(); it != m_entriesByURL.end(); ++it) {
+        const String& url = ""
+        group.addVisitedLink(url.characters(), url.length());
     }
 }
-
-void WebHistory::addVisitedLinksToPageGroup(PageGroup& group)
-{
-    CFDictionaryApplyFunction(m_entriesByURL.get(), addVisitedLinkToPageGroup, &group);
-}

Modified: trunk/Source/WebKit/win/WebHistory.h (156050 => 156051)


--- trunk/Source/WebKit/win/WebHistory.h	2013-09-18 17:27:08 UTC (rev 156050)
+++ trunk/Source/WebKit/win/WebHistory.h	2013-09-18 17:32:29 UTC (rev 156051)
@@ -118,6 +118,7 @@
 
     typedef int64_t DateKey;
     typedef HashMap<DateKey, RetainPtr<CFMutableArrayRef> > DateToEntriesMap;
+    typedef HashMap<WTF::String, COMPtr<IWebHistoryItem> > URLToEntriesMap;
 
 private:
 
@@ -134,7 +135,7 @@
     HRESULT postNotification(NotificationType notifyType, IPropertyBag* userInfo = 0);
     HRESULT removeItem(IWebHistoryItem* entry);
     HRESULT addItem(IWebHistoryItem* entry, bool discardDuplicate, bool* added);
-    HRESULT removeItemForURLString(CFStringRef urlString);
+    HRESULT removeItemForURLString(const WTF::String& urlString);
     HRESULT addItemToDateCaches(IWebHistoryItem* entry);
     HRESULT removeItemFromDateCaches(IWebHistoryItem* entry);
     HRESULT insertItem(IWebHistoryItem* entry, DateKey);
@@ -142,10 +143,9 @@
     bool findKey(DateKey*, CFAbsoluteTime forDay);
     static CFAbsoluteTime timeToDate(CFAbsoluteTime time);
     BSTR getNotificationString(NotificationType notifyType);
-    HRESULT itemForURLString(CFStringRef urlString, IWebHistoryItem** item) const;
 
     ULONG m_refCount;
-    RetainPtr<CFMutableDictionaryRef> m_entriesByURL;
+    URLToEntriesMap m_entriesByURL;
     DateToEntriesMap m_entriesByDate;
     OwnArrayPtr<DATE> m_orderedLastVisitedDays;
     COMPtr<WebPreferences> m_preferences;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to