[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

rniwa at webkit.org rniwa at webkit.org
Wed Dec 22 11:38:52 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 8bbe08be730d006eca6146dfde21bfbc2bcbcd28
Author: rniwa at webkit.org <rniwa at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Mon Aug 2 18:13:59 2010 +0000

    2010-08-02  Ryosuke Niwa  <rniwa at webkit.org>
    
            Reviewed by Kent Tamura.
    
            Group functions used in createMarkup (range version) into a class so they are easier to understand
            https://bugs.webkit.org/show_bug.cgi?id=43227
    
            Added MarkupAccumulatorWrapper to group getStartMarkup, getEndMarkup, joinMarkups, and addStyleMarkup.
            MarkupAccumulatorWrapper is intended to be merged with MarkupAccumulator in the future.
    
            No new tests added since this is a clean up.
    
            * editing/markup.cpp:
            (WebCore::MarkupAccumulatorWrapper::MarkupAccumulatorWrapper): Added.
            (WebCore::MarkupAccumulatorWrapper::insertString): Added.
            (WebCore::MarkupAccumulatorWrapper::insertOpenTag): Added.
            (WebCore::MarkupAccumulatorWrapper::insertEndTag): Added.
            (WebCore::MarkupAccumulatorWrapper::wrapWithNode): Added.
            (WebCore::MarkupAccumulatorWrapper::wrapWithStyleNode): Added.
            (WebCore::MarkupAccumulatorWrapper::takeResults): Added.
            (WebCore::createMarkup): Uses MarkupAccumulatorWrapper.
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@64477 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index d24f5cc..4b6d9f1 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2010-08-02  Ryosuke Niwa  <rniwa at webkit.org>
+
+        Reviewed by Kent Tamura.
+
+        Group functions used in createMarkup (range version) into a class so they are easier to understand
+        https://bugs.webkit.org/show_bug.cgi?id=43227
+
+        Added MarkupAccumulatorWrapper to group getStartMarkup, getEndMarkup, joinMarkups, and addStyleMarkup.
+        MarkupAccumulatorWrapper is intended to be merged with MarkupAccumulator in the future.
+
+        No new tests added since this is a clean up.
+
+        * editing/markup.cpp:
+        (WebCore::MarkupAccumulatorWrapper::MarkupAccumulatorWrapper): Added.
+        (WebCore::MarkupAccumulatorWrapper::insertString): Added.
+        (WebCore::MarkupAccumulatorWrapper::insertOpenTag): Added.
+        (WebCore::MarkupAccumulatorWrapper::insertEndTag): Added.
+        (WebCore::MarkupAccumulatorWrapper::wrapWithNode): Added.
+        (WebCore::MarkupAccumulatorWrapper::wrapWithStyleNode): Added.
+        (WebCore::MarkupAccumulatorWrapper::takeResults): Added.
+        (WebCore::createMarkup): Uses MarkupAccumulatorWrapper.
+
 2010-08-02  Brian Weinstein  <bweinstein at apple.com>
 
         Add a missing </File> tag to WebCore.vcproj.
diff --git a/WebCore/editing/markup.cpp b/WebCore/editing/markup.cpp
index dcb53d6..22e700a 100644
--- a/WebCore/editing/markup.cpp
+++ b/WebCore/editing/markup.cpp
@@ -555,13 +555,6 @@ static void appendStartMarkup(Vector<UChar>& result, const Node* node, const Ran
     }
 }
 
-static String getStartMarkup(const Node* node, const Range* range, EAnnotateForInterchange annotate, EAbsoluteURLs absoluteURLs, bool convertBlocksToInlines = false, HashMap<AtomicStringImpl*, AtomicStringImpl*>* namespaces = 0, RangeFullySelectsNode rangeFullySelectsNode = DoesFullySelectNode)
-{
-    Vector<UChar> result;
-    appendStartMarkup(result, node, range, annotate, absoluteURLs, convertBlocksToInlines, namespaces, rangeFullySelectsNode);
-    return String::adopt(result);
-}
-
 static inline bool doesHTMLForbidEndTag(const Node *node)
 {
     if (node->isHTMLElement()) {
@@ -598,13 +591,6 @@ static void appendEndMarkup(Vector<UChar>& result, const Node* node)
     result.append('>');
 }
 
-static String getEndMarkup(const Node *node)
-{
-    Vector<UChar> result;
-    appendEndMarkup(result, node);
-    return String::adopt(result);
-}
-
 class MarkupAccumulator {
 public:
     MarkupAccumulator(Node* nodeToSkip, Vector<Node*>* nodes)
@@ -722,30 +708,6 @@ static bool isElementPresentational(const Node* node)
     return !propertyMissingOrEqualToNone(style.get(), CSSPropertyTextDecoration);
 }
 
-static String joinMarkups(const Vector<String>& preMarkups, const Vector<String>& postMarkups)
-{
-    size_t length = 0;
-
-    size_t preCount = preMarkups.size();
-    for (size_t i = 0; i < preCount; ++i)
-        length += preMarkups[i].length();
-
-    size_t postCount = postMarkups.size();
-    for (size_t i = 0; i < postCount; ++i)
-        length += postMarkups[i].length();
-
-    Vector<UChar> result;
-    result.reserveInitialCapacity(length);
-
-    for (size_t i = preCount; i > 0; --i)
-        append(result, preMarkups[i - 1]);
-
-    for (size_t i = 0; i < postCount; ++i)
-        append(result, postMarkups[i]);
-
-    return String::adopt(result);
-}
-
 static bool isSpecialAncestorBlock(Node* node)
 {
     if (!node || !isBlock(node))
@@ -773,24 +735,88 @@ static bool shouldIncludeWrapperForFullySelectedRoot(Node* fullySelectedRoot, CS
            style->getPropertyCSSValue(CSSPropertyBackgroundColor);
 }
 
-static void addStyleMarkup(Vector<String>& preMarkups, Vector<String>& postMarkups, CSSStyleDeclaration* style, Document* document, bool isBlock = false)
-{
-    // All text-decoration-related elements should have been treated as special ancestors
-    // If we ever hit this ASSERT, we should export StyleChange in ApplyStyleCommand and use it here
-    ASSERT(propertyMissingOrEqualToNone(style, CSSPropertyTextDecoration) && propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect));
-    DEFINE_STATIC_LOCAL(const String, divStyle, ("<div style=\""));
-    DEFINE_STATIC_LOCAL(const String, divClose, ("</div>"));
-    DEFINE_STATIC_LOCAL(const String, styleSpanOpen, ("<span class=\"" AppleStyleSpanClass "\" style=\""));
-    DEFINE_STATIC_LOCAL(const String, styleSpanClose, ("</span>"));
-    Vector<UChar> openTag;
-    append(openTag, isBlock ? divStyle : styleSpanOpen);
-    appendAttributeValue(openTag, style->cssText(), document->isHTMLDocument());
-    openTag.append('\"');
-    openTag.append('>');
-    preMarkups.append(String::adopt(openTag));
-
-    postMarkups.append(isBlock ? divClose : styleSpanClose);
-}
+class MarkupAccumulatorWrapper {
+public:
+    MarkupAccumulatorWrapper()
+    {
+    }
+
+    void insertString(const String& s)
+    {
+        postMarkups.append(s);
+    }
+
+    void insertOpenTag(const Node* node, const Range* range, EAnnotateForInterchange annotate, EAbsoluteURLs absoluteURLs, bool convertBlocksToInlines = false, RangeFullySelectsNode rangeFullySelectsNode = DoesFullySelectNode)
+    {
+        Vector<UChar> result;
+        appendStartMarkup(result, node, range, annotate, absoluteURLs, convertBlocksToInlines, 0, rangeFullySelectsNode);
+        postMarkups.append(String::adopt(result));
+    }
+
+    void insertEndTag(const Node* node)
+    {
+        Vector<UChar> result;
+        appendEndMarkup(result, node);
+        postMarkups.append(String::adopt(result));
+    }
+
+    void wrapWithNode(const Node* node, const Range* range, EAnnotateForInterchange annotate, EAbsoluteURLs absoluteURLs, bool convertBlocksToInlines = false, RangeFullySelectsNode rangeFullySelectsNode = DoesFullySelectNode)
+    {
+        Vector<UChar> result;
+        appendStartMarkup(result, node, range, annotate, absoluteURLs, convertBlocksToInlines, 0, rangeFullySelectsNode);
+        preMarkups.append(String::adopt(result));
+        insertEndTag(node);
+    }
+
+    void wrapWithStyleNode(CSSStyleDeclaration* style, Document* document, bool isBlock = false)
+    {
+        // All text-decoration-related elements should have been treated as special ancestors
+        // If we ever hit this ASSERT, we should export StyleChange in ApplyStyleCommand and use it here
+        ASSERT(propertyMissingOrEqualToNone(style, CSSPropertyTextDecoration) && propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect));
+        DEFINE_STATIC_LOCAL(const String, divStyle, ("<div style=\""));
+        DEFINE_STATIC_LOCAL(const String, divClose, ("</div>"));
+        DEFINE_STATIC_LOCAL(const String, styleSpanOpen, ("<span class=\"" AppleStyleSpanClass "\" style=\""));
+        DEFINE_STATIC_LOCAL(const String, styleSpanClose, ("</span>"));
+        Vector<UChar> openTag;
+        WebCore::append(openTag, isBlock ? divStyle : styleSpanOpen);
+        appendAttributeValue(openTag, style->cssText(), document->isHTMLDocument());
+        openTag.append('\"');
+        openTag.append('>');
+        preMarkups.append(String::adopt(openTag));
+        postMarkups.append(isBlock ? divClose : styleSpanClose);
+    }
+
+    // FIXME: This is a very inefficient way of accumulating the markup.
+    // We're converting results of appendStartMarkup and appendEndMarkup from Vector<UChar> to String
+    // and then back to Vector<UChar> and again to String here.
+    String takeResults()
+    {
+        size_t length = 0;
+
+        size_t preCount = preMarkups.size();
+        for (size_t i = 0; i < preCount; ++i)
+            length += preMarkups[i].length();
+
+        size_t postCount = postMarkups.size();
+        for (size_t i = 0; i < postCount; ++i)
+            length += postMarkups[i].length();
+
+        Vector<UChar> result;
+        result.reserveInitialCapacity(length);
+
+        for (size_t i = preCount; i > 0; --i)
+            WebCore::append(result, preMarkups[i - 1]);
+
+        for (size_t i = 0; i < postCount; ++i)
+            WebCore::append(result, postMarkups[i]);
+
+        return String::adopt(result);
+    }
+
+private:
+    Vector<String> preMarkups;
+    Vector<String> postMarkups;
+};
 
 // FIXME: Shouldn't we omit style info when annotate == DoNotAnnotateForInterchange? 
 // FIXME: At least, annotation and style info should probably not be included in range.markupString()
@@ -828,8 +854,7 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
 
     document->updateLayoutIgnorePendingStylesheets();
 
-    Vector<String> markups;
-    Vector<String> preMarkups;
+    MarkupAccumulatorWrapper accumulator;
     Node* pastEnd = updatedRange->pastLastNode();
     Node* lastClosed = 0;
     Vector<Node*> ancestorsToClose;
@@ -844,7 +869,7 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
             return interchangeNewlineString;
         }
 
-        markups.append(interchangeNewlineString);
+        accumulator.insertString(interchangeNewlineString);
         startNode = visibleStart.next().deepEquivalent().node();
 
         if (pastEnd && Range::compareBoundaryPoints(startNode, 0, pastEnd, 0) >= 0) {
@@ -883,7 +908,8 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
         
         // Add the node to the markup.
         if (addMarkupForNode) {
-            markups.append(getStartMarkup(n, updatedRange.get(), annotate, absoluteURLs));
+
+            accumulator.insertOpenTag(n, updatedRange.get(), annotate, absoluteURLs);
             if (nodes)
                 nodes->append(n);
         }
@@ -891,7 +917,7 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
         if (n->firstChild() == 0 || skipDescendants) {
             // Node has no children, or we are skipping it's descendants, add its close tag now.
             if (addMarkupForNode) {
-                markups.append(getEndMarkup(n));
+                accumulator.insertEndTag(n);
                 lastClosed = n;
             }
             
@@ -904,7 +930,7 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
                         if (next != pastEnd && next->isDescendantOf(ancestor))
                             break;
                         // Not at the end of the range, close ancestors up to sibling of next node.
-                        markups.append(getEndMarkup(ancestor));
+                        accumulator.insertEndTag(ancestor);
                         lastClosed = ancestor;
                         ancestorsToClose.removeLast();
                     } while (!ancestorsToClose.isEmpty());
@@ -920,8 +946,7 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
                             continue;
                         // or b) ancestors that we never encountered during a pre-order traversal starting at startNode:
                         ASSERT(startNode->isDescendantOf(parent));
-                        preMarkups.append(getStartMarkup(parent, updatedRange.get(), annotate, absoluteURLs));
-                        markups.append(getEndMarkup(parent));
+                        accumulator.wrapWithNode(parent, updatedRange.get(), annotate, absoluteURLs);
                         if (nodes)
                             nodes->append(parent);
                         lastClosed = parent;
@@ -1001,13 +1026,12 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
                         fullySelectedRootStyle->setProperty(CSSPropertyTextDecoration, CSSValueNone);
                     if (!propertyMissingOrEqualToNone(fullySelectedRootStyle.get(), CSSPropertyWebkitTextDecorationsInEffect))
                         fullySelectedRootStyle->setProperty(CSSPropertyWebkitTextDecorationsInEffect, CSSValueNone);
-                    addStyleMarkup(preMarkups, markups, fullySelectedRootStyle.get(), document, true);
+                    accumulator.wrapWithStyleNode(fullySelectedRootStyle.get(), document, true);
                 }
             } else {
                 // Since this node and all the other ancestors are not in the selection we want to set RangeFullySelectsNode to DoesNotFullySelectNode
                 // so that styles that affect the exterior of the node are not included.
-                preMarkups.append(getStartMarkup(ancestor, updatedRange.get(), annotate, absoluteURLs, convertBlocksToInlines, 0, DoesNotFullySelectNode));
-                markups.append(getEndMarkup(ancestor));
+                accumulator.wrapWithNode(ancestor, updatedRange.get(), annotate, absoluteURLs, convertBlocksToInlines, DoesNotFullySelectNode);
             }
             if (nodes)
                 nodes->append(ancestor);
@@ -1039,7 +1063,7 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
             style->removeBlockProperties();
 
         if (style->length() > 0)
-            addStyleMarkup(preMarkups, markups, style.get(), document);
+            accumulator.wrapWithStyleNode(style.get(), document);
     }
     
     if (lastClosed && lastClosed != document->documentElement()) {
@@ -1049,17 +1073,17 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
         RefPtr<CSSMutableStyleDeclaration> defaultStyle = ApplyStyleCommand::editingStyleAtPosition(Position(document->documentElement(), 0));
 
         if (defaultStyle->length() > 0)
-            addStyleMarkup(preMarkups, markups, defaultStyle.get(), document);
+            accumulator.wrapWithStyleNode(defaultStyle.get(), document);
     }
 
     // FIXME: The interchange newline should be placed in the block that it's in, not after all of the content, unconditionally.
     if (annotate && needInterchangeNewlineAfter(visibleEnd.previous()))
-        markups.append(interchangeNewlineString);
-    
+        accumulator.insertString(interchangeNewlineString);
+
     if (deleteButton)
         deleteButton->enable();
 
-    return joinMarkups(preMarkups, markups);
+    return accumulator.takeResults();
 }
 
 PassRefPtr<DocumentFragment> createFragmentFromMarkup(Document* document, const String& markup, const String& baseURL, FragmentScriptingPermission scriptingPermission)

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list