Prune layout update calls from Editor::*appliedEditing

third_party/WebKit/Source/core/editing/Editor.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  *
  * 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
@@ skipping 865 matching lines @@
 static void dispatchEditableContentChangedEvents(Element* startRoot,
                                                  Element* endRoot) {
   if (startRoot)
     startRoot->dispatchEvent(
         Event::create(EventTypeNames::webkitEditableContentChanged));
   if (endRoot && endRoot != startRoot)
     endRoot->dispatchEvent(
         Event::create(EventTypeNames::webkitEditableContentChanged));
 }
 
-static VisibleSelection correctedVisibleSelection(
-    const VisibleSelection& passedSelection) {
+static SelectionInDOMTree correctedSelectionAfterCommand(
+    const VisibleSelection& passedSelection,

[yoichio, 2017/03/21 06:41:57]

Could you use SelectionINDOMTree& passedSelection instead of VS?

[Xiaocheng, 2017/03/21 20:53:39]

It doesn't seem to introduce any benefit, since EditCommand::endingSelection()
is a VS.

Anyway it's not important. I can change if you insist.

[yoichio, 2017/03/22 01:48:57]

Yes, no benefit from a view of semantics.
However, we're going to reduce VS usage in code.
Eventually EditCommand::endingSelection() will turn Selection.

[Xiaocheng, 2017/03/22 02:12:44]

Sorry, I just realized that we can't change it in this patch.

VisibleSelection::asSelection() will hit DCHECK if the VS is ill-formed, which
means we need to check validity of the VS before calling VS::asSelection(). This
function does exactly the job.

+    Document* document) {
   if (!passedSelection.base().isConnected() ||
-      !passedSelection.extent().isConnected())
-    return VisibleSelection();
-  DCHECK(!passedSelection.base().document()->needsLayoutTreeUpdate());
-  return createVisibleSelection(passedSelection.asSelection());
+      !passedSelection.extent().isConnected() ||
+      passedSelection.base().document() != document ||
+      passedSelection.base().document() != passedSelection.extent().document())
+    return SelectionInDOMTree();
+  return passedSelection.asSelection();
 }
 
 void Editor::appliedEditing(CompositeEditCommand* cmd) {
   DCHECK(!cmd->isCommandGroupWrapper());
   EventQueueScope scope;
 
   // Request spell checking before any further DOM change.
   spellChecker().markMisspellingsAfterApplyingCommand(*cmd);
 
   UndoStep* undoStep = cmd->undoStep();
   DCHECK(undoStep);
   dispatchEditableContentChangedEvents(undoStep->startingRootEditableElement(),
                                        undoStep->endingRootEditableElement());
   // TODO(chongz): Filter empty InputType after spec is finalized.
   dispatchInputEventEditableContentChanged(
       undoStep->startingRootEditableElement(),
       undoStep->endingRootEditableElement(), cmd->inputType(),
       cmd->textDataForInputEvent(), isComposingFromCommand(cmd));
 
-  // TODO(editing-dev): The use of updateStyleAndLayoutIgnorePendingStylesheets
-  // needs to be audited.  See http://crbug.com/590369 for more details.
-  // The clean layout is consumed by |mostBackwardCaretPosition|, called through
-  // |changeSelectionAfterCommand|. In the long term, we should postpone visible
-  // selection canonicalization so that selection update does not need layout.
-  frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
-
-  const VisibleSelection& newSelection =
-      correctedVisibleSelection(cmd->endingSelection());
+  const SelectionInDOMTree& newSelection = correctedSelectionAfterCommand(
+      cmd->endingSelection(), frame().document());
 
   // Don't clear the typing style with this selection change. We do those things
   // elsewhere if necessary.
-  changeSelectionAfterCommand(newSelection.asSelection(), 0);
+  changeSelectionAfterCommand(newSelection, 0);
 
   if (!cmd->preservesTypingStyle())
     clearTypingStyle();
 
   // Command will be equal to last edit command only in the case of typing
   if (m_lastEditCommand.get() == cmd) {
     DCHECK(cmd->isTypingCommand());
   } else if (m_lastEditCommand && m_lastEditCommand->isDragAndDropCommand() &&
              (cmd->inputType() == InputEvent::InputType::DeleteByDrag ||
               cmd->inputType() == InputEvent::InputType::InsertFromDrop)) {
     // Only register undo entry when combined with other commands.
     if (!m_lastEditCommand->undoStep())
       m_undoStack->registerUndoStep(m_lastEditCommand->ensureUndoStep());
     m_lastEditCommand->appendCommandToUndoStep(cmd);
   } else {
     // Only register a new undo command if the command passed in is
     // different from the last command
     m_lastEditCommand = cmd;
     m_undoStack->registerUndoStep(m_lastEditCommand->ensureUndoStep());
   }
 
-  respondToChangedContents(newSelection.start());
+  respondToChangedContents(newSelection.base());
 }
 
 void Editor::unappliedEditing(UndoStep* cmd) {
   EventQueueScope scope;
 
   dispatchEditableContentChangedEvents(cmd->startingRootEditableElement(),
                                        cmd->endingRootEditableElement());
   dispatchInputEventEditableContentChanged(
       cmd->startingRootEditableElement(), cmd->endingRootEditableElement(),
       InputEvent::InputType::HistoryUndo, nullAtom,
       InputEvent::EventIsComposing::NotComposing);
 
-  // TODO(editing-dev): The use of updateStyleAndLayoutIgnorePendingStylesheets
-  // needs to be audited.  See http://crbug.com/590369 for more details.
-  // In the long term, we should stop editing commands from storing
-  // VisibleSelections as starting and ending selections.
-  frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
-
-  const VisibleSelection& newSelection =
-      correctedVisibleSelection(cmd->startingSelection());
-  DCHECK(newSelection.isValidFor(*frame().document())) << newSelection;
+  const SelectionInDOMTree& newSelection = correctedSelectionAfterCommand(
+      cmd->startingSelection(), frame().document());
   changeSelectionAfterCommand(
-      newSelection.asSelection(),
+      newSelection,
       FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle);
 
   m_lastEditCommand = nullptr;
   m_undoStack->registerRedoStep(cmd);
-  respondToChangedContents(newSelection.start());
+  respondToChangedContents(newSelection.base());
 }
 
 void Editor::reappliedEditing(UndoStep* cmd) {
   EventQueueScope scope;
 
   dispatchEditableContentChangedEvents(cmd->startingRootEditableElement(),
                                        cmd->endingRootEditableElement());
   dispatchInputEventEditableContentChanged(
       cmd->startingRootEditableElement(), cmd->endingRootEditableElement(),
       InputEvent::InputType::HistoryRedo, nullAtom,
       InputEvent::EventIsComposing::NotComposing);
 
-  // TODO(editing-dev): The use of updateStyleAndLayoutIgnorePendingStylesheets
-  // needs to be audited.  See http://crbug.com/590369 for more details.
-  // In the long term, we should stop editing commands from storing
-  // VisibleSelections as starting and ending selections.
-  frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
-  const VisibleSelection& newSelection =
-      correctedVisibleSelection(cmd->endingSelection());
-  DCHECK(newSelection.isValidFor(*frame().document())) << newSelection;
+  const SelectionInDOMTree& newSelection = correctedSelectionAfterCommand(
+      cmd->endingSelection(), frame().document());
   changeSelectionAfterCommand(
-      newSelection.asSelection(),
+      newSelection,
       FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle);
 
   m_lastEditCommand = nullptr;
   m_undoStack->registerUndoStep(cmd);
-  respondToChangedContents(newSelection.start());
+  respondToChangedContents(newSelection.base());
 }
 
 Editor* Editor::create(LocalFrame& frame) {
   return new Editor(frame);
 }
 
 Editor::Editor(LocalFrame& frame)
     : m_frame(&frame),
       m_undoStack(UndoStack::create()),
       m_preventRevealSelection(0),
@@ skipping 783 matching lines @@
 
 DEFINE_TRACE(Editor) {
   visitor->trace(m_frame);
   visitor->trace(m_lastEditCommand);
   visitor->trace(m_undoStack);
   visitor->trace(m_mark);
   visitor->trace(m_typingStyle);
 }
 
 }  // namespace blink