Ensure valid VisiblePositions in InsertListCommand::doApply

Ensure valid VisiblePositions in InsertListCommand::doApply

This patch ensures that the VisiblePositions created in
InsertListCommand::doApply are always valid when used.

This patch is a prepartion of pruning createVisibleSelectionDeprecated
from EditCommand::setEndingSelection.

BUG=648949
Committed: https://crrev.com/af07c6533b27d117b9f57595253e6d3dde192734
Cr-Commit-Position: refs/heads/master@{#424727}

[Xiaocheng, 2016-10-12 03:55:45]

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

[commit-bot: I haz the power, 2016-10-12 03:55:56]

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

[Xiaocheng, 2016-10-12 05:20:00]

xiaochengh@chromium.org changed reviewers:
+ yosin@chromium.org

[Xiaocheng, 2016-10-12 05:20:01]

PTAL.

[commit-bot: I haz the power, 2016-10-12 06:03:05]

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

[commit-bot: I haz the power, 2016-10-12 06:03:06]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2016-10-12 06:27:43]

https://codereview.chromium.org/2412493004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp
(right):

https://codereview.chromium.org/2412493004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp:228: if
(endOfSelection.isNull() || endOfSelection.position().isOrphan() ||
It it better to introduce: PositionWithAffinity::isOrphan() and |isConnected()|?
Actually, it is better to use |!Position::isConnected()| instead of |p.isNull()
|| p.isOrphan()|.

bool isConnected() const { return m_anchorNode && m_anchorNode->isConnected(); }
bool isOrphan() const { return m_anchorNode && !m_anchorNode->isConnected(); }

WDYT?

[Xiaocheng, 2016-10-12 06:31:06]

https://codereview.chromium.org/2412493004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp
(right):

https://codereview.chromium.org/2412493004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp:228: if
(endOfSelection.isNull() || endOfSelection.position().isOrphan() ||
On 2016/10/12 at 06:27:43, Yosi_UTC9 wrote:
> It it better to introduce: PositionWithAffinity::isOrphan() and
|isConnected()|?
> Actually, it is better to use |!Position::isConnected()| instead of
|p.isNull() || p.isOrphan()|.
> 
> bool isConnected() const { return m_anchorNode && m_anchorNode->isConnected();
}
> bool isOrphan() const { return m_anchorNode && !m_anchorNode->isConnected(); }
> 
> WDYT?

Reasonable. Will do.

[yosin_UTC9, 2016-10-12 06:38:47]

https://codereview.chromium.org/2412493004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp
(right):

https://codereview.chromium.org/2412493004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp:228: if
(endOfSelection.isNull() || endOfSelection.position().isOrphan() ||
On 2016/10/12 at 06:31:06, Xiaocheng wrote:
> On 2016/10/12 at 06:27:43, Yosi_UTC9 wrote:
> > It it better to introduce: PositionWithAffinity::isOrphan() and
|isConnected()|?
> > Actually, it is better to use |!Position::isConnected()| instead of
|p.isNull() || p.isOrphan()|.
> > 
> > bool isConnected() const { return m_anchorNode &&
m_anchorNode->isConnected(); }
> > bool isOrphan() const { return m_anchorNode && !m_anchorNode->isConnected();
}
> > 
> > WDYT?
> 
> Reasonable. Will do.

In this patch, it is OK to write |endOfSelection.position().isConnected()|.
It is up to you, introduce PWA::isConnected() and isOrphan(), then use here.
As usual, introducing PWA::isConnected() should be in another patch. ;-)

[Xiaocheng, 2016-10-12 07:06:41]

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

[commit-bot: I haz the power, 2016-10-12 07:07:05]

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

[Xiaocheng, 2016-10-12 07:51:52]

Updated with new functions introduced to PWA
(https://codereview.chromium.org/2406163004).

PTAL.

[yosin_UTC9, 2016-10-12 07:56:39]

lgtm

[commit-bot: I haz the power, 2016-10-12 08:53:38]

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

[commit-bot: I haz the power, 2016-10-12 08:53:39]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2016-10-12 13:23:14]

The CQ bit was checked by xiaochengh@chromium.org

[commit-bot: I haz the power, 2016-10-12 13:23:26]

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-12 13:30:16]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-12 13:33:52]

Description was changed from

==========
Ensure valid VisiblePositions in InsertListCommand::doApply

This patch ensures that the VisiblePositions created in
InsertListCommand::doApply are always valid when used.

This patch is a prepartion of pruning createVisibleSelectionDeprecated
from EditCommand::setEndingSelection.

BUG=648949
==========

to

==========
Ensure valid VisiblePositions in InsertListCommand::doApply

This patch ensures that the VisiblePositions created in
InsertListCommand::doApply are always valid when used.

This patch is a prepartion of pruning createVisibleSelectionDeprecated
from EditCommand::setEndingSelection.

BUG=648949

Committed: https://crrev.com/af07c6533b27d117b9f57595253e6d3dde192734
Cr-Commit-Position: refs/heads/master@{#424727}
==========

[commit-bot: I haz the power, 2016-10-12 13:33:53]

Patchset 2 (id:??) landed as
https://crrev.com/af07c6533b27d117b9f57595253e6d3dde192734
Cr-Commit-Position: refs/heads/master@{#424727}