Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 708453 Make LayoutObject.selectionState marking Editing friendly
Starred by 2 users Project Member Reported by yoichio@chromium.org, Apr 5 Back to list
Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Task

Blocking:
issue 708452



Sign in to add a comment
This is ready for Selection NG.
 
Blocking: 708452
Project Member Comment 3 by bugdroid1@chromium.org, Apr 6
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a85cc3a3dd57a9811b794878a0056a27cbb058d

commit 4a85cc3a3dd57a9811b794878a0056a27cbb058d
Author: yoichio <yoichio@chromium.org>
Date: Thu Apr 06 10:15:09 2017

Rename FrameSelection::m_pendingSelection to m_layoutSelection

TEST=No change in behavior
BUG=708453

Review-Url: https://codereview.chromium.org/2802983002
Cr-Commit-Position: refs/heads/master@{#462411}

[modify] https://crrev.com/4a85cc3a3dd57a9811b794878a0056a27cbb058d/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/4a85cc3a3dd57a9811b794878a0056a27cbb058d/third_party/WebKit/Source/core/editing/FrameSelection.h

Project Member Comment 4 by bugdroid1@chromium.org, Apr 12
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b33836726c84241997ec990495acd9f9616008d0

commit b33836726c84241997ec990495acd9f9616008d0
Author: yoichio <yoichio@chromium.org>
Date: Wed Apr 12 10:00:20 2017

Move layout/LayoutView::setSelection() to editing/LayoutSelection

This moves layout marking functionality from LayoutView to LayoutSelection
 to prepare for LayoutNG.
This is refatoring but difference of each lifetime makes different
 LayoutObject member clearing timing(see FrameSelection::ContextDestroyed).

Detail:
LayoutView::
 -SetSelection()
 -SelectionBounds()
 -InvalidatePaintForSelection()
 -selection_start_, selection_end_
 -selection_start_pos_, selection_end_pos_
-> move to LayoutSelection. Leave function definition in LayoutView and will move it
 to LayoutSelection later.

 ClearSelection(), SelectionStartEnd()
  They are used from few LayoutObject classes so move function body to
 LayoutSelection and make LayoutView and FrameSelection proxies.

LayoutSelection::
 +SetSelection()
 +SelectionBounds()
 +InvalidatePaintForSelection()
 +selection_start_, selection_end_
 +selection_start_pos_, selection_end_pos_
 +OnDocumentShutdown();
  This just clear above 4 members because LayoutSelection lives longer than
 LayoutView.

LayoutViewItem::
 -SelectionBounds()
 -InvalidatePaintForSelection()
  They are only used from FrameSelection. Since FrameSelection has LayoutSelection,
 just use it directly.

BUG=708453

Review-Url: https://codereview.chromium.org/2800813006
Cr-Commit-Position: refs/heads/master@{#463978}

[modify] https://crrev.com/b33836726c84241997ec990495acd9f9616008d0/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/b33836726c84241997ec990495acd9f9616008d0/third_party/WebKit/Source/core/editing/FrameSelection.h
[modify] https://crrev.com/b33836726c84241997ec990495acd9f9616008d0/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
[modify] https://crrev.com/b33836726c84241997ec990495acd9f9616008d0/third_party/WebKit/Source/core/editing/LayoutSelection.h
[modify] https://crrev.com/b33836726c84241997ec990495acd9f9616008d0/third_party/WebKit/Source/core/layout/LayoutView.cpp
[modify] https://crrev.com/b33836726c84241997ec990495acd9f9616008d0/third_party/WebKit/Source/core/layout/LayoutView.h
[modify] https://crrev.com/b33836726c84241997ec990495acd9f9616008d0/third_party/WebKit/Source/core/layout/api/LayoutViewItem.h

Project Member Comment 5 by bugdroid1@chromium.org, Apr 13
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ce68a3d93044122537ef22d6490319bac338c88

commit 0ce68a3d93044122537ef22d6490319bac338c88
Author: yoichio <yoichio@chromium.org>
Date: Thu Apr 13 08:58:55 2017

Move LayouetSelection function definition from layout/LayoutView.cpp to editing/LayoutSelection.cpp

This is just moving:
https://codereview.chromium.org/2800813006/

TEST=No change in behavior.
BUG=708453

Review-Url: https://codereview.chromium.org/2813403002
Cr-Commit-Position: refs/heads/master@{#464346}

[modify] https://crrev.com/0ce68a3d93044122537ef22d6490319bac338c88/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
[modify] https://crrev.com/0ce68a3d93044122537ef22d6490319bac338c88/third_party/WebKit/Source/core/layout/LayoutView.cpp

Project Member Comment 6 by bugdroid1@chromium.org, Apr 13
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/230b4e0eb7f14d23c70bc4134b8a23a9ddccd5a8

commit 230b4e0eb7f14d23c70bc4134b8a23a9ddccd5a8
Author: yoichio <yoichio@chromium.org>
Date: Thu Apr 13 09:09:39 2017

Remove ClearSelection() from Layout{BlockFlow,Inline}::WillbeDestroyed()

LayoutView::ClearSelection was originally introduced at 2004 to assure no
 crash:
https://chromium.googlesource.com/chromium/src/+/10f7ac6ea6784e33161c7979e9a59c5e2cae14b5

Even now that code doesn't make sense because we update LayoutSelection after
layout in following sequence:
1. FrameView::PerformPostLayoutTasks() checks
 LayoutSelection::SetHasPendingSelection()
2. PaintLayerCompositor::UpdateIfNeededRecursiveInternal() calls
 LayoutSelection::Commit() and it updates layout selection.

BUG=708453

Review-Url: https://codereview.chromium.org/2811333003
Cr-Commit-Position: refs/heads/master@{#464352}

[modify] https://crrev.com/230b4e0eb7f14d23c70bc4134b8a23a9ddccd5a8/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
[modify] https://crrev.com/230b4e0eb7f14d23c70bc4134b8a23a9ddccd5a8/third_party/WebKit/Source/core/layout/LayoutInline.cpp

Project Member Comment 7 by bugdroid1@chromium.org, Apr 13
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5d560486bf5b9e6880ad8501bf1e6b8af3a0e2e

commit e5d560486bf5b9e6880ad8501bf1e6b8af3a0e2e
Author: yoichio <yoichio@chromium.org>
Date: Thu Apr 13 09:22:49 2017

Remove ClearSelection() from DragCaret::NodeWillBeRemoved()

This line was introduced before 2011 splitting DragCaretController from
 FrameSelection:
https://chromium.googlesource.com/chromium/src/+/fa14cd2b25316fb358f7f5ebc16f752ebcb4093a

Now DragCaret doesn't touch Selection Layout and this is unnecessary call.

BUG=708453

Review-Url: https://codereview.chromium.org/2813883005
Cr-Commit-Position: refs/heads/master@{#464356}

[modify] https://crrev.com/e5d560486bf5b9e6880ad8501bf1e6b8af3a0e2e/third_party/WebKit/Source/core/editing/DragCaret.cpp

Project Member Comment 9 by bugdroid1@chromium.org, Apr 14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1210f0f9a5941ce09b2ece0c4d2c593ef64986e5

commit 1210f0f9a5941ce09b2ece0c4d2c593ef64986e5
Author: yoichio <yoichio@chromium.org>
Date: Fri Apr 14 04:06:41 2017

Shorten LayoutSelectionStartEnd() call stack

LayoutObject calls FrameSelection::LayoutSelectionStartEnd() via
 LayoutView. Let's call it directly.

BUG=708453

Review-Url: https://codereview.chromium.org/2818703002
Cr-Commit-Position: refs/heads/master@{#464674}

[modify] https://crrev.com/1210f0f9a5941ce09b2ece0c4d2c593ef64986e5/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/1210f0f9a5941ce09b2ece0c4d2c593ef64986e5/third_party/WebKit/Source/core/layout/LayoutView.cpp
[modify] https://crrev.com/1210f0f9a5941ce09b2ece0c4d2c593ef64986e5/third_party/WebKit/Source/core/layout/LayoutView.h

Project Member Comment 11 by bugdroid1@chromium.org, Apr 25 (2 days ago)
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb75c0c4e9721fcbca5f6229cf2e5dab5e8d9fcd

commit fb75c0c4e9721fcbca5f6229cf2e5dab5e8d9fcd
Author: yoichio <yoichio@chromium.org>
Date: Tue Apr 25 07:01:27 2017

Remove CommitPendingSelection() from LayoutView::HitTestNoLifecycleUpdate()

That's because marking LayoutObject::SelectionState doesn't affect hit test.

BUG=708453

Review-Url: https://codereview.chromium.org/2836843002
Cr-Commit-Position: refs/heads/master@{#466908}

[modify] https://crrev.com/fb75c0c4e9721fcbca5f6229cf2e5dab5e8d9fcd/third_party/WebKit/Source/core/layout/LayoutView.cpp

Comment 12 by yoichio@chromium.org, Apr 25 (2 days ago)
Summary: Make LayoutObject.selectionState marking Editing friendly (was: Restruct LayoutObject.selectionState flag out of LayoutObject)
Comment 13 by yosin@chromium.org, Today (19 hours ago)
Labels: -Type-Bug Type-Task
Sign in to add a comment