Don't return null from ReplaceSelectionCommand::insertAsListItems()

Don't return null from ReplaceSelectionCommand::insertAsListItems() 

When trying to replace list by list, we insert list item instead of list itself, 
by calling insertAsListItems(). However, insertAsListItems() returns null when
it takes empty list.

This patch changes to ReplaceSelectionCommand::insertAsListItems() not to return null.

BUG=238170
TEST=LayoutTests/editing/execCommand/crash-replacing-list-by-list.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170821

[yosin_UTC9, 2014-04-02 08:55:02]

Could you review this patch?
Thanks in advance.

P.S.
win_blink_rel bot failure isn't related to this patch.

[yoichio, 2014-04-02 09:07:25]

lgtm

[Yuta Kitamura, 2014-04-02 09:34:49]

I'm not sure if I could follow the implementation of
insertAsListItems() completely...

I'd like to better understand how it could return null; at least
it isn't obvious to me, and doesn't look like an expected behavior,
either.

I'm asking this because the null case *could* be handled within
insertAsListItems() and there is a possibility that that's
a better solution for this crash.

nitpick on the change description:
"Although" -> "However" ("although" is a conjunction, so it must
be used in a form "Although [S]+[V], [S']+[V'].")
"before use it" -> "before it is used"

https://codereview.chromium.org/220233013/diff/1/Source/core/editing/ReplaceS...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/220233013/diff/1/Source/core/editing/ReplaceS...
Source/core/editing/ReplaceSelectionCommand.cpp:1085: if (!refNode ||
!refNode->inDocument())
nit: Test for |!refNode| should probably be done right after
insertAsListItems(), since transformation into null can only
happen in the first "if" block.

[yosin_UTC9, 2014-04-03 05:01:49]

PTAL

https://codereview.chromium.org/220233013/diff/1/Source/core/editing/ReplaceS...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/220233013/diff/1/Source/core/editing/ReplaceS...
Source/core/editing/ReplaceSelectionCommand.cpp:1085: if (!refNode ||
!refNode->inDocument())
On 2014/04/02 09:34:49, Yuta Kitamura wrote:
> nit: Test for |!refNode| should probably be done right after
> insertAsListItems(), since transformation into null can only
> happen in the first "if" block.

Done.

[Yuta Kitamura, 2014-04-03 05:10:38]

lgtm

[yosin_UTC9, 2014-04-03 05:44:21]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2014-04-03 05:44:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yosin@chromium.org/220233013/20001

[commit-bot: I haz the power, 2014-04-03 05:47:54]

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

[commit-bot: I haz the power, 2014-04-03 05:47:54]

Try jobs failed on following builders:
  tryserver.blink on blink_presubmit

[yosin_UTC9, 2014-04-04 03:33:38]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2014-04-04 03:33:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yosin@chromium.org/220233013/20001

[commit-bot: I haz the power, 2014-04-04 04:53:57]

Change committed as 170821