Get rid of VisibleSelection::selectionFromContentsOfNode()

Get rid of VisibleSelection::selectionFromContentsOfNode()

This patch gets rid of |VisibleSelection::selectionFromContentsOfNode()| with
introducing |SelectionTemplate<T>::selectAllChildren()| by using
|setSelection()| taking |SelectionTemplate<T>| to simplify |VisibleSelection|
class for improving code health.

BUG=657237
TEST=n/a; no behavior changes

Committed: https://crrev.com/95dd59cb1bf42824ce20e77c13b51b5d33dabef0
Cr-Commit-Position: refs/heads/master@{#426437}

[yosin_UTC9, 2016-10-19 06:30:05]

Description was changed from

==========
2016-10-19T15:27:12

BUG=
==========

to

==========
2016-10-19T15:27:12

BUG=Introduce SelectionTemplate<T>::Builder::selectAllChildren()
==========

[yosin_UTC9, 2016-10-19 06:30:12]

The CQ bit was checked by yosin@chromium.org to run a CQ dry run

[yosin_UTC9, 2016-10-19 06:45:55]

The CQ bit was checked by yosin@chromium.org to run a CQ dry run

[yosin_UTC9, 2016-10-19 08:22:43]

Description was changed from

==========
2016-10-19T15:27:12

BUG=Introduce SelectionTemplate<T>::Builder::selectAllChildren()
==========

to

==========
Introduce SelectionTemplate<T>::Builder::selectAllChildren()

BUG=657237
TEST=n/a; no behavior changes
==========

[commit-bot: I haz the power, 2016-10-19 09:31:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 09:54:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-19 09:54:11]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[yosin_UTC9, 2016-10-20 04:09:15]

The CQ bit was checked by yosin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 04:09:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-20 04:22:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-20 04:22:15]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[yosin_UTC9, 2016-10-20 04:47:36]

The CQ bit was checked by yosin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 04:47:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-20 07:34:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-20 07:34:34]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2016-10-20 07:43:37]

Description was changed from

==========
Introduce SelectionTemplate<T>::Builder::selectAllChildren()

BUG=657237
TEST=n/a; no behavior changes
==========

to

==========
Get rid of VisibleSelection::selectionFromContentsOfNode()

This patch gets rid of |VisibleSelection::selectionFromContentsOfNode()| with
introducing |SelectionTemplate<T>::selectAllChildren()| by using
|setSelection()| taking |SelectionTemplate<T>| to simplify |VisibleSelection|
class for improving code health.

BUG=657237
TEST=n/a; no behavior changes
==========

[yosin_UTC9, 2016-10-20 07:44:12]

yosin@chromium.org changed reviewers:
+ tkent@chromium.org, xiaochengh@chromium.org, yoichio@chromium.org

[yosin_UTC9, 2016-10-20 07:44:12]

PTAL

Not only core/editing but also
 - core/page/DragController.cpp
 - core/svg/UnsafeSVGAttributeSanitizationTest.cpp

[Xiaocheng, 2016-10-20 07:51:03]

lgtm

https://codereview.chromium.org/2432173005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/VisibleSelection.cpp (left):

https://codereview.chromium.org/2432173005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleSelection.cpp:213:
DCHECK(!Strategy::editingIgnoresContent(node));
Can we keep this DCHECK in SelectionTemplate::Builder::selectAllChildren?

[tkent, 2016-10-20 08:07:54]

lgtm

[yosin_UTC9, 2016-10-20 08:30:51]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-10-20 08:30:51]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
xiaochengh@chromium.org
Link to the patchset: https://codereview.chromium.org/2432173005/#ps60001
(title: "2016-10-20T17:14:18")

[commit-bot: I haz the power, 2016-10-20 08:31:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yosin_UTC9, 2016-10-20 08:35:34]

Thanks for reviewing!

Add DCHECK() to SelectionTemplate<T>::selectAllChildren()

https://codereview.chromium.org/2432173005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/VisibleSelection.cpp (left):

https://codereview.chromium.org/2432173005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/VisibleSelection.cpp:213:
DCHECK(!Strategy::editingIgnoresContent(node));
On 2016/10/20 at 07:51:03, Xiaocheng wrote:
> Can we keep this DCHECK in SelectionTemplate::Builder::selectAllChildren?

Sure but use |Node::canContainRangeEndPoint()| only since
|isEmptyNonEditableNodeInEditable(node)| requires clean layout tree due by
hasEditableStyle().

Note: WebKit's editingIgnoresContent(node) = !node.canContainRangeEndPoint()
http://trac.webkit.org/browser/trunk/Source/WebCore/editing/htmlediting.h

It seems we should make Blink use editingIgnoresContent() as of WebKit,
otherwise we should put update layout into call path of editingIgnoresContent().

[commit-bot: I haz the power, 2016-10-20 09:52:43]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-21 13:16:52]

Description was changed from

==========
Get rid of VisibleSelection::selectionFromContentsOfNode()

This patch gets rid of |VisibleSelection::selectionFromContentsOfNode()| with
introducing |SelectionTemplate<T>::selectAllChildren()| by using
|setSelection()| taking |SelectionTemplate<T>| to simplify |VisibleSelection|
class for improving code health.

BUG=657237
TEST=n/a; no behavior changes
==========

to

==========
Get rid of VisibleSelection::selectionFromContentsOfNode()

This patch gets rid of |VisibleSelection::selectionFromContentsOfNode()| with
introducing |SelectionTemplate<T>::selectAllChildren()| by using
|setSelection()| taking |SelectionTemplate<T>| to simplify |VisibleSelection|
class for improving code health.

BUG=657237
TEST=n/a; no behavior changes

Committed: https://crrev.com/95dd59cb1bf42824ce20e77c13b51b5d33dabef0
Cr-Commit-Position: refs/heads/master@{#426437}
==========

[commit-bot: I haz the power, 2016-10-21 13:16:52]

Patchset 4 (id:??) landed as
https://crrev.com/95dd59cb1bf42824ce20e77c13b51b5d33dabef0
Cr-Commit-Position: refs/heads/master@{#426437}