Attachment 93860 Details for Bug 56814 – Patch

[patch] Patch

bug-56814-20110518122221.patch (text/plain), 38.52 KB, created by Hajime Morrita on 2011-05-17 20:22:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Hajime Morrita

Created: 2011-05-17 20:22:23 PDT

Size: 38.52 KB

patch

obsolete

>Subversion Revision: 86725
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 13c080681d6ed00230452dd71c11d730d4402594..aed158094f4ae53ca0f931c01b42276542386f2e 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,69 @@
>+2011-05-17  MORITA Hajime  <morrita@google.com>
>+
>+        Reviewed by Tony Chang.
>+
>+        [Refactoring] Member variables of DocumentMarker should be encapsulated.
>+        https://bugs.webkit.org/show_bug.cgi?id=56814
>+
>+        - Moved DocumentMarker's member variables to private and added getters for them.
>+        - Added DocumentMarker setters and constructors, which contain assertions against m_type values
>+          because description and activeMatch are used with specific type of MarkerType.
>+        - Moved chromium's WebKit::WebFrameImpl::addMarker() to DocumentMarkerController::addTextMatchMarker()
>+          because it accesses DocumentMarker internals.
>+        - Moved a version of DMC::addMarker() to private and add alternatives that hide
>+          internals of DocumentMarker. (The internal will be renewed by upcoming change.)
>+        
>+        * dom/DocumentMarker.h:
>+        (WebCore::DocumentMarker::type):
>+        (WebCore::DocumentMarker::startOffset):
>+        (WebCore::DocumentMarker::endOffset):
>+        (WebCore::DocumentMarker::description):
>+        (WebCore::DocumentMarker::hasDescription):
>+        (WebCore::DocumentMarker::activeMatch):
>+        (WebCore::DocumentMarker::clearDescription):
>+        (WebCore::DocumentMarker::setStartOffset):
>+        (WebCore::DocumentMarker::setEndOffset):
>+        (WebCore::DocumentMarker::operator==):
>+        (WebCore::DocumentMarker::DocumentMarker):
>+        (WebCore::DocumentMarker::shiftOffsets):
>+        (WebCore::DocumentMarker::setActiveMatch):
>+        * dom/DocumentMarkerController.cpp:
>+        (WebCore::DocumentMarkerController::addMarker):
>+        (WebCore::DocumentMarkerController::addTextMatchMarker):
>+        (WebCore::DocumentMarkerController::copyMarkers):
>+        (WebCore::DocumentMarkerController::removeMarkers):
>+        (WebCore::DocumentMarkerController::markerContainingPoint):
>+        (WebCore::DocumentMarkerController::markersInRange):
>+        (WebCore::DocumentMarkerController::renderedRectsForMarkers):
>+        (WebCore::DocumentMarkerController::removeMarkersFromList):
>+        (WebCore::DocumentMarkerController::repaintMarkers):
>+        (WebCore::DocumentMarkerController::shiftMarkers):
>+        (WebCore::DocumentMarkerController::setMarkersActive):
>+        (WebCore::DocumentMarkerController::hasMarkers):
>+        (WebCore::DocumentMarkerController::clearDescriptionOnMarkersIntersectingRange):
>+        (WebCore::DocumentMarkerController::showMarkers):
>+        * dom/DocumentMarkerController.h:
>+        * editing/CompositeEditCommand.cpp:
>+        (WebCore::CompositeEditCommand::replaceTextInNodePreservingMarkers):
>+        * editing/DeleteSelectionCommand.cpp:
>+        (WebCore::DeleteSelectionCommand::originalStringForAutocorrectionAtBeginningOfSelection):
>+        * editing/Editor.cpp:
>+        (WebCore::Editor::selectionStartHasMarkerFor):
>+        * editing/SpellingCorrectionController.cpp:
>+        (WebCore::SpellingCorrectionController::respondToChangedSelection):
>+        * editing/SpellingCorrectionController.h:
>+        (WebCore::SpellingCorrectionController::shouldStartTimerFor):
>+        * rendering/HitTestResult.cpp:
>+        (WebCore::HitTestResult::spellingToolTip):
>+        (WebCore::HitTestResult::replacedString):
>+        * rendering/InlineTextBox.cpp:
>+        (WebCore::InlineTextBox::paintSpellingOrGrammarMarker):
>+        (WebCore::InlineTextBox::paintTextMatchMarker):
>+        (WebCore::InlineTextBox::computeRectForReplacementMarker):
>+        (WebCore::InlineTextBox::paintDocumentMarkers):
>+        * rendering/svg/SVGInlineFlowBox.cpp:
>+        (WebCore::SVGInlineFlowBox::computeTextMatchMarkerRectForRenderer):
>+
> 2011-05-17  Cris Neckar  <cdn@chromium.org>
> 
>         Reviewed by Adam Barth.
>diff --git a/Source/WebCore/dom/DocumentMarker.h b/Source/WebCore/dom/DocumentMarker.h
>index c8778d3467de761ed6ed9b2add48873cb8d9b2da..077ac6fb7b97ee6917ed0ae5133e4733de153902 100644
>--- a/Source/WebCore/dom/DocumentMarker.h
>+++ b/Source/WebCore/dom/DocumentMarker.h
>@@ -84,24 +84,80 @@ struct DocumentMarker {
>         {
>         }
>     };
>-    
>-    MarkerType type;
>-    unsigned startOffset;
>-    unsigned endOffset;
>-    String description;
>-    bool activeMatch;
>+
>+
>+    DocumentMarker();
>+    DocumentMarker(MarkerType, unsigned startOffset, unsigned endOffset);
>+    DocumentMarker(MarkerType, unsigned startOffset, unsigned endOffset, const String& description);
>+    DocumentMarker(unsigned startOffset, unsigned endOffset, bool activeMatch);
>+
>+    MarkerType type() const { return m_type; }
>+    unsigned startOffset() const { return m_startOffset; }
>+    unsigned endOffset() const { return m_endOffset; }
>+    const String& description() const { return m_description; }
>+    bool hasDescription() const { return !m_description.isEmpty(); }
>+    bool activeMatch() const { return m_activeMatch; }
>+
>+    void setActiveMatch(bool);
>+    void clearDescription() { m_description = String(); }
>+
>+    // Offset modifications are done by DocumentMarkerController.
>+    // Other classes should not call following setters.
>+    void setStartOffset(unsigned offset) { m_startOffset = offset; }
>+    void setEndOffset(unsigned offset) { m_endOffset = offset; }
>+    void shiftOffsets(int delta);
> 
>     bool operator==(const DocumentMarker& o) const
>     {
>-        return type == o.type && startOffset == o.startOffset && endOffset == o.endOffset;
>+        return type() == o.type() && startOffset() == o.startOffset() && endOffset() == o.endOffset();
>     }
> 
>     bool operator!=(const DocumentMarker& o) const
>     {
>         return !(*this == o);
>     }
>+
>+private:    
>+    MarkerType m_type;
>+    unsigned m_startOffset;
>+    unsigned m_endOffset;
>+    String m_description;
>+    bool m_activeMatch;
> };
> 
>+inline DocumentMarker::DocumentMarker() 
>+    : m_type(Spelling), m_startOffset(0), m_endOffset(0), m_activeMatch(false)
>+{
>+}
>+
>+inline DocumentMarker::DocumentMarker(MarkerType type, unsigned startOffset, unsigned endOffset)
>+    : m_type(type), m_startOffset(startOffset), m_endOffset(endOffset), m_activeMatch(false)
>+{
>+}
>+
>+inline DocumentMarker::DocumentMarker(MarkerType type, unsigned startOffset, unsigned endOffset, const String& description)
>+    : m_type(type), m_startOffset(startOffset), m_endOffset(endOffset), m_description(description), m_activeMatch(false)
>+{
>+    ASSERT(type == DocumentMarker::Grammar || DocumentMarker::Autocorrected);
>+}
>+
>+inline DocumentMarker::DocumentMarker(unsigned startOffset, unsigned endOffset, bool activeMatch)
>+    : m_type(DocumentMarker::TextMatch), m_startOffset(startOffset), m_endOffset(endOffset), m_activeMatch(activeMatch)
>+{
>+}
>+
>+inline void DocumentMarker::shiftOffsets(int delta)
>+{
>+    m_startOffset += delta;
>+    m_endOffset +=  delta;
>+}
>+
>+inline void DocumentMarker::setActiveMatch(bool active)
>+{
>+    ASSERT(m_type == DocumentMarker::TextMatch);
>+    m_activeMatch = active;
>+}
>+
> } // namespace WebCore
> 
> #endif // DocumentMarker_h
>diff --git a/Source/WebCore/dom/DocumentMarkerController.cpp b/Source/WebCore/dom/DocumentMarkerController.cpp
>index 74d727df8d6507b3fc5c487f3230b7fd4da34210..19334994634307fbee2049b0839872f5337721c0 100644
>--- a/Source/WebCore/dom/DocumentMarkerController.cpp
>+++ b/Source/WebCore/dom/DocumentMarkerController.cpp
>@@ -54,14 +54,50 @@ void DocumentMarkerController::detach()
>     m_markers.clear();
> }
> 
>-void DocumentMarkerController::addMarker(Range* range, DocumentMarker::MarkerType type, String description)
>+void DocumentMarkerController::addMarker(Range* range, DocumentMarker::MarkerType type, const String& description)
> {
>     // Use a TextIterator to visit the potentially multiple nodes the range covers.
>     for (TextIterator markedText(range); !markedText.atEnd(); markedText.advance()) {
>         RefPtr<Range> textPiece = markedText.range();
>         int exception = 0;
>-        DocumentMarker marker = {type, textPiece->startOffset(exception), textPiece->endOffset(exception), description, false};
>-        addMarker(textPiece->startContainer(exception), marker);
>+        addMarker(textPiece->startContainer(exception), 
>+                  DocumentMarker(type, textPiece->startOffset(exception), textPiece->endOffset(exception), description));
>+    }
>+}
>+
>+void DocumentMarkerController::addMarker(Range* range, DocumentMarker::MarkerType type)
>+{
>+    // Use a TextIterator to visit the potentially multiple nodes the range covers.
>+    for (TextIterator markedText(range); !markedText.atEnd(); markedText.advance()) {
>+        RefPtr<Range> textPiece = markedText.range();
>+        int exception = 0;
>+        addMarker(textPiece->startContainer(exception), 
>+                  DocumentMarker(type, textPiece->startOffset(exception), textPiece->endOffset(exception)));
>+    }
>+
>+}
>+
>+
>+void DocumentMarkerController::addTextMatchMarker(Range* range, bool activeMatch)
>+{
>+    // Use a TextIterator to visit the potentially multiple nodes the range covers.
>+    for (TextIterator markedText(range); !markedText.atEnd(); markedText.advance()) {
>+        RefPtr<Range> textPiece = markedText.range();
>+        int exception = 0;
>+        unsigned startOffset = textPiece->startOffset(exception);
>+        unsigned endOffset = textPiece->endOffset(exception);
>+        addMarker(textPiece->startContainer(exception), DocumentMarker(startOffset, endOffset, activeMatch));
>+        if (endOffset > startOffset) {
>+            // Rendered rects for markers in WebKit are not populated until each time
>+            // the markers are painted. However, we need it to happen sooner, because
>+            // the whole purpose of tickmarks on the scrollbar is to show where
>+            // matches off-screen are (that haven't been painted yet).
>+            Node* node = textPiece->startContainer(exception);
>+            Vector<DocumentMarker> markers = markersForNode(node);
>+            setRenderedRectForMarker(textPiece->startContainer(exception),
>+                                     markers[markers.size() - 1],
>+                                     range->boundingBox());
>+        }
>     }
> }
> 
>@@ -84,11 +120,11 @@ void DocumentMarkerController::removeMarkers(Range* range, DocumentMarker::Marke
> 
> void DocumentMarkerController::addMarker(Node* node, const DocumentMarker& newMarker) 
> {
>-    ASSERT(newMarker.endOffset >= newMarker.startOffset);
>-    if (newMarker.endOffset == newMarker.startOffset)
>+    ASSERT(newMarker.endOffset() >= newMarker.startOffset());
>+    if (newMarker.endOffset() == newMarker.startOffset())
>         return;
> 
>-    m_possiblyExistingMarkerTypes.add(newMarker.type);
>+    m_possiblyExistingMarkerTypes.add(newMarker.type());
> 
>     MarkerList* list = m_markers.get(node);
> 
>@@ -105,10 +141,10 @@ void DocumentMarkerController::addMarker(Node* node, const DocumentMarker& newMa
>         // (there is at most one), remove it and adjust the new marker's start offset to encompass it.
>         for (i = 0; i < numMarkers; ++i) {
>             DocumentMarker marker = list->at(i);
>-            if (marker.startOffset > toInsert.startOffset)
>+            if (marker.startOffset() > toInsert.startOffset())
>                 break;
>-            if (marker.type == toInsert.type && marker.endOffset >= toInsert.startOffset) {
>-                toInsert.startOffset = marker.startOffset;
>+            if (marker.type() == toInsert.type() && marker.endOffset() >= toInsert.startOffset()) {
>+                toInsert.setStartOffset(marker.startOffset());
>                 list->remove(i);
>                 numMarkers--;
>                 break;
>@@ -120,12 +156,12 @@ void DocumentMarkerController::addMarker(Node* node, const DocumentMarker& newMa
>         // adjusting the new marker's end offset to cover them if necessary.
>         while (j < numMarkers) {
>             DocumentMarker marker = list->at(j);
>-            if (marker.startOffset > toInsert.endOffset)
>+            if (marker.startOffset() > toInsert.endOffset())
>                 break;
>-            if (marker.type == toInsert.type) {
>+            if (marker.type() == toInsert.type()) {
>                 list->remove(j);
>-                if (toInsert.endOffset <= marker.endOffset) {
>-                    toInsert.endOffset = marker.endOffset;
>+                if (toInsert.endOffset() <= marker.endOffset()) {
>+                    toInsert.setEndOffset(marker.endOffset());
>                     break;
>                 }
>                 numMarkers--;
>@@ -162,21 +198,20 @@ void DocumentMarkerController::copyMarkers(Node* srcNode, unsigned startOffset,
>         DocumentMarker marker = list->at(i);
> 
>         // stop if we are now past the specified range
>-        if (marker.startOffset > endOffset)
>+        if (marker.startOffset() > endOffset)
>             break;
> 
>         // skip marker that is before the specified range or is the wrong type
>-        if (marker.endOffset < startOffset)
>+        if (marker.endOffset() < startOffset)
>             continue;
> 
>         // pin the marker to the specified range and apply the shift delta
>         docDirty = true;
>-        if (marker.startOffset < startOffset)
>-            marker.startOffset = startOffset;
>-        if (marker.endOffset > endOffset)
>-            marker.endOffset = endOffset;
>-        marker.startOffset += delta;
>-        marker.endOffset += delta;
>+        if (marker.startOffset() < startOffset)
>+            marker.setStartOffset(startOffset);
>+        if (marker.endOffset() > endOffset)
>+            marker.setEndOffset(endOffset);
>+        marker.shiftOffsets(delta);
> 
>         addMarker(dstNode, marker);
>     }
>@@ -205,11 +240,11 @@ void DocumentMarkerController::removeMarkers(Node* node, unsigned startOffset, i
>         DocumentMarker marker = list->at(i);
> 
>         // markers are returned in order, so stop if we are now past the specified range
>-        if (marker.startOffset >= endOffset)
>+        if (marker.startOffset() >= endOffset)
>             break;
> 
>         // skip marker that is wrong type or before target
>-        if (marker.endOffset <= startOffset || !markerTypes.contains(marker.type)) {
>+        if (marker.endOffset() <= startOffset || !markerTypes.contains(marker.type())) {
>             i++;
>             continue;
>         }
>@@ -225,16 +260,16 @@ void DocumentMarkerController::removeMarkers(Node* node, unsigned startOffset, i
>             continue;
> 
>         // add either of the resulting slices that are left after removing target
>-        if (startOffset > marker.startOffset) {
>+        if (startOffset > marker.startOffset()) {
>             DocumentMarker newLeft = marker;
>-            newLeft.endOffset = startOffset;
>+            newLeft.setEndOffset(startOffset);
>             list->insert(i, RenderedDocumentMarker(newLeft));
>             // i now points to the newly-inserted node, but we want to skip that one
>             i++;
>         }
>-        if (marker.endOffset > endOffset) {
>+        if (marker.endOffset() > endOffset) {
>             DocumentMarker newRight = marker;
>-            newRight.startOffset = endOffset;
>+            newRight.setStartOffset(endOffset);
>             list->insert(i, RenderedDocumentMarker(newRight));
>             // i now points to the newly-inserted node, but we want to skip that one
>             i++;
>@@ -270,7 +305,7 @@ DocumentMarker* DocumentMarkerController::markerContainingPoint(const IntPoint&
>             RenderedDocumentMarker& marker = list->at(markerIndex);
> 
>             // skip marker that is wrong type
>-            if (marker.type != markerType)
>+            if (marker.type() != markerType)
>                 continue;
> 
>             if (marker.contains(point))
>@@ -311,11 +346,11 @@ 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 (!markerTypes.contains(it->type))
>+            if (!markerTypes.contains(it->type()))
>                 continue;
>-            if (node == startContainer && it->endOffset <= static_cast<unsigned>(range->startOffset()))
>+            if (node == startContainer && it->endOffset() <= static_cast<unsigned>(range->startOffset()))
>                 continue;
>-            if (node == endContainer && it->startOffset >= static_cast<unsigned>(range->endOffset()))
>+            if (node == endContainer && it->startOffset() >= static_cast<unsigned>(range->endOffset()))
>                 continue;
>             foundMarkers.append(*it);
>         }
>@@ -341,7 +376,7 @@ Vector<IntRect> DocumentMarkerController::renderedRectsForMarkers(DocumentMarker
>             const RenderedDocumentMarker& marker = list->at(markerIndex);
> 
>             // skip marker that is wrong type
>-            if (marker.type != markerType)
>+            if (marker.type() != markerType)
>                 continue;
> 
>             if (!marker.isRendered())
>@@ -393,7 +428,7 @@ void DocumentMarkerController::removeMarkersFromList(Node* node, MarkerList* lis
>             DocumentMarker marker = list->at(i);
> 
>             // skip nodes that are not of the specified type
>-            if (!markerTypes.contains(marker.type)) {
>+            if (!markerTypes.contains(marker.type())) {
>                 ++i;
>                 continue;
>             }
>@@ -440,7 +475,7 @@ void DocumentMarkerController::repaintMarkers(DocumentMarker::MarkerTypes marker
>             DocumentMarker marker = list->at(i);
> 
>             // skip nodes that are not of the specified type
>-            if (markerTypes.contains(marker.type)) {
>+            if (markerTypes.contains(marker.type())) {
>                 nodeNeedsRepaint = true;
>                 break;
>             }
>@@ -501,10 +536,9 @@ void DocumentMarkerController::shiftMarkers(Node* node, unsigned startOffset, in
>     bool docDirty = false;
>     for (size_t i = 0; i != list->size(); ++i) {
>         RenderedDocumentMarker& marker = list->at(i);
>-        if (marker.startOffset >= startOffset) {
>-            ASSERT((int)marker.startOffset + delta >= 0);
>-            marker.startOffset += delta;
>-            marker.endOffset += delta;
>+        if (marker.startOffset() >= startOffset) {
>+            ASSERT((int)marker.startOffset() + delta >= 0);
>+            marker.shiftOffsets(delta);
>             docDirty = true;
> 
>             // Marker moved, so previously-computed rendered rectangle is now invalid
>@@ -546,14 +580,14 @@ void DocumentMarkerController::setMarkersActive(Node* node, unsigned startOffset
>         DocumentMarker& marker = list->at(i);
> 
>         // Markers are returned in order, so stop if we are now past the specified range.
>-        if (marker.startOffset >= endOffset)
>+        if (marker.startOffset() >= endOffset)
>             break;
> 
>         // Skip marker that is wrong type or before target.
>-        if (marker.endOffset < startOffset || marker.type != DocumentMarker::TextMatch)
>+        if (marker.endOffset() < startOffset || marker.type() != DocumentMarker::TextMatch)
>             continue;
> 
>-        marker.activeMatch = active;
>+        marker.setActiveMatch(active);
>         docDirty = true;
>     }
> 
>@@ -578,11 +612,11 @@ 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.contains(it->type))
>+            if (!markerTypes.contains(it->type()))
>                 continue;
>-            if (node == startContainer && it->endOffset <= static_cast<unsigned>(range->startOffset()))
>+            if (node == startContainer && it->endOffset() <= static_cast<unsigned>(range->startOffset()))
>                 continue;
>-            if (node == endContainer && it->startOffset >= static_cast<unsigned>(range->endOffset()))
>+            if (node == endContainer && it->startOffset() >= static_cast<unsigned>(range->endOffset()))
>                 continue;
>             return true;
>         }
>@@ -611,16 +645,16 @@ void DocumentMarkerController::clearDescriptionOnMarkersIntersectingRange(Range*
>             DocumentMarker& marker = list->at(i);
> 
>             // markers are returned in order, so stop if we are now past the specified range
>-            if (marker.startOffset >= endOffset)
>+            if (marker.startOffset() >= endOffset)
>                 break;
> 
>             // skip marker that is wrong type or before target
>-            if (marker.endOffset <= startOffset || !markerTypes.contains(marker.type)) {
>+            if (marker.endOffset() <= startOffset || !markerTypes.contains(marker.type())) {
>                 i++;
>                 continue;
>             }
> 
>-            marker.description = String();
>+            marker.clearDescription();
>         }
>     }
> }
>@@ -634,8 +668,11 @@ void DocumentMarkerController::showMarkers() const
>         Node* node = nodeIterator->first.get();
>         fprintf(stderr, "%p", node);
>         MarkerList* list = nodeIterator->second;
>-        for (unsigned markerIndex = 0; markerIndex < list->size(); ++markerIndex)
>-            fprintf(stderr, " %d:[%d:%d](%d)", list->at(markerIndex).type, list->at(markerIndex).startOffset, list->at(markerIndex).endOffset, list->at(markerIndex).activeMatch);
>+        for (unsigned markerIndex = 0; markerIndex < list->size(); ++markerIndex) {
>+            const DocumentMarker& marker = list->at(markerIndex);
>+            fprintf(stderr, " %d:[%d:%d](%d)", marker.type(), marker.startOffset(), marker.endOffset(), marker.activeMatch());
>+        }
>+
>         fprintf(stderr, "\n");
>     }
> }
>diff --git a/Source/WebCore/dom/DocumentMarkerController.h b/Source/WebCore/dom/DocumentMarkerController.h
>index 1b158d8f63b86a58a1c4a4e773b24e5cb0f0d3db..ee4117ddf07ee78a9104b4ec235d822dcf3978d9 100644
>--- a/Source/WebCore/dom/DocumentMarkerController.h
>+++ b/Source/WebCore/dom/DocumentMarkerController.h
>@@ -47,8 +47,10 @@ public:
>     ~DocumentMarkerController() { detach(); }
> 
>     void detach();
>-    void addMarker(Range*, DocumentMarker::MarkerType, String description = String());
>-    void addMarker(Node*, const DocumentMarker&);
>+    void addMarker(Range*, DocumentMarker::MarkerType);
>+    void addMarker(Range*, DocumentMarker::MarkerType, const String& description);
>+    void addTextMatchMarker(Range*, bool activeMatch);
>+
>     void copyMarkers(Node* srcNode, unsigned startOffset, int length, Node* dstNode, int delta);
>     bool hasMarkers(Range*, DocumentMarker::MarkerTypes = DocumentMarker::AllMarkers());
> 
>@@ -79,6 +81,8 @@ public:
> #endif
> 
> private:
>+    void addMarker(Node*, const DocumentMarker&);
>+
>     typedef Vector<RenderedDocumentMarker> MarkerList;
>     typedef HashMap<RefPtr<Node>, MarkerList*> MarkerMap;
>     bool possiblyHasMarkers(DocumentMarker::MarkerTypes);
>diff --git a/Source/WebCore/editing/CompositeEditCommand.cpp b/Source/WebCore/editing/CompositeEditCommand.cpp
>index dd5051b2d9a5c4e66a11fb66bb237a4b646f5c48..0ab83a851a7a320e97b0400bdd26ee636bcf8cd5 100644
>--- a/Source/WebCore/editing/CompositeEditCommand.cpp
>+++ b/Source/WebCore/editing/CompositeEditCommand.cpp
>@@ -348,8 +348,12 @@ void CompositeEditCommand::replaceTextInNodePreservingMarkers(PassRefPtr<Text> p
>     Vector<DocumentMarker> markers = markerController->markersInRange(Range::create(document(), node, offset, node, offset + count).get(), DocumentMarker::AllMarkers());
>     replaceTextInNode(node, offset, count, replacementText);
>     RefPtr<Range> newRange = Range::create(document(), node, offset, node, offset + replacementText.length());
>-    for (size_t i = 0; i < markers.size(); ++i)
>-        markerController->addMarker(newRange.get(), markers[i].type, markers[i].description);
>+    for (size_t i = 0; i < markers.size(); ++i) {
>+        if (markers[i].hasDescription())
>+            markerController->addMarker(newRange.get(), markers[i].type(), markers[i].description());
>+        else
>+            markerController->addMarker(newRange.get(), markers[i].type());
>+    }
> }
> 
> Position CompositeEditCommand::positionOutsideTabSpan(const Position& pos)
>diff --git a/Source/WebCore/editing/DeleteSelectionCommand.cpp b/Source/WebCore/editing/DeleteSelectionCommand.cpp
>index 749923032cfe8f4afd93bcfa25b590a152bf0671..aaa6d9597b4c9ebe0e8d71b00d62f95059661fca 100644
>--- a/Source/WebCore/editing/DeleteSelectionCommand.cpp
>+++ b/Source/WebCore/editing/DeleteSelectionCommand.cpp
>@@ -762,9 +762,9 @@ String DeleteSelectionCommand::originalStringForAutocorrectionAtBeginningOfSelec
>     Vector<DocumentMarker> markers = document()->markers()->markersInRange(rangeOfFirstCharacter.get(), DocumentMarker::Autocorrected);
>     for (size_t i = 0; i < markers.size(); ++i) {
>         const DocumentMarker& marker = markers[i];
>-        int startOffset = marker.startOffset;
>+        int startOffset = marker.startOffset();
>         if (startOffset == startOfSelection.deepEquivalent().offsetInContainerNode())
>-            return marker.description;
>+            return marker.description();
>     }
>     return String();
> }
>diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
>index 9e706c0156d2a8a371d16f83222f60add969b8f5..c70df9c3b20b6c02f4e63118d1371300756c4927 100644
>--- a/Source/WebCore/editing/Editor.cpp
>+++ b/Source/WebCore/editing/Editor.cpp
>@@ -3219,7 +3219,7 @@ bool Editor::selectionStartHasMarkerFor(DocumentMarker::MarkerType markerType, i
>     Vector<DocumentMarker> markers = m_frame->document()->markers()->markersForNode(node);
>     for (size_t i = 0; i < markers.size(); ++i) {
>         DocumentMarker marker = markers[i];
>-        if (marker.startOffset <= startOffset && endOffset <= marker.endOffset && marker.type == markerType)
>+        if (marker.startOffset() <= startOffset && endOffset <= marker.endOffset() && marker.type() == markerType)
>             return true;
>     }
> 
>diff --git a/Source/WebCore/editing/SpellingCorrectionController.cpp b/Source/WebCore/editing/SpellingCorrectionController.cpp
>index 03fae12180971db17b8abd3d65fb8a0fee088739..26dfb10deacfa2e909af82b08a63ae8017504b80 100644
>--- a/Source/WebCore/editing/SpellingCorrectionController.cpp
>+++ b/Source/WebCore/editing/SpellingCorrectionController.cpp
>@@ -409,17 +409,17 @@ void SpellingCorrectionController::respondToChangedSelection(const VisibleSelect
>         const DocumentMarker& marker = markers[i];
>         if (!shouldStartTimerFor(marker, endOffset))
>             continue;
>-        RefPtr<Range> wordRange = Range::create(m_frame->document(), node, marker.startOffset, node, marker.endOffset);
>+        RefPtr<Range> wordRange = Range::create(m_frame->document(), node, marker.startOffset(), node, marker.endOffset());
>         String currentWord = plainText(wordRange.get());
>         if (!currentWord.length())
>             continue;
> 
>         m_correctionPanelInfo.rangeToBeReplaced = wordRange;
>         m_correctionPanelInfo.replacedString = currentWord;
>-        if (marker.type == DocumentMarker::Spelling)
>+        if (marker.type() == DocumentMarker::Spelling)
>             startCorrectionPanelTimer(CorrectionPanelInfo::PanelTypeSpellingSuggestions);
>         else {
>-            m_correctionPanelInfo.replacementString = marker.description;
>+            m_correctionPanelInfo.replacementString = marker.description();
>             startCorrectionPanelTimer(CorrectionPanelInfo::PanelTypeReversion);
>         }
> 
>diff --git a/Source/WebCore/editing/SpellingCorrectionController.h b/Source/WebCore/editing/SpellingCorrectionController.h
>index 7b9368520d3775bc0c9719fe5c8cfd2f92ee7ec9..815748f4842de578abd9d87d8c321142e3d74abd 100644
>--- a/Source/WebCore/editing/SpellingCorrectionController.h
>+++ b/Source/WebCore/editing/SpellingCorrectionController.h
>@@ -128,8 +128,8 @@ private:
> 
>     bool shouldStartTimerFor(const DocumentMarker& marker, int endOffset) const
>     {
>-        return (((marker.type == DocumentMarker::Replacement && !marker.description.isNull()) 
>-                 || marker.type == DocumentMarker::Spelling) && static_cast<int>(marker.endOffset) == endOffset);
>+        return (((marker.type() == DocumentMarker::Replacement && !marker.description().isNull()) 
>+                 || marker.type() == DocumentMarker::Spelling) && static_cast<int>(marker.endOffset()) == endOffset);
>     }
> 
>     EditorClient* client();
>diff --git a/Source/WebCore/rendering/HitTestResult.cpp b/Source/WebCore/rendering/HitTestResult.cpp
>index 71dfeadfc1816e09a9808d555a033f7a147501ec..d5a1b5c858cc40e0ff577ce2ac1c18034061d0da 100644
>--- a/Source/WebCore/rendering/HitTestResult.cpp
>+++ b/Source/WebCore/rendering/HitTestResult.cpp
>@@ -201,7 +201,7 @@ String HitTestResult::spellingToolTip(TextDirection& dir) const
> 
>     if (RenderObject* renderer = m_innerNonSharedNode->renderer())
>         dir = renderer->style()->direction();
>-    return marker->description;
>+    return marker->description();
> }
> 
> String HitTestResult::replacedString() const
>@@ -215,7 +215,7 @@ String HitTestResult::replacedString() const
>     if (!marker)
>         return String();
>     
>-    return marker->description;
>+    return marker->description();
> }    
>     
> String HitTestResult::title(TextDirection& dir) const
>diff --git a/Source/WebCore/rendering/InlineTextBox.cpp b/Source/WebCore/rendering/InlineTextBox.cpp
>index 1cee1fce4dffebe47293bd51b526dd977c5e368b..0eda7ebf4d3105c32da71af87078916e1384828c 100644
>--- a/Source/WebCore/rendering/InlineTextBox.cpp
>+++ b/Source/WebCore/rendering/InlineTextBox.cpp
>@@ -992,16 +992,16 @@ void InlineTextBox::paintSpellingOrGrammarMarker(GraphicsContext* pt, const Floa
> 
>     // Determine whether we need to measure text
>     bool markerSpansWholeBox = true;
>-    if (m_start <= (int)marker.startOffset)
>+    if (m_start <= (int)marker.startOffset())
>         markerSpansWholeBox = false;
>-    if ((end() + 1) != marker.endOffset)      // end points at the last char, not past it
>+    if ((end() + 1) != marker.endOffset()) // end points at the last char, not past it
>         markerSpansWholeBox = false;
>     if (m_truncation != cNoTruncation)
>         markerSpansWholeBox = false;
> 
>     if (!markerSpansWholeBox || grammar) {
>-        int startPosition = max<int>(marker.startOffset - m_start, 0);
>-        int endPosition = min<int>(marker.endOffset - m_start, m_len);
>+        int startPosition = max<int>(marker.startOffset() - m_start, 0);
>+        int endPosition = min<int>(marker.endOffset() - m_start, m_len);
>         
>         if (m_truncation != cNoTruncation)
>             endPosition = min<int>(endPosition, m_truncation);
>@@ -1043,7 +1043,7 @@ void InlineTextBox::paintSpellingOrGrammarMarker(GraphicsContext* pt, const Floa
>         // In larger fonts, though, place the underline up near the baseline to prevent a big gap.
>         underlineOffset = baseline + 2;
>     }
>-    pt->drawLineForTextChecking(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + underlineOffset), width, textCheckingLineStyleForMarkerType(marker.type));
>+    pt->drawLineForTextChecking(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + underlineOffset), width, textCheckingLineStyleForMarkerType(marker.type()));
> }
> 
> void InlineTextBox::paintTextMatchMarker(GraphicsContext* pt, const FloatPoint& boxOrigin, const DocumentMarker& marker, RenderStyle* style, const Font& font)
>@@ -1053,8 +1053,8 @@ void InlineTextBox::paintTextMatchMarker(GraphicsContext* pt, const FloatPoint&
>     int deltaY = renderer()->style()->isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
>     int selHeight = selectionHeight();
> 
>-    int sPos = max(marker.startOffset - m_start, (unsigned)0);
>-    int ePos = min(marker.endOffset - m_start, (unsigned)m_len);    
>+    int sPos = max(marker.startOffset() - m_start, (unsigned)0);
>+    int ePos = min(marker.endOffset() - m_start, (unsigned)m_len);    
>     TextRun run(textRenderer()->text()->characters() + m_start, m_len, textRenderer()->allowTabs(), textPos(), m_expansion, expansionBehavior(), direction(), m_dirOverride || style->visuallyOrdered());
>     
>     // Always compute and store the rect associated with this marker. The computed rect is in absolute coordinates.
>@@ -1064,7 +1064,7 @@ void InlineTextBox::paintTextMatchMarker(GraphicsContext* pt, const FloatPoint&
>     
>     // Optionally highlight the text
>     if (renderer()->frame()->editor()->markedTextMatchesAreHighlighted()) {
>-        Color color = marker.activeMatch ?
>+        Color color = marker.activeMatch() ?
>             renderer()->theme()->platformActiveTextSearchHighlightColor() :
>             renderer()->theme()->platformInactiveTextSearchHighlightColor();
>         GraphicsContextStateSaver stateSaver(*pt);
>@@ -1080,8 +1080,8 @@ void InlineTextBox::computeRectForReplacementMarker(const DocumentMarker& marker
>     int y = selectionTop();
>     int h = selectionHeight();
>     
>-    int sPos = max(marker.startOffset - m_start, (unsigned)0);
>-    int ePos = min(marker.endOffset - m_start, (unsigned)m_len);    
>+    int sPos = max(marker.startOffset() - m_start, (unsigned)0);
>+    int ePos = min(marker.endOffset() - m_start, (unsigned)m_len);    
>     TextRun run(textRenderer()->text()->characters() + m_start, m_len, textRenderer()->allowTabs(), textPos(), m_expansion, expansionBehavior(), direction(), m_dirOverride || style->visuallyOrdered());
>     IntPoint startPoint = IntPoint(m_x, y);
>     
>@@ -1105,7 +1105,7 @@ void InlineTextBox::paintDocumentMarkers(GraphicsContext* pt, const FloatPoint&
>         const DocumentMarker& marker = *markerIt;
>         
>         // Paint either the background markers or the foreground markers, but not both
>-        switch (marker.type) {
>+        switch (marker.type()) {
>             case DocumentMarker::Grammar:
>             case DocumentMarker::Spelling:
>             case DocumentMarker::CorrectionIndicator:
>@@ -1121,17 +1121,17 @@ void InlineTextBox::paintDocumentMarkers(GraphicsContext* pt, const FloatPoint&
>                 continue;
>         }
> 
>-        if (marker.endOffset <= start())
>+        if (marker.endOffset() <= start())
>             // marker is completely before this run.  This might be a marker that sits before the
>             // first run we draw, or markers that were within runs we skipped due to truncation.
>             continue;
>         
>-        if (marker.startOffset > end())
>+        if (marker.startOffset() > end())
>             // marker is completely after this run, bail.  A later run will paint it.
>             break;
>         
>         // marker intersects this run.  Paint it.
>-        switch (marker.type) {
>+        switch (marker.type()) {
>             case DocumentMarker::Spelling:
>                 paintSpellingOrGrammarMarker(pt, boxOrigin, marker, style, font, false);
>                 break;
>diff --git a/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp b/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp
>index 1d0043bbe55ecb6c6a48d91022774aeadf88fa27..a0028671012ecb2b9f29fb521fcd24df9c8b151d 100644
>--- a/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp
>+++ b/Source/WebCore/rendering/svg/SVGInlineFlowBox.cpp
>@@ -99,7 +99,7 @@ void SVGInlineFlowBox::computeTextMatchMarkerRectForRenderer(RenderSVGInlineText
>         const DocumentMarker& marker = *markerIt;
> 
>         // SVG is only interessted in the TextMatch marker, for now.
>-        if (marker.type != DocumentMarker::TextMatch)
>+        if (marker.type() != DocumentMarker::TextMatch)
>             continue;
> 
>         FloatRect markerRect;
>@@ -109,8 +109,8 @@ void SVGInlineFlowBox::computeTextMatchMarkerRectForRenderer(RenderSVGInlineText
> 
>             SVGInlineTextBox* textBox = static_cast<SVGInlineTextBox*>(box);
> 
>-            int markerStartPosition = max<int>(marker.startOffset - textBox->start(), 0);
>-            int markerEndPosition = min<int>(marker.endOffset - textBox->start(), textBox->len());
>+            int markerStartPosition = max<int>(marker.startOffset() - textBox->start(), 0);
>+            int markerEndPosition = min<int>(marker.endOffset() - textBox->start(), textBox->len());
> 
>             if (markerStartPosition >= markerEndPosition)
>                 continue;
>diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
>index 192196e4a92e512d49e058f3be805011d5edbaee..c26061b9f992e339acc65e7e3d0440c3c6f77d15 100644
>--- a/Source/WebKit/chromium/ChangeLog
>+++ b/Source/WebKit/chromium/ChangeLog
>@@ -1,3 +1,16 @@
>+2011-05-17  MORITA Hajime  <morrita@google.com>
>+
>+        Reviewed by Tony Chang.
>+
>+        [Refactoring] Member variables of DocumentMarker should be encapsulated.        
>+        https://bugs.webkit.org/show_bug.cgi?id=56814
>+
>+        Moved addMarker() implementation to
>+        WebCore::DocumentMarkerController::addTextMatchMarker().
>+        
>+        * src/WebFrameImpl.cpp:
>+        (WebKit::WebFrameImpl::addMarker):
>+
> 2011-05-17  Tao Bai  <michaelbai@chromium.org>
> 
>         Reviewed by Darin Fisher.
>diff --git a/Source/WebKit/chromium/src/WebFrameImpl.cpp b/Source/WebKit/chromium/src/WebFrameImpl.cpp
>index 3ecdfd92e15194e0ad0423c4286aa04c8d00d3d1..5297e9857d781ea981e8714d5edae946889002ec 100644
>--- a/Source/WebKit/chromium/src/WebFrameImpl.cpp
>+++ b/Source/WebKit/chromium/src/WebFrameImpl.cpp
>@@ -2232,37 +2232,7 @@ void WebFrameImpl::invalidateArea(AreaToInvalidate area)
> 
> void WebFrameImpl::addMarker(Range* range, bool activeMatch)
> {
>-    // Use a TextIterator to visit the potentially multiple nodes the range
>-    // covers.
>-    TextIterator markedText(range);
>-    for (; !markedText.atEnd(); markedText.advance()) {
>-        RefPtr<Range> textPiece = markedText.range();
>-        int exception = 0;
>-
>-        DocumentMarker marker = {
>-            DocumentMarker::TextMatch,
>-            textPiece->startOffset(exception),
>-            textPiece->endOffset(exception),
>-            "",
>-            activeMatch
>-        };
>-
>-        if (marker.endOffset > marker.startOffset) {
>-            // Find the node to add a marker to and add it.
>-            Node* node = textPiece->startContainer(exception);
>-            frame()->document()->markers()->addMarker(node, marker);
>-
>-            // Rendered rects for markers in WebKit are not populated until each time
>-            // the markers are painted. However, we need it to happen sooner, because
>-            // the whole purpose of tickmarks on the scrollbar is to show where
>-            // matches off-screen are (that haven't been painted yet).
>-            Vector<DocumentMarker> markers = frame()->document()->markers()->markersForNode(node);
>-            frame()->document()->markers()->setRenderedRectForMarker(
>-                textPiece->startContainer(exception),
>-                markers[markers.size() - 1],
>-                range->boundingBox());
>-        }
>-    }
>+    frame()->document()->markers()->addTextMatchMarker(range, activeMatch);
> }
> 
> void WebFrameImpl::setMarkerActive(Range* range, bool active)

Flags:

tony: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 56814: 93622 | 93624 | 93860