Changeset 149654 in webkit


Ignore:
Timestamp:
May 6, 2013 5:45:31 PM (11 years ago)
Author:
Darin Adler
Message:

Use OwnPtr instead of deleteAllValues in DocumentMarkerController
​https://bugs.webkit.org/show_bug.cgi?id=115655

Reviewed by Andreas Kling.

  • dom/DocumentMarkerController.cpp:

(WebCore::DocumentMarkerController::~DocumentMarkerController): Added here
so the destructor isn't implicitly inline.
(WebCore::DocumentMarkerController::detach): Removed now-unneeded code to
call deleteAllValues. Also moved code to set m_possiblyExistingMarkerTypes
to after clearing m_markers to be consistent with how other functions do it.
(WebCore::DocumentMarkerController::addMarker): Changed code to use the
add idiom to avoid double hash table lookup. Changed to use adoptPtr since
the map now contains OwnPtr.
(WebCore::DocumentMarkerController::removeMarkers): Removed explicit calls
to delete list since removing the entry from the map now deletes the list.
Moved the code to check if m_markers has become empty so it's only called
when we actually remove something from m_markers.
(WebCore::DocumentMarkerController::markerContainingPoint): Added get().
(WebCore::DocumentMarkerController::renderedRectsForMarkers): Added get().
(WebCore::DocumentMarkerController::removeMarkers): Changed to use a new
interface to removeMarkersFromList. This eliminated the need to copy the
entire map when removing markers; instead we can just copy the keys.
(WebCore::DocumentMarkerController::removeMarkersFromList): Changed to use
an iterator instead of being passed the key/value pair from the map. Also
rearranged the logic so there is less repeated code and removed some now-
unneeded comments.
(WebCore::DocumentMarkerController::repaintMarkers): Added get().
(WebCore::DocumentMarkerController::invalidateRenderedRectsForMarkersInRect):
Added get().
(WebCore::DocumentMarkerController::showMarkers): Added get().

  • dom/DocumentMarkerController.h: Removed implementation of the destructor

that called the detach function; that was just sharing the now-unneeded
call to deleteAllValues. Changed the type of the map to use an OwnPtr for
the value. Changed the interface of removeMarkersFromList to take a map
iterator instead of a key/value pair.

Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r149653 r149654  
     12013-05-06  Darin Adler  <darin@apple.com>
     2
     3        Use OwnPtr instead of deleteAllValues in DocumentMarkerController
     4        https://bugs.webkit.org/show_bug.cgi?id=115655
     5
     6        Reviewed by Andreas Kling.
     7
     8        * dom/DocumentMarkerController.cpp:
     9        (WebCore::DocumentMarkerController::~DocumentMarkerController): Added here
     10        so the destructor isn't implicitly inline.
     11        (WebCore::DocumentMarkerController::detach): Removed now-unneeded code to
     12        call deleteAllValues. Also moved code to set m_possiblyExistingMarkerTypes
     13        to after clearing m_markers to be consistent with how other functions do it.
     14        (WebCore::DocumentMarkerController::addMarker): Changed code to use the
     15        add idiom to avoid double hash table lookup. Changed to use adoptPtr since
     16        the map now contains OwnPtr.
     17        (WebCore::DocumentMarkerController::removeMarkers): Removed explicit calls
     18        to delete list since removing the entry from the map now deletes the list.
     19        Moved the code to check if m_markers has become empty so it's only called
     20        when we actually remove something from m_markers.
     21        (WebCore::DocumentMarkerController::markerContainingPoint): Added get().
     22        (WebCore::DocumentMarkerController::renderedRectsForMarkers): Added get().
     23        (WebCore::DocumentMarkerController::removeMarkers): Changed to use a new
     24        interface to removeMarkersFromList. This eliminated the need to copy the
     25        entire map when removing markers; instead we can just copy the keys.
     26        (WebCore::DocumentMarkerController::removeMarkersFromList): Changed to use
     27        an iterator instead of being passed the key/value pair from the map. Also
     28        rearranged the logic so there is less repeated code and removed some now-
     29        unneeded comments.
     30        (WebCore::DocumentMarkerController::repaintMarkers): Added get().
     31        (WebCore::DocumentMarkerController::invalidateRenderedRectsForMarkersInRect):
     32        Added get().
     33        (WebCore::DocumentMarkerController::showMarkers): Added get().
     34
     35        * dom/DocumentMarkerController.h: Removed implementation of the destructor
     36        that called the detach function; that was just sharing the now-unneeded
     37        call to deleteAllValues. Changed the type of the map to use an OwnPtr for
     38        the value. Changed the interface of removeMarkersFromList to take a map
     39        iterator instead of a key/value pair.
     40
    1412013-05-06  Beth Dakin  <bdakin@apple.com>
    242

  • trunk/Source/WebCore/dom/DocumentMarkerController.cpp

    r141877 r149654  
    3434#include "RenderedDocumentMarker.h"
    3535#include "TextIterator.h"
    36 
    37 #ifndef NDEBUG
    3836#include <stdio.h>
    39 #endif
    4037
    4138namespace WebCore {
    … …  
    5148}
    5249
     50DocumentMarkerController::~DocumentMarkerController()
     51{
     52}
     53
    5354void DocumentMarkerController::detach()
    5455{
     56    m_markers.clear();
    5557    m_possiblyExistingMarkerTypes = 0;
    56     if (m_markers.isEmpty())
    57         return;
    58     deleteAllValues(m_markers);
    59     m_markers.clear();
    6058}
    6159
    … …  
    135133    m_possiblyExistingMarkerTypes.add(newMarker.type());
    136134
    137     MarkerList* list = m_markers.get(node);
     135    OwnPtr<MarkerList>& list = m_markers.add(node, nullptr).iterator->value;
    138136
    139137    if (!list) {
    140         list = new MarkerList;
     138        list = adoptPtr(new MarkerList);
    141139        list->append(RenderedDocumentMarker(newMarker));
    142         m_markers.set(node, list);
    143140    } else {
    144141        RenderedDocumentMarker toInsert(newMarker);
    … …  
    287284    if (list->isEmpty()) {
    288285        m_markers.remove(node);
    289         delete list;
    290     }
    291    
    292     if (m_markers.isEmpty())
    293         m_possiblyExistingMarkerTypes = 0;
     286        if (m_markers.isEmpty())
     287            m_possiblyExistingMarkerTypes = 0;
     288    }
    294289
    295290    // repaint the affected node
    … …  
    308303    for (MarkerMap::iterator nodeIterator = m_markers.begin(); nodeIterator != end; ++nodeIterator) {
    309304        // inner loop; process each marker in this node
    310         MarkerList* list = nodeIterator->value;
     305        MarkerList* list = nodeIterator->value.get();
    311306        unsigned markerCount = list->size();
    312307        for (unsigned markerIndex = 0; markerIndex < markerCount; ++markerIndex) {
    … …  
    396391    for (MarkerMap::iterator nodeIterator = m_markers.begin(); nodeIterator != end; ++nodeIterator) {
    397392        // inner loop; process each marker in this node
    398         MarkerList* list = nodeIterator->value;
     393        MarkerList* list = nodeIterator->value.get();
    399394        unsigned markerCount = list->size();
    400395        for (unsigned markerIndex = 0; markerIndex < markerCount; ++markerIndex) {
    … …  
    423418    MarkerMap::iterator iterator = m_markers.find(node);
    424419    if (iterator != m_markers.end())
    425         removeMarkersFromList(node, iterator->value, markerTypes);
     420        removeMarkersFromList(iterator, markerTypes);
    426421}
    427422
    … …  
    432427    ASSERT(!m_markers.isEmpty());
    433428
    434     // outer loop: process each markered node in the document
    435     MarkerMap markerMapCopy = m_markers;
    436     MarkerMap::iterator end = markerMapCopy.end();
    437     for (MarkerMap::iterator i = markerMapCopy.begin(); i != end; ++i)
    438         removeMarkersFromList(i->key.get(), i->value, markerTypes);
     429    Vector<RefPtr<Node> > nodesWithMarkers;
     430    copyKeysToVector(m_markers, nodesWithMarkers);
     431    unsigned size = nodesWithMarkers.size();
     432    for (unsigned i = 0; i < size; ++i) {
     433        MarkerMap::iterator iterator = m_markers.find(nodesWithMarkers[i]);
     434        if (iterator != m_markers.end())
     435            removeMarkersFromList(iterator, markerTypes);
     436    }
     437
    439438    m_possiblyExistingMarkerTypes.remove(markerTypes);
    440439}
    441440
    442 // This function may release node and vectorPair.
    443 void DocumentMarkerController::removeMarkersFromList(Node* node, MarkerList* list, DocumentMarker::MarkerTypes markerTypes)
    444 {
     441void DocumentMarkerController::removeMarkersFromList(MarkerMap::iterator iterator, DocumentMarker::MarkerTypes markerTypes)
     442{
     443    bool needsRepainting = false;
     444    bool listCanBeRemoved;
     445
    445446    if (markerTypes == DocumentMarker::AllMarkers()) {
    446         delete list;
    447         m_markers.remove(node);
    448         if (RenderObject* renderer = node->renderer())
    449             renderer->repaint();
     447        needsRepainting = true;
     448        listCanBeRemoved = true;
    450449    } else {
    451         bool needsRepaint = false;
    452         for (size_t i = 0; i != list->size();) {
     450        MarkerList* list = iterator->value.get();
     451
     452        for (size_t i = 0; i != list->size(); ) {
    453453            DocumentMarker marker = list->at(i);
    454454
    … …  
    461461            // pitch the old marker
    462462            list->remove(i);
    463             needsRepaint = true;
     463            needsRepainting = true;
    464464            // i now is the index of the next marker
    465465        }
    466466
    467         // Redraw the node if it changed. Do this before the node is removed from m_markers, since
    468         // m_markers might contain the last reference to the node.
    469         if (needsRepaint) {
    470             RenderObject* renderer = node->renderer();
    471             if (renderer)
    472                 renderer->repaint();
    473         }
    474 
    475         // delete the node's list if it is now empty
    476         if (list->isEmpty()) {
    477             m_markers.remove(node);
    478             delete list;
    479         }
    480     }
    481     if (m_markers.isEmpty())
    482         m_possiblyExistingMarkerTypes = 0;
     467        listCanBeRemoved = list->isEmpty();
     468    }
     469
     470    if (needsRepainting) {
     471        if (RenderObject* renderer = iterator->key->renderer())
     472            renderer->repaint();
     473    }
     474
     475    if (listCanBeRemoved) {
     476        m_markers.remove(iterator);
     477        if (m_markers.isEmpty())
     478            m_possiblyExistingMarkerTypes = 0;
     479    }
    483480}
    484481
    … …  
    495492
    496493        // inner loop: process each marker in the current node
    497         MarkerList* list = i->value;
     494        MarkerList* list = i->value.get();
    498495        bool nodeNeedsRepaint = false;
    499496        for (size_t i = 0; i != list->size(); ++i) {
    … …  
    523520
    524521        // inner loop: process each rect in the current node
    525         MarkerList* list = i->value;
     522        MarkerList* list = i->value.get();
    526523        for (size_t listIndex = 0; listIndex < list->size(); ++listIndex)
    527524            list->at(listIndex).invalidate(r);
    … …  
    674671        Node* node = nodeIterator->key.get();
    675672        fprintf(stderr, "%p", node);
    676         MarkerList* list = nodeIterator->value;
     673        MarkerList* list = nodeIterator->value.get();
    677674        for (unsigned markerIndex = 0; markerIndex < list->size(); ++markerIndex) {
    678675            const DocumentMarker& marker = list->at(markerIndex);
    … …  
    687684} // namespace WebCore
    688685
    689 
    690686#ifndef NDEBUG
    691687void showDocumentMarkers(const WebCore::DocumentMarkerController* controller)

  • trunk/Source/WebCore/dom/DocumentMarkerController.h

    r133779 r149654  
    4646
    4747    DocumentMarkerController();
    48     ~DocumentMarkerController() { detach(); }
     48    ~DocumentMarkerController();
    4949
    5050    void detach();
    … …  
    8888
    8989    typedef Vector<RenderedDocumentMarker> MarkerList;
    90     typedef HashMap<RefPtr<Node>, MarkerList*> MarkerMap;
     90    typedef HashMap<RefPtr<Node>, OwnPtr<MarkerList> > MarkerMap;
    9191    bool possiblyHasMarkers(DocumentMarker::MarkerTypes);
    92     void removeMarkersFromList(Node*, MarkerList*, DocumentMarker::MarkerTypes);
     92    void removeMarkersFromList(MarkerMap::iterator, DocumentMarker::MarkerTypes);
    9393
    9494    MarkerMap m_markers;
Note: See TracChangeset for help on using the changeset viewer.