Introduce SelectionForUndoStep to reduce usage of VisibleSelection |
|||||
Issue descriptionSince we always canonicalized when we retrieve selection from undo stack, we don't need to put VisibleSelection in undo stack.
,
Jul 24 2017
,
Jul 25 2017
Experimental patch: http://crrev.com/c/582724 Crash is caused by: - CreateVisbilePosition() with dirty layout - SelecitonInUndoStep::AsSelection() with disconnected position - SetEndingSelection() -> SelectionInUndoStep::RootEditableElement() with dirty layout - CompositeEditCommand::CompositeEditCommand() -> ComputeVisbileSelectionInDOMTree() Lots of failures: Expected to fail, but passed: (4) virtual/layout_ng/fast/block/basic/011.html virtual/layout_ng/fast/block/basic/014.html virtual/mojo-loading/fast/table/column-in-inline.html virtual/rootlayerscrolls/fast/scrolling/scrollbar-prevent-default.html Regressions: Unexpected text-only failures (4) editing/deleting/delete_after_span_ws.html [ Failure ] editing/deleting/delete_tab.html [ Failure ] virtual/display_list_2d_canvas/fast/canvas/rendering-contexts-back-references.html [ Failure ] virtual/threaded/fast/scroll-behavior/first-scroll-runs-on-compositor.html [ Failure ] Regressions: Unexpected image-only failures (1) css3/blending/background-blend-mode-overlapping-accelerated-elements.html [ Failure ] Regressions: Unexpected crashes (94) accessibility/textbox-role-on-contenteditable-crash.html [ Crash ] css3/flexbox/inline-flex-crash.html [ Crash ] css3/flexbox/inline-flex-crash2.html [ Crash ] css3/flexbox/insert-text-crash.html [ Crash ] editing/caret/caret-direction-auto.html [ Crash ] editing/deleting/25322-3.html [ Crash ] editing/deleting/2610675-1.html [ Crash ] editing/deleting/2610675-2.html [ Crash ] editing/deleting/4875189.html [ Crash ] editing/deleting/5890684.html [ Crash ] editing/deleting/backspace-merge-into-block.html [ Crash ] editing/deleting/backspace-merge-into-list-item.html [ Crash ] editing/deleting/delete-and-cleanup.html [ Crash ] editing/deleting/delete-block-table.html [ Crash ] editing/deleting/delete-br-013.html [ Crash ] editing/deleting/delete-line-004.html [ Crash ] editing/deleting/delete-line-006.html [ Crash ] editing/deleting/delete-line-011.html [ Crash ] editing/deleting/delete-line-015.html [ Crash ] editing/deleting/delete-line-break-before-underlined-content.html [ Crash ] editing/deleting/delete-line-break-between-paragraphs-with-same-style.html [ Crash ] editing/deleting/delete_at_paragraph_boundaries.html [ Crash ] editing/deleting/delete_block_merge_whitespace.html [ Crash ] editing/deleting/delete_by_word.html [ Crash ] editing/deleting/delete_keep_style.html [ Crash ] editing/deleting/delete_list_item.html [ Crash ] editing/deleting/delete_reply_test.html [ Crash ] editing/deleting/maintain-style-after-delete.html [ Crash ] editing/deleting/merge-different-styles.html [ Crash ] editing/deleting/merge-list-items-in-same-list.html [ Crash ] editing/deleting/merge-paragraph-containing-noneditable.html [ Crash ] editing/deleting/merge-paragraph-with-style-from-rule.html [ Crash ] editing/deleting/merge_paragraph_from_h6.html [ Crash ] editing/deleting/paste-with-transparent-background-color.html [ Crash ] editing/execCommand/19089.html [ Crash ] editing/execCommand/35791.html [ Crash ] editing/execCommand/4916235.html [ Crash ] editing/execCommand/4917055.html [ Crash ] editing/execCommand/5575101-1.html [ Crash ] editing/execCommand/5658933-3.html [ Crash ] editing/execCommand/apply-style-id-name.html [ Crash ] editing/execCommand/break-out-of-empty-list-item.html [ Crash ] editing/execCommand/convert-style-elements-to-spans.html [ Crash ] editing/execCommand/crash-line-break-after-outdent.html [ Crash ] editing/execCommand/format-block-multiple-paragraphs-in-pre.html [ Crash ] editing/execCommand/format-block-with-block.html [ Crash ] editing/execCommand/format-block-with-braces.html [ Crash ] editing/execCommand/format_block/no-visible-content.html [ Crash ] editing/execCommand/format_block/unrooted-selection-start-crash.html [ Crash ] editing/execCommand/indent-block-in-list.html [ Crash ] editing/execCommand/indent-images-3.html [ Crash ] editing/execCommand/indent-nested-blockquotes.html [ Crash ] editing/execCommand/indent-nested-inlines-1.html [ Crash ] editing/execCommand/indent-partial-table.html [ Crash ] editing/execCommand/indent-with-after-content-crash.html [ Crash ] editing/execCommand/indent/indent_empty_quote.html [ Crash ] editing/execCommand/indent/indent_nested_lists.html [ Crash ] editing/execCommand/indent/indent_with_style.html [ Crash ] editing/execCommand/inline-style-after-indentoutdent.html [ Crash ] editing/execCommand/insert-list-br-with-child-crash.html [ Crash ] editing/execCommand/insert-list-infinite-loop2.html [ Crash ] editing/execCommand/insert-list-items-inside-another-list.html [ Crash ] editing/execCommand/insert-lists-inside-another-list.html [ Crash ] editing/execCommand/insert-ordered-list-crash2.html [ Crash ] editing/execCommand/insert-paragraph-in-list-item-anchor-crash.html [ Crash ] editing/execCommand/insertHTML-mutation-crash.html [ Crash ] editing/execCommand/insert_list/insert_list_in_summary_crash.html [ Crash ] editing/execCommand/insert_list/insert_list_ul_li_to_ol.html [ Crash ] editing/execCommand/insert_paragraph/insert_paragraph_pre_crash.html [ Crash ] editing/execCommand/list-wrapping-image-crash.html [ Crash ] editing/execCommand/listify-output-crash.html [ Crash ] editing/execCommand/outdent/outdent_nested_lists.html [ Crash ] editing/execCommand/remove-list-from-multi-list-items.html [ Crash ] editing/execCommand/remove-list-from-range-selection.html [ Crash ] editing/execCommand/selection-after-insert-list.html [ Crash ] editing/execCommand/selection-after-switch-type-of-listitem.html [ Crash ] editing/execCommand/toggle-styles.html [ Crash ] editing/input/ime-composition-clearpreedit.html [ Crash ] editing/input/keyboard-ctrl-enter-no-newline.html [ Crash ] editing/input/password-echo-passnode.html [ Crash ] editing/input/password-echo-passnode2.html [ Crash ] editing/input/password-echo-passnode3.html [ Crash ] editing/input/password-echo-textnode.html [ Crash ] editing/input/reveal-caret-of-transformed-input-scrollable-parent.html [ Crash ] editing/input/reveal-caret-of-transformed-multiline-input.html [ Crash ] editing/input/reveal-contenteditable-on-paste-vertically.html [ Crash ] editing/input/scroll-to-edge-if-line-break-at-end-of-document-contenteditable.html [ Crash ] editing/input/scroll-to-edge-if-line-break-at-end-of-document-textarea.html [ Crash ] editing/input/scroll-to-edge-if-paragraph-separator-at-end-of-document-contenteditable.html [ Crash ] editing/input/set-value-on-input-and-forward-delete.html [ Crash ] http/tests/inspector/extensions-useragent.html [ Crash ] virtual/exotic-color-space/images/crossfade-client-not-removed-crash.html [ Crash ] virtual/gpu-rasterization/images/crossfade-client-not-removed-crash.html [ Crash ] virtual/mojo-loading/http/tests/inspector/extensions-iframe-eval.html [ Crash ] Regressions: Unexpected timeouts (6) http/tests/css/css-resources-referrer.html [ Timeout ] virtual/threaded/animations/composited-animations-rotate-zero-degrees.html [ Timeout ] virtual/threaded/animations/responsive/filter-responsive-neutral-keyframe.html [ Timeout ] virtual/threaded/animations/svg/text-decoration.html [ Timeout ] virtual/threaded/transitions/multiple-mask-transitions.html [ Timeout ] virtual/threaded/transitions/transition-end-event-destroy-iframe.html [ Timeout ]
,
Jul 26 2017
Now five crashes with CRASH for disconnected position: 1. editing/pasteboard/drag-list-item.html 2. editing/pasteboard/drag-drop-list.html 3. editing/pasteboard/drag-drop-url-with-style.html 4. editing/selection/mouse/drag_focus_node.html 5. editing/selection/mouse/drag_table_cell.html Sample stack trace: [350436:407504:0725/183524.233:2708653156:FATAL:SelectionTemplate.cpp(213)] Check failed: position.IsConnected(). BR@beforeAnchor Backtrace: base::debug::StackTrace::StackTrace [0x0000021C89FC5DA5+69] (c:\src\w\cr2\src\base\debug\stack_trace_win.cc:217) base::debug::StackTrace::StackTrace [0x0000021C89FC58C8+24] (c:\src\w\cr2\src\base\debug\stack_trace.cc:199) logging::LogMessage::~LogMessage [0x0000021C8A036F50+112] (c:\src\w\cr2\src\base\logging.cc:554) blink::SelectionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> >::Builder::Collapse [0x0000021C930178F6+182] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\selectiontemplate.cpp:214) blink::SelectionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> >::Builder::SetBaseAndExtent [0x0000021C9301916A+426] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\selectiontemplate.cpp:283) blink::SelectionInUndoStep::AsSelection [0x0000021C9309AFDA+74] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\commands\selectioninundostep.cpp:62) blink::CreateVisibleSelection [0x0000021C9309B03D+29] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\commands\selectioninundostep.cpp:123) blink::CreateVisibleSelection [0x0000021C9309B0CD+173] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\commands\selectioninundostep.cpp:22) blink::SelectionInUndoStep::RootEditableElement [0x0000021C9309B3AE+30] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\commands\selectioninundostep.cpp:103) blink::UndoStep::SetEndingSelection [0x0000021C930A96BE+46] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\commands\undostep.cpp:99) blink::Editor::AppliedEditing [0x0000021C92FCC485+1269] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\editor.cpp:968) blink::CompositeEditCommand::Apply [0x0000021C93064E26+774] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\commands\compositeeditcommand.cpp:145) blink::Editor::DeleteSelectionWithSmartDelete [0x0000021C92FCEFA6+342] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\editor.cpp:470) blink::Editor::DeleteSelectionAfterDraggingWithEvents [0x0000021C92FCEE3A+218] (c:\src\w\cr2\src\third_party\webkit\source\core\editing\editor.cpp:734) blink::DragController::ConcludeEditDrag [0x0000021C93CF9463+2051] (c:\src\w\cr2\src\third_party\webkit\source\core\page\dragcontroller.cpp:635) blink::DragController::PerformDrag [0x0000021C93CFC873+851] (c:\src\w\cr2\src\third_party\webkit\source\core\page\dragcontroller.cpp:284) blink::WebFrameWidgetBase::DragTargetDrop [0x0000021C93368312+562] (c:\src\w\cr2\src\third_party\webkit\source\core\frame\webframewidgetbase.cpp:136) test_runner::EventSender::FinishDragAndDrop [0x0000021C9819E317+391] (c:\src\w\cr2\src\content\shell\test_runner\event_sender.cc:2701) test_runner::EventSender::DoDragAfterMouseUp [0x0000021C9819D77C+524] (c:\src\w\cr2\src\content\shell\test_runner\event_sender.cc:2738) test_runner::EventSender::ReplaySavedEvents [0x0000021C981A7D61+961] (c:\src\w\cr2\src\content\shell\test_runner\event_sender.cc:2804) test_runner::EventSender::DoDragDrop [0x0000021C9819DAFD+733] (c:\src\w\cr2\src\content\shell\test_runner\event_sender.cc:1389) test_runner::WebWidgetTestClient::StartDragging [0x0000021C982D81AE+78] (c:\src\w\cr2\src\content\shell\test_runner\web_widget_test_client.cc:103) test_runner::WebViewTestProxy<content::RenderViewImpl,content::CompositorDependencies * __ptr64,content::mojom::CreateViewParams const & __ptr64>::StartDragging [0x0000000140C5C425+101] (c:\src\w\cr2\src\content\shell\test_runner\web_view_test_proxy.h:183) blink::WebFrameWidgetBase::StartDragging [0x0000021C9336896D+109] (c:\src\w\cr2\src\third_party\webkit\source\core\frame\webframewidgetbase.cpp:182) blink::ChromeClientImpl::StartDragging [0x0000021C93CEEE0E+142] (c:\src\w\cr2\src\third_party\webkit\source\core\page\chromeclientimpl.cpp:252) blink::DragController::DoSystemDrag [0x0000021C93CFA37E+830] (c:\src\w\cr2\src\third_party\webkit\source\core\page\dragcontroller.cpp:1268) blink::DragController::StartDrag [0x0000021C93CFD896+1174] (c:\src\w\cr2\src\third_party\webkit\source\core\page\dragcontroller.cpp:1156) blink::MouseEventManager::TryStartDrag [0x0000021C9372EB5F+687] (c:\src\w\cr2\src\third_party\webkit\source\core\input\mouseeventmanager.cpp:952) blink::MouseEventManager::HandleDrag [0x0000021C9372C73C+1484] (c:\src\w\cr2\src\third_party\webkit\source\core\input\mouseeventmanager.cpp:895) blink::MouseEventManager::HandleMouseDraggedEvent [0x0000021C9372CB90+384] (c:\src\w\cr2\src\third_party\webkit\source\core\input\mouseeventmanager.cpp:787) blink::EventHandler::HandleMouseMoveOrLeaveEvent [0x0000021C9371DF79+3017] (c:\src\w\cr2\src\third_party\webkit\source\core\input\eventhandler.cpp:930) blink::EventHandler::PassMouseMoveEventToSubframe [0x0000021C93720F0B+123] (c:\src\w\cr2\src\third_party\webkit\source\core\input\eventhandler.cpp:2093) blink::EventHandler::HandleMouseMoveOrLeaveEvent [0x0000021C9371DD9B+2539] (c:\src\w\cr2\src\third_party\webkit\source\core\input\eventhandler.cpp:902) blink::EventHandler::HandleMouseMoveEvent [0x0000021C9371D25F+271] (c:\src\w\cr2\src\third_party\webkit\source\core\input\eventhandler.cpp:740) blink::PageWidgetEventHandler::HandleMouseMove [0x0000021C93D25E06+134] (c:\src\w\cr2\src\third_party\webkit\source\core\page\pagewidgetdelegate.cpp:236) blink::PageWidgetDelegate::HandleInputEvent [0x0000021C93D25A14+628] (c:\src\w\cr2\src\third_party\webkit\source\core\page\pagewidgetdelegate.cpp:155)
,
Jul 26 2017
One failure for PS#7 editing/undo/undo-delete-boundary.html This is a testharness.js-based test. PASS android: backward delete PASS android: forward delete PASS android: backward and forward delete PASS mac: backward delete FAIL mac: forward delete LayoutTests/editing/undo/undo-delete-boundary.html:30:14) expected <div contenteditable>This ^wo<b>rd| </b>should be selected only on mac.</div>, but got <div contenteditable>This wo<b>^r|d </b>should be selected only on mac.</div>, sameupto <div contenteditable>This PASS mac: backward and forward delete PASS unix: backward delete PASS unix: forward delete PASS unix: backward and forward delete PASS win: backward delete PASS win: forward delete PASS win: backward and forward delete Harness: the test ran to completion.
,
Jul 27 2017
Because SelectionInUndoStep is stored into UndoStep and CompositeEditCommand, it is better to use SelectionForUndoStep.
,
Jul 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6 commit c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6 Author: Yoshifumi Inoue <yosin@chromium.org> Date: Fri Jul 28 02:22:42 2017 Rename CompositeEditCommand::EndingSelection() to EndingVisibleSelection() This patch renames |CompositeEditCommand::EndingSelection()| to |EndingVisibleSelection()| as a preparation of introducing |SelectionForUndoStep|[1]. Note: This is done by global replacement tool. [1] http://crrev.com/c/588874: Introduce SelectionForUndoStep Bug: 747795 Change-Id: I19e2ae3a75745559930c225ccc4ac7501b33a683 Reviewed-on: https://chromium-review.googlesource.com/588609 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#490213} [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/BreakBlockquoteCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/CreateLinkCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/IndentOutdentCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/commands/UnlinkCommand.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp [modify] https://crrev.com/c37a2a46dbb2b98e9b35cb5de0c03502c075c6d6/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
,
Jul 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96b8cfa0076f4e1f6bc5238ca79e711882c4ed22 commit 96b8cfa0076f4e1f6bc5238ca79e711882c4ed22 Author: Yoshifumi Inoue <yosin@chromium.org> Date: Fri Jul 28 03:29:45 2017 Utilize RootEditableElementOf() in UndoStep.cpp This patch changes |VisibleSelection::RootEditableElement()| with |RootEditableElementOf()| as a preparation of getting rid of |VisibleSelection::RootEditableElement()| for simplify |VisibleSelection|. This patch is also a preparation of the patch[1]. Note: |RootElementElementOf(visible_selection.Start())| and |RootElementElementOf(visible_selection.Base())| are identical since base and extent of visible selection are in same editable root element as result of adjustment in |VisibleSelection::Validate()|. [1] http://crrev.com/c/588874: Introduce SelectionForUndoStep Bug: 657237, 747795 Change-Id: Ifaa4d62c1e4e60687c760184f198313f89f86ccb Reviewed-on: https://chromium-review.googlesource.com/588210 Reviewed-by: Yoichi Osato <yoichio@chromium.org> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#490239} [modify] https://crrev.com/96b8cfa0076f4e1f6bc5238ca79e711882c4ed22/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp
,
Aug 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/44e59041eab4efb2d9cae5e3e7e5623963dd131a commit 44e59041eab4efb2d9cae5e3e7e5623963dd131a Author: Yoshifumi Inoue <yosin@chromium.org> Date: Thu Aug 03 07:13:06 2017 Introduce SelectionForUndoStep This patch introduces |SelectionForUndoStep| to hold selection for undo/redo instead of |VisibleSelection| and avoid dirty |VisibleSelection|[1] for improving code health. Before this patch, we hold selection as |VisibleSelection| in |UndoStep| and recompute "visible selection" by |CorrectedSelectionAfterCommand()| at undo/redo time, DOM tree can be different when we save |VisibleSelection| in |UndoStep|. After this patch, we use |SelectionForUndoStep| to hold selection at command execution and change |EndingVisibleSelection()| to compute visible selection before we used. This patch looks large, but most of them are replacing |EndingVisibleSelection()| to |EndingSelection()| to use |SelectionInUndoStep| instead of updated |VisibleSelection|. Essential part of this change is * "CompositeEditCommand.{cpp,h}": holding starting and ending selection in |SelectionInUndoStep| * "TypginCommand.{cpp,h}": Computing |SelectionInUndoStep| other than |FrameSelection|. * "SelectionInUndoStep.{cpp.h}": The implementation * "UndoStep.{cpp,h}": holding staring and ending seleciton in |SelectionInUndoStep| Just for reference, there is another attempt in the patch: - http://crrev.com/2457523004: Introduce SelectionForUndoStep - http://crrev.com/c/582724 Introduce SelectionForUndoStep [1] Dirty VisibleSelection: |VisibleSelection| holds canonicalized base and extent positions by using layout tree. Once layout tree is updated, e.g. by DOM change or style change, |VisibleSelection| may not be canonicalized. In other words, once we get |VisibleSelection|, it is valid only until layout tree updated. Bug: 747795 Change-Id: I2da864e03fbd2e06f5150072eafe133430dcbfcb Reviewed-on: https://chromium-review.googlesource.com/588874 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#491667} [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/BUILD.gn [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/BreakBlockquoteCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/CreateLinkCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp [add] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.cpp [add] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.h [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/TypingCommand.h [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/UndoStep.h [modify] https://crrev.com/44e59041eab4efb2d9cae5e3e7e5623963dd131a/third_party/WebKit/Source/core/editing/commands/UnlinkCommand.cpp
,
Aug 3 2017
,
Aug 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d8242f55d3861d817d70a12d249e1bb8357340fe commit d8242f55d3861d817d70a12d249e1bb8357340fe Author: Tsuyoshi Horo <horo@chromium.org> Date: Thu Aug 03 08:56:04 2017 Revert "Introduce SelectionForUndoStep" This reverts commit 44e59041eab4efb2d9cae5e3e7e5623963dd131a. Reason for revert: Caused LayoutTest failures on ASAN bot. BUG:752013 Original change's description: > Introduce SelectionForUndoStep > > This patch introduces |SelectionForUndoStep| to hold selection for undo/redo > instead of |VisibleSelection| and avoid dirty |VisibleSelection|[1] for > improving code health. > > Before this patch, we hold selection as |VisibleSelection| in |UndoStep| and > recompute "visible selection" by |CorrectedSelectionAfterCommand()| at undo/redo > time, DOM tree can be different when we save |VisibleSelection| in |UndoStep|. > > After this patch, we use |SelectionForUndoStep| to hold selection at command > execution and change |EndingVisibleSelection()| to compute visible selection > before we used. > > This patch looks large, but most of them are replacing > |EndingVisibleSelection()| to |EndingSelection()| to use |SelectionInUndoStep| > instead of updated |VisibleSelection|. > > Essential part of this change is > * "CompositeEditCommand.{cpp,h}": holding starting and ending selection in > |SelectionInUndoStep| > > * "TypginCommand.{cpp,h}": Computing |SelectionInUndoStep| other than > |FrameSelection|. > * "SelectionInUndoStep.{cpp.h}": The implementation > * "UndoStep.{cpp,h}": holding staring and ending seleciton in > |SelectionInUndoStep| > > Just for reference, there is another attempt in the patch: > - http://crrev.com/2457523004: Introduce SelectionForUndoStep > - http://crrev.com/c/582724 Introduce SelectionForUndoStep > > [1] Dirty VisibleSelection: |VisibleSelection| holds canonicalized base and > extent positions by using layout tree. Once layout tree is updated, e.g. by > DOM change or style change, |VisibleSelection| may not be canonicalized. In > other words, once we get |VisibleSelection|, it is valid only until layout tree > updated. > > Bug: 747795 > Change-Id: I2da864e03fbd2e06f5150072eafe133430dcbfcb > Reviewed-on: https://chromium-review.googlesource.com/588874 > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#491667} TBR=yosin@chromium.org,yoichio@chromium.org,xiaochengh@chromium.org Change-Id: I84188673f225161fc65da6ad1693a1721d2d8f5a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 747795 Reviewed-on: https://chromium-review.googlesource.com/599667 Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Commit-Queue: Tsuyoshi Horo <horo@chromium.org> Cr-Commit-Position: refs/heads/master@{#491683} [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/BUILD.gn [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/BreakBlockquoteCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/CreateLinkCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp [delete] https://crrev.com/75f445688a893bf2da56ba664e1155628f5601ce/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.cpp [delete] https://crrev.com/75f445688a893bf2da56ba664e1155628f5601ce/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.h [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/TypingCommand.h [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/UndoStep.h [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/UnlinkCommand.cpp
,
Aug 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f471810d59597fda437adc2db434a2f5f1e58cfc commit f471810d59597fda437adc2db434a2f5f1e58cfc Author: Yoshifumi Inoue <yosin@chromium.org> Date: Thu Aug 03 13:01:50 2017 Reland "Introduce SelectionForUndoStep" This is a reland of 44e59041eab4efb2d9cae5e3e7e5623963dd131a Changes of original is replace |const SelectionForUndo&| to |const SelectionForUndo| at end of DeleteKeyPressed() and ForwardDeleteKeyPressed() as work around of clang's bug which doesn't handle extent of |const T&| as end of scope. Original change's description: > Introduce SelectionForUndoStep > > This patch introduces |SelectionForUndoStep| to hold selection for undo/redo > instead of |VisibleSelection| and avoid dirty |VisibleSelection|[1] for > improving code health. > > Before this patch, we hold selection as |VisibleSelection| in |UndoStep| and > recompute "visible selection" by |CorrectedSelectionAfterCommand()| at undo/redo > time, DOM tree can be different when we save |VisibleSelection| in |UndoStep|. > > After this patch, we use |SelectionForUndoStep| to hold selection at command > execution and change |EndingVisibleSelection()| to compute visible selection > before we used. > > This patch looks large, but most of them are replacing > |EndingVisibleSelection()| to |EndingSelection()| to use |SelectionInUndoStep| > instead of updated |VisibleSelection|. > > Essential part of this change is > * "CompositeEditCommand.{cpp,h}": holding starting and ending selection in > |SelectionInUndoStep| > > * "TypginCommand.{cpp,h}": Computing |SelectionInUndoStep| other than > |FrameSelection|. > * "SelectionInUndoStep.{cpp.h}": The implementation > * "UndoStep.{cpp,h}": holding staring and ending seleciton in > |SelectionInUndoStep| > > Just for reference, there is another attempt in the patch: > - http://crrev.com/2457523004: Introduce SelectionForUndoStep > - http://crrev.com/c/582724 Introduce SelectionForUndoStep > > [1] Dirty VisibleSelection: |VisibleSelection| holds canonicalized base and > extent positions by using layout tree. Once layout tree is updated, e.g. by > DOM change or style change, |VisibleSelection| may not be canonicalized. In > other words, once we get |VisibleSelection|, it is valid only until layout tree > updated. > > Bug: 747795 > Change-Id: I2da864e03fbd2e06f5150072eafe133430dcbfcb > Reviewed-on: https://chromium-review.googlesource.com/588874 > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#491667} TBR=xiaochengh@chromium.org Bug: 747795 , 752013 Change-Id: I3135599416fea2af25aeba1ce76348dbf3b36628 Reviewed-on: https://chromium-review.googlesource.com/600007 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#491715} [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/BUILD.gn [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/BreakBlockquoteCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/CreateLinkCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp [add] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.cpp [add] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.h [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/TypingCommand.h [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/UndoStep.h [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/UnlinkCommand.cpp |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by yosin@chromium.org
, Jul 24 2017Status: Available (was: Untriaged)