[editing] Avoid CHECK failure in CreateMarkupAlgorithm::CreateMarkup

CompositeEditCommand::MoveParagraphs has a start and end positions, and
constrains them with Most{Forward,Backward}CaretPosition. The problem
was that the resulting positions could end up reversed, triggering a
CHECK failure in CreateMarkup.

This patch changes CompositeEditCommand::MoveParagraphs so that it only
attempts to contrain the positions if they are different, and also
changes MostBackwardOrForwardCaretPosition to ensure that the shadow-
adjusted candidate is not further from the initial position than the
non-adjusted candidate.

Bug: 1165325

TEST=CompositeEditCommandTest.MoveParagraphContentsToNewBlockWithUAShadowDOM
TEST=SelectionAdjusterTest.AdjustSelectionTypeWithShadow
TEST=VisibleSelectionTest.ShadowCrossing
TEST=VisibleSelectionTest.ShadowNested

Change-Id: Ib52ed472809e15aa551fdfc4ed1fa2323433d4cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2627368
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843406}

third_party/blink/renderer/core/editing/commands/composite_edit_command.cc

@@ -1503,13 +1503,19 @@ void CompositeEditCommand::MoveParagraphs(
       NextPositionOf(end_of_paragraph_to_move, kCannotCrossEditingBoundary)
           .DeepEquivalent());
 
+  const Position& start_candidate = start_of_paragraph_to_move.DeepEquivalent();
+  const Position& end_candidate = end_of_paragraph_to_move.DeepEquivalent();
+  DCHECK_LE(start_candidate, end_candidate);
+
   // We upstream() the end and downstream() the start so that we don't include
   // collapsed whitespace in the move. When we paste a fragment, spaces after
   // the end and before the start are treated as though they were rendered.
+  bool equal = start_candidate == end_candidate;
   Position start =
-      MostForwardCaretPosition(start_of_paragraph_to_move.DeepEquivalent());
+      equal ? start_candidate : MostForwardCaretPosition(start_candidate);
   Position end =
-      MostBackwardCaretPosition(end_of_paragraph_to_move.DeepEquivalent());
+      equal ? end_candidate : MostBackwardCaretPosition(end_candidate);
+  DCHECK_LE(start, end);
 
   // FIXME: This is an inefficient way to preserve style on nodes in the
   // paragraph to move. It shouldn't matter though, since moved paragraphs will

third_party/blink/renderer/core/editing/commands/composite_edit_command_test.cc

@@ -9,6 +9,7 @@
 #include "third_party/blink/renderer/core/dom/text.h"
 #include "third_party/blink/renderer/core/editing/frame_selection.h"
 #include "third_party/blink/renderer/core/editing/testing/editing_test_base.h"
+#include "third_party/blink/renderer/core/editing/visible_position.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 
 namespace blink {
@@ -142,6 +143,23 @@ TEST_F(CompositeEditCommandTest,
       body->innerHTML());
 }
 
+TEST_F(CompositeEditCommandTest,
+       MoveParagraphContentsToNewBlockWithUAShadowDOM) {
+  SetBodyContent("<object contenteditable><input></object>");
+  base::RunLoop().RunUntilIdle();
+
+  SampleCommand& sample = *MakeGarbageCollected<SampleCommand>(GetDocument());
+  Element* input = GetDocument().QuerySelector("input");
+  Position pos = Position::BeforeNode(*input);
+  EditingState editing_state;
+
+  // Should not crash
+  sample.MoveParagraphContentsToNewBlockIfNecessary(pos, &editing_state);
+  EXPECT_FALSE(editing_state.IsAborted());
+  EXPECT_EQ("<object contenteditable=\"\"><div><input></div></object>",
+            GetDocument().body()->innerHTML());
+}
+
 TEST_F(CompositeEditCommandTest, InsertNodeOnDisconnectedParent) {
   SetBodyContent("<p><b></b></p>");
   SampleCommand& sample = *MakeGarbageCollected<SampleCommand>(GetDocument());

third_party/blink/renderer/core/editing/selection_adjuster_test.cc

@@ -523,17 +523,17 @@ TEST_F(SelectionAdjusterTest, AdjustSelectionTypeWithShadow) {
   SetShadowContent("bar<slot></slot>", "host");
 
   Element* host = GetDocument().getElementById("host");
+  const Position& base = Position(host->firstChild(), 0);
+  const Position& extent = Position(host, 0);
   const SelectionInDOMTree& selection =
-      SelectionInDOMTree::Builder()
-          .Collapse(Position(host->firstChild(), 0))
-          .Extend(Position(host, 0))
-          .Build();
+      SelectionInDOMTree::Builder().Collapse(base).Extend(extent).Build();
+
   // Should not crash
   const SelectionInDOMTree& adjusted =
       SelectionAdjuster::AdjustSelectionType(selection);
 
-  EXPECT_EQ(Position::BeforeNode(*host), adjusted.Base());
-  EXPECT_EQ(Position::BeforeNode(*host), adjusted.Extent());
+  EXPECT_EQ(base, adjusted.Base());
+  EXPECT_EQ(extent, adjusted.Extent());
 }
 
 }  // namespace blink

third_party/blink/renderer/core/editing/visible_selection_test.cc

@@ -462,7 +462,7 @@ TEST_F(VisibleSelectionTest, ShadowCrossing) {
 
   EXPECT_EQ(Position(host, PositionAnchorType::kBeforeAnchor),
             selection.Start());
-  EXPECT_EQ(Position(host, PositionAnchorType::kBeforeAnchor), selection.End());
+  EXPECT_EQ(Position(one->firstChild(), 0), selection.End());
   EXPECT_EQ(PositionInFlatTree(one->firstChild(), 0),
             selection_in_flat_tree.Start());
   EXPECT_EQ(PositionInFlatTree(six->firstChild(), 2),
@@ -510,7 +510,7 @@ TEST_F(VisibleSelectionTest, ShadowNested) {
 
   EXPECT_EQ(Position(host, PositionAnchorType::kBeforeAnchor),
             selection.Start());
-  EXPECT_EQ(Position(host, PositionAnchorType::kBeforeAnchor), selection.End());
+  EXPECT_EQ(Position(one->firstChild(), 0), selection.End());
   EXPECT_EQ(PositionInFlatTree(eight->firstChild(), 2),
             selection_in_flat_tree.Start());
   EXPECT_EQ(PositionInFlatTree(eight->firstChild(), 2),

third_party/blink/renderer/core/editing/visible_units.cc

@@ -648,6 +648,12 @@ static Position MostBackwardOrForwardCaretPosition(
   const SelectionInDOMTree& shadow_adjusted_selection =
       SelectionAdjuster::AdjustSelectionToAvoidCrossingShadowBoundaries(
           selection);
+  const Position& adjusted_candidate = shadow_adjusted_selection.Extent();
+
+  // The adjusted candidate should be between the candidate and the original
+  // position. Otherwise, return the original position.
+  if (position.CompareTo(candidate) == candidate.CompareTo(adjusted_candidate))
+    return position;
 
   // If we have to adjust the position, the editability may change, so avoid
   // crossing editing boundaries if it's not allowed.
@@ -658,7 +664,7 @@ static Position MostBackwardOrForwardCaretPosition(
             shadow_adjusted_selection);
     return editing_adjusted_selection.Extent();
   }
-  return shadow_adjusted_selection.Extent();
+  return adjusted_candidate;
 }
 
 template <typename Strategy>