Make ApplyStyleCommand::applyRelativeFontStyleChange() to handle AfterAnchor position correctly

third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp

 /*
  * Copyright (C) 2005, 2006, 2008, 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 347 matching lines @@
         start = startPosition();
         end = endPosition();
     }
 
     if (isValidCaretPositionInTextNode(end)) {
         splitTextAtEnd(start, end);
         start = startPosition();
         end = endPosition();
     }
 
+    DCHECK(start.anchorNode());
+    DCHECK(end.anchorNode());
     // Calculate loop end point.
     // If the end node is before the start node (can only happen if the end node is
     // an ancestor of the start node), we gather nodes up to the next sibling of the end node
-    Node* beyondEnd;
-    DCHECK(start.anchorNode());
-    DCHECK(end.anchorNode());
-    if (start.anchorNode()->isDescendantOf(end.anchorNode()))
-        beyondEnd = NodeTraversal::nextSkippingChildren(*end.anchorNode());
-    else
-        beyondEnd = NodeTraversal::next(*end.anchorNode());
-
+    const Node* const beyondEnd = end.nodeAsRangePastLastNode();
     start = mostBackwardCaretPosition(start); // Move upstream to ensure we do not add redundant spans.
     Node* startNode = start.anchorNode();
     DCHECK(startNode);
 
     // Make sure we're not already at the end or the next NodeTraversal::next() will traverse
     // past it.
     if (startNode == beyondEnd)
         return;
 
     if (startNode->isTextNode() && start.computeOffsetInContainerNode() >= caretMaxOffset(startNode)) {
         // Move out of text node if range does not include its characters.
         startNode = NodeTraversal::next(*startNode);
         if (!startNode)
             return;
     }
 
     // Store away font size before making any changes to the document.
     // This ensures that changes to one node won't effect another.
     HeapHashMap<Member<Node>, float> startingFontSizes;
     for (Node* node = startNode; node != beyondEnd; node = NodeTraversal::next(*node)) {
         DCHECK(node);
         startingFontSizes.set(node, computedFontSize(node));
     }
 
     // These spans were added by us. If empty after font size changes, they can be removed.
     HeapVector<Member<HTMLElement>> unstyledSpans;
 
     Node* lastStyledNode = nullptr;
     for (Node* node = startNode; node != beyondEnd; node = NodeTraversal::next(*node)) {

[yoichio, 2016/07/05 05:50:57]

This end condition looks very fragile.
Could you introduce something like |node '<' beyondEnd| or DCHECK it?

[yosin_UTC9, 2016/07/05 07:41:24]

When |beyondEnd| is a wrong value, e.g. |beyondEnd| appears before |startNode|,
it is caught by |DCHECK(node)| in L403.

I don't think we don't need to have  
DCHECK(beyondEnd && (startNode->compareDocumentPosition(beyondEnd) &
Node::DOCUMENT_POSITION_FOLLOWING))

         DCHECK(node);
         HTMLElement* element = nullptr;
         if (node->isHTMLElement()) {
             // Only work on fully selected nodes.
             if (!elementFullySelected(toHTMLElement(*node), start, end))
                 continue;
             element = toHTMLElement(node);
         } else if (node->isTextNode() && node->layoutObject() && node->parentNode() != lastStyledNode) {
             // Last styled node was not parent node of this text node, but we wish to style this
             // text node. To make this possible, add a style span to surround this text node.
@@ skipping 1313 matching lines @@
 DEFINE_TRACE(ApplyStyleCommand)
 {
     visitor->trace(m_style);
     visitor->trace(m_start);
     visitor->trace(m_end);
     visitor->trace(m_styledInlineElement);
     CompositeEditCommand::trace(visitor);
 }
 
 } // namespace blink