2011-04-07 MORITA Hajime <morrita@google.com>

omo · omo · commit bd0f9808c895 · 2011-04-15T17:57:06.000Z
Reviewed by Ryosuke Niwa. DocumentMaker::AllMarkers should not be a part of DocumentMarker::MarkerType https://bugs.webkit.org/show_bug.cgi?id=58112 - Converted DocumentMarker::MarkerTypes from unsigned int to a class. - Converted DocumentMarker::AllMarkers from an enum entry to a subclass of MarkerTypes. - Changed type of some MarkerType argument on DocumentMarkerController API to MarkerTypes which should allow a combination of MarkerType constansts. - Removed some MarkerType arguments on DocumentMarkerController API which only received AllMarkers. No new tests, no behavior change. * WebCore.exp.in: * dom/DocumentMarker.h: (WebCore::DocumentMarker::MarkerTypes::MarkerTypes): (WebCore::DocumentMarker::MarkerTypes::contains): (WebCore::DocumentMarker::MarkerTypes::intersects): (WebCore::DocumentMarker::MarkerTypes::operator==): (WebCore::DocumentMarker::MarkerTypes::add): (WebCore::DocumentMarker::MarkerTypes::remove): (WebCore::DocumentMarker::AllMarkers::AllMarkers): * dom/DocumentMarkerController.cpp: (WebCore::DocumentMarkerController::possiblyHasMarkers): (WebCore::DocumentMarkerController::addMarker): (WebCore::DocumentMarkerController::copyMarkers): (WebCore::DocumentMarkerController::removeMarkers): (WebCore::DocumentMarkerController::markerContainingPoint): (WebCore::DocumentMarkerController::markersInRange): (WebCore::DocumentMarkerController::renderedRectsForMarkers): (WebCore::DocumentMarkerController::removeMarkersFromMarkerMapVectorPair): (WebCore::DocumentMarkerController::repaintMarkers): (WebCore::DocumentMarkerController::shiftMarkers): (WebCore::DocumentMarkerController::setMarkersActive): (WebCore::DocumentMarkerController::hasMarkers): (WebCore::DocumentMarkerController::clearDescriptionOnMarkersIntersectingRange): * dom/DocumentMarkerController.h: Canonical link: https://commits.webkit.org/73745@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@83991 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,44 @@
+2011-04-07  MORITA Hajime  <morrita@google.com>
+
+        Reviewed by Ryosuke Niwa.
+
+        DocumentMaker::AllMarkers should not be a part of DocumentMarker::MarkerType
+        https://bugs.webkit.org/show_bug.cgi?id=58112
+        
+        - Converted DocumentMarker::MarkerTypes from unsigned int to a class.
+        - Converted DocumentMarker::AllMarkers from an enum entry to a subclass of MarkerTypes.
+        - Changed type of some MarkerType argument on DocumentMarkerController API to MarkerTypes
+          which should allow a combination of MarkerType constansts.
+        - Removed some MarkerType arguments on DocumentMarkerController API
+          which only received AllMarkers.
+        
+        No new tests, no behavior change.
+
+        * WebCore.exp.in:
+        * dom/DocumentMarker.h:
+        (WebCore::DocumentMarker::MarkerTypes::MarkerTypes):
+        (WebCore::DocumentMarker::MarkerTypes::contains):
+        (WebCore::DocumentMarker::MarkerTypes::intersects):
+        (WebCore::DocumentMarker::MarkerTypes::operator==):
+        (WebCore::DocumentMarker::MarkerTypes::add):
+        (WebCore::DocumentMarker::MarkerTypes::remove):
+        (WebCore::DocumentMarker::AllMarkers::AllMarkers):
+        * dom/DocumentMarkerController.cpp:
+        (WebCore::DocumentMarkerController::possiblyHasMarkers):
+        (WebCore::DocumentMarkerController::addMarker):
+        (WebCore::DocumentMarkerController::copyMarkers):
+        (WebCore::DocumentMarkerController::removeMarkers):
+        (WebCore::DocumentMarkerController::markerContainingPoint):
+        (WebCore::DocumentMarkerController::markersInRange):
+        (WebCore::DocumentMarkerController::renderedRectsForMarkers):
+        (WebCore::DocumentMarkerController::removeMarkersFromMarkerMapVectorPair):
+        (WebCore::DocumentMarkerController::repaintMarkers):
+        (WebCore::DocumentMarkerController::shiftMarkers):
+        (WebCore::DocumentMarkerController::setMarkersActive):
+        (WebCore::DocumentMarkerController::hasMarkers):
+        (WebCore::DocumentMarkerController::clearDescriptionOnMarkersIntersectingRange):
+        * dom/DocumentMarkerController.h:
+
 2011-04-14  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Oliver Hunt.
diff --git a/Source/WebCore/WebCore.exp.in b/Source/WebCore/WebCore.exp.in
@@ -573,7 +573,7 @@ __ZN7WebCore23getHostnamesWithCookiesERN3WTF7HashSetINS0_6StringENS0_10StringHas
 __ZN7WebCore23overrideDefaultLanguageERKN3WTF6StringE
 __ZN7WebCore24BinaryPropertyListWriter17writePropertyListEv
 __ZN7WebCore24CachedResourceHandleBase11setResourceEPNS_14CachedResourceE
-__ZN7WebCore24DocumentMarkerController13removeMarkersEj
+__ZN7WebCore24DocumentMarkerController13removeMarkersENS_14DocumentMarker11MarkerTypesE
 __ZN7WebCore24DocumentMarkerController14markersForNodeEPNS_4NodeE
 __ZN7WebCore24DocumentMarkerController14markersForNodeEPNS_4NodeE
 __ZN7WebCore24DocumentMarkerController23renderedRectsForMarkersENS_14DocumentMarker10MarkerTypeE
diff --git a/Source/WebCore/dom/DocumentMarker.h b/Source/WebCore/dom/DocumentMarker.h
@@ -53,10 +53,33 @@ struct DocumentMarker {
         Autocorrected = 1 << 6,
         // On some platforms, this prevents the text from being spellchecked again.
         SpellCheckingExemption = 1 << 7,
-        AllMarkers = Spelling | Grammar | TextMatch | Replacement | CorrectionIndicator | RejectedCorrection | Autocorrected | SpellCheckingExemption
     };
+
+    class MarkerTypes {
+    public:
+        // The constructor is intentionally implicit to allow conversion from the bit-wise sum of above types
+        MarkerTypes(unsigned mask) : m_mask(mask) { }
+
+        bool contains(MarkerType type) const { return m_mask & type; }
+        bool intersects(const MarkerTypes& types) const { return (m_mask & types.m_mask); }
+        bool operator==(const MarkerTypes& other) const { return m_mask == other.m_mask; }
+
+        void add(const MarkerTypes& types) { m_mask |= types.m_mask; }
+        void remove(const MarkerTypes& types) { m_mask &= ~types.m_mask; }
+
+    private:
+        unsigned m_mask;
+    };
+
+    class AllMarkers : public MarkerTypes {
+    public:
+        AllMarkers()
+            : MarkerTypes(Spelling | Grammar | TextMatch | Replacement | CorrectionIndicator | RejectedCorrection | Autocorrected | SpellCheckingExemption)
+        {
+        }
+    };
+    
     MarkerType type;
-    typedef unsigned MarkerTypes;
     unsigned startOffset;
     unsigned endOffset;
     String description;
diff --git a/Source/WebCore/dom/DocumentMarkerController.cpp b/Source/WebCore/dom/DocumentMarkerController.cpp
@@ -40,7 +40,7 @@ static IntRect placeholderRectForMarker()
 
 inline bool DocumentMarkerController::possiblyHasMarkers(DocumentMarker::MarkerTypes types)
 {
-    return m_possiblyExistingMarkerTypes & types;
+    return m_possiblyExistingMarkerTypes.intersects(types);
 }
 
 DocumentMarkerController::DocumentMarkerController()
@@ -91,7 +91,7 @@ void DocumentMarkerController::addMarker(Node* node, DocumentMarker newMarker)
     if (newMarker.endOffset == newMarker.startOffset)
         return;
 
-    m_possiblyExistingMarkerTypes |= newMarker.type;
+    m_possiblyExistingMarkerTypes.add(newMarker.type);
 
     MarkerMapVectorPair* vectorPair = m_markers.get(node);
 
@@ -152,12 +152,12 @@ void DocumentMarkerController::addMarker(Node* node, DocumentMarker newMarker)
 
 // copies markers from srcNode to dstNode, applying the specified shift delta to the copies.  The shift is
 // useful if, e.g., the caller has created the dstNode from a non-prefix substring of the srcNode.
-void DocumentMarkerController::copyMarkers(Node* srcNode, unsigned startOffset, int length, Node* dstNode, int delta, DocumentMarker::MarkerType markerType)
+void DocumentMarkerController::copyMarkers(Node* srcNode, unsigned startOffset, int length, Node* dstNode, int delta)
 {
     if (length <= 0)
         return;
 
-    if (!possiblyHasMarkers(markerType))
+    if (!possiblyHasMarkers(DocumentMarker::AllMarkers()))
         return;
     ASSERT(!m_markers.isEmpty());
 
@@ -178,7 +178,7 @@ void DocumentMarkerController::copyMarkers(Node* srcNode, unsigned startOffset,
             break;
 
         // skip marker that is before the specified range or is the wrong type
-        if (marker.endOffset < startOffset || (marker.type != markerType && markerType != DocumentMarker::AllMarkers))
+        if (marker.endOffset < startOffset)
             continue;
 
         // pin the marker to the specified range and apply the shift delta
@@ -224,7 +224,7 @@ void DocumentMarkerController::removeMarkers(Node* node, unsigned startOffset, i
             break;
 
         // skip marker that is wrong type or before target
-        if (marker.endOffset <= startOffset || !(marker.type & markerTypes)) {
+        if (marker.endOffset <= startOffset || !markerTypes.contains(marker.type)) {
             i++;
             continue;
         }
@@ -292,7 +292,7 @@ DocumentMarker* DocumentMarkerController::markerContainingPoint(const IntPoint&
             DocumentMarker& marker = markers[markerIndex];
 
             // skip marker that is wrong type
-            if (marker.type != markerType && markerType != DocumentMarker::AllMarkers)
+            if (marker.type != markerType)
                 continue;
 
             IntRect& r = rects[markerIndex];
@@ -334,7 +334,7 @@ Vector<DocumentMarker> DocumentMarkerController::markersInRange(Range* range, Do
         Vector<DocumentMarker> markers = markersForNode(node);
         Vector<DocumentMarker>::const_iterator end = markers.end();
         for (Vector<DocumentMarker>::const_iterator it = markers.begin(); it != end; ++it) {
-            if (!(markerType & it->type))
+            if (markerType != it->type)
                 continue;
             if (node == startContainer && it->endOffset <= static_cast<unsigned>(range->startOffset()))
                 continue;
@@ -367,7 +367,7 @@ Vector<IntRect> DocumentMarkerController::renderedRectsForMarkers(DocumentMarker
             DocumentMarker marker = markers[markerIndex];
 
             // skip marker that is wrong type
-            if (marker.type != markerType && markerType != DocumentMarker::AllMarkers)
+            if (marker.type != markerType)
                 continue;
 
             IntRect r = rects[markerIndex];
@@ -408,13 +408,13 @@ void DocumentMarkerController::removeMarkers(DocumentMarker::MarkerTypes markerT
         removeMarkersFromMarkerMapVectorPair(node, vectorPair, markerTypes);
     }
 
-    m_possiblyExistingMarkerTypes &= ~markerTypes;
+    m_possiblyExistingMarkerTypes.remove(markerTypes);
 }
 
 // This function may release node and vectorPair.
 void DocumentMarkerController::removeMarkersFromMarkerMapVectorPair(Node* node, MarkerMapVectorPair* vectorPair, DocumentMarker::MarkerTypes markerTypes)
 {
-    if (!~(markerTypes & DocumentMarker::AllMarkers)) {
+    if (markerTypes == DocumentMarker::AllMarkers()) {
         delete vectorPair;
         m_markers.remove(node);
         if (RenderObject* renderer = node->renderer())
@@ -428,7 +428,7 @@ void DocumentMarkerController::removeMarkersFromMarkerMapVectorPair(Node* node,
             DocumentMarker marker = markers[i];
 
             // skip nodes that are not of the specified type
-            if (!(marker.type & markerTypes)) {
+            if (!markerTypes.contains(marker.type)) {
                 ++i;
                 continue;
             }
@@ -459,9 +459,9 @@ void DocumentMarkerController::removeMarkersFromMarkerMapVectorPair(Node* node,
         m_possiblyExistingMarkerTypes = 0;
 }
 
-void DocumentMarkerController::repaintMarkers(DocumentMarker::MarkerType markerType)
+void DocumentMarkerController::repaintMarkers(DocumentMarker::MarkerTypes markerTypes)
 {
-    if (!possiblyHasMarkers(markerType))
+    if (!possiblyHasMarkers(markerTypes))
         return;
     ASSERT(!m_markers.isEmpty());
 
@@ -478,7 +478,7 @@ void DocumentMarkerController::repaintMarkers(DocumentMarker::MarkerType markerT
             DocumentMarker marker = markers[i];
 
             // skip nodes that are not of the specified type
-            if (marker.type == markerType || markerType == DocumentMarker::AllMarkers) {
+            if (markerTypes.contains(marker.type)) {
                 nodeNeedsRepaint = true;
                 break;
             }
@@ -532,9 +532,9 @@ void DocumentMarkerController::invalidateRenderedRectsForMarkersInRect(const Int
     }
 }
 
-void DocumentMarkerController::shiftMarkers(Node* node, unsigned startOffset, int delta, DocumentMarker::MarkerType markerType)
+void DocumentMarkerController::shiftMarkers(Node* node, unsigned startOffset, int delta)
 {
-    if (!possiblyHasMarkers(markerType))
+    if (!possiblyHasMarkers(DocumentMarker::AllMarkers()))
         return;
     ASSERT(!m_markers.isEmpty());
 
@@ -549,7 +549,7 @@ void DocumentMarkerController::shiftMarkers(Node* node, unsigned startOffset, in
     bool docDirty = false;
     for (size_t i = 0; i != markers.size(); ++i) {
         DocumentMarker& marker = markers[i];
-        if (marker.startOffset >= startOffset && (markerType == DocumentMarker::AllMarkers || marker.type == markerType)) {
+        if (marker.startOffset >= startOffset) {
             ASSERT((int)marker.startOffset + delta >= 0);
             marker.startOffset += delta;
             marker.endOffset += delta;
@@ -567,7 +567,7 @@ void DocumentMarkerController::shiftMarkers(Node* node, unsigned startOffset, in
 
 void DocumentMarkerController::setMarkersActive(Range* range, bool active)
 {
-    if (!possiblyHasMarkers(DocumentMarker::AllMarkers))
+    if (!possiblyHasMarkers(DocumentMarker::AllMarkers()))
         return;
     ASSERT(!m_markers.isEmpty());
 
@@ -629,7 +629,7 @@ bool DocumentMarkerController::hasMarkers(Range* range, DocumentMarker::MarkerTy
         Vector<DocumentMarker> markers = markersForNode(node);
         Vector<DocumentMarker>::const_iterator end = markers.end();
         for (Vector<DocumentMarker>::const_iterator it = markers.begin(); it != end; ++it) {
-            if (!(markerTypes & it->type))
+            if (!markerTypes.contains(it->type))
                 continue;
             if (node == startContainer && it->endOffset <= static_cast<unsigned>(range->startOffset()))
                 continue;
@@ -667,7 +667,7 @@ void DocumentMarkerController::clearDescriptionOnMarkersIntersectingRange(Range*
                 break;
 
             // skip marker that is wrong type or before target
-            if (marker.endOffset <= startOffset || !(marker.type & markerTypes)) {
+            if (marker.endOffset <= startOffset || !markerTypes.contains(marker.type)) {
                 i++;
                 continue;
             }
diff --git a/Source/WebCore/dom/DocumentMarkerController.h b/Source/WebCore/dom/DocumentMarkerController.h
@@ -47,29 +47,29 @@ class DocumentMarkerController {
     void detach();
     void addMarker(Range*, DocumentMarker::MarkerType, String description = String());
     void addMarker(Node*, DocumentMarker);
-    void copyMarkers(Node* srcNode, unsigned startOffset, int length, Node* dstNode, int delta, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
-    bool hasMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers);
+    void copyMarkers(Node* srcNode, unsigned startOffset, int length, Node* dstNode, int delta);
+    bool hasMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
 
     // When a marker partially overlaps with range, if removePartiallyOverlappingMarkers is true, we completely
     // remove the marker. If the argument is false, we will adjust the span of the marker so that it retains
     // the portion that is outside of the range.
     enum RemovePartiallyOverlappingMarkerOrNot { DoNotRemovePartiallyOverlappingMarker, RemovePartiallyOverlappingMarker };
-    void removeMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers, RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
-    void removeMarkers(Node*, unsigned startOffset, int length, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers,  RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
+    void removeMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(), RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
+    void removeMarkers(Node*, unsigned startOffset, int length, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers(),  RemovePartiallyOverlappingMarkerOrNot = DoNotRemovePartiallyOverlappingMarker);
 
-    void removeMarkers(DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers);
-    void removeMarkers(Node*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers);
-    void repaintMarkers(DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
+    void removeMarkers(DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
+    void removeMarkers(Node*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
+    void repaintMarkers(DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
     void setRenderedRectForMarker(Node*, const DocumentMarker&, const IntRect&);
     void invalidateRenderedRectsForMarkersInRect(const IntRect&);
-    void shiftMarkers(Node*, unsigned startOffset, int delta, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
+    void shiftMarkers(Node*, unsigned startOffset, int delta);
     void setMarkersActive(Range*, bool);
     void setMarkersActive(Node*, unsigned startOffset, unsigned endOffset, bool);
 
-    DocumentMarker* markerContainingPoint(const IntPoint&, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
+    DocumentMarker* markerContainingPoint(const IntPoint&, DocumentMarker::MarkerType);
     Vector<DocumentMarker> markersForNode(Node*);
     Vector<DocumentMarker> markersInRange(Range*, DocumentMarker::MarkerType);
-    Vector<IntRect> renderedRectsForMarkers(DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
+    Vector<IntRect> renderedRectsForMarkers(DocumentMarker::MarkerType);
     void clearDescriptionOnMarkersIntersectingRange(Range*, DocumentMarker::MarkerTypes);
 
 #ifndef NDEBUG
