Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

This patch also changes a variable name reference |insertionPos| to
|enclosingBlockOfInsertionPos| to match with current code.

Note: This assertion is introduced by https://bugs.webkit.org/show_bug.cgi?id=8592

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html

Committed: https://crrev.com/8f313902e23e2589ddd9d3d2bfed8ee61e979f17
Cr-Commit-Position: refs/heads/master@{#417889}

[yosin_UTC9, 2016-09-09 08:15:45]

Description was changed from

==========
2016-09-09T17:11:56

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

to

==========
Get rid of wrong assertion in ReplaceSelectionCommand::doApply()




BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

[yosin_UTC9, 2016-09-09 08:15:51]

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

[commit-bot: I haz the power, 2016-09-09 08:15:57]

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

[yosin_UTC9, 2016-09-09 08:24:43]

Description was changed from

==========
Get rid of wrong assertion in ReplaceSelectionCommand::doApply()




BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

to

==========
Get rid of a wrong assertion in ReplaceSelectionCommand::doApply()

This patch gets rid of a wrong assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
in |ReplaceSelectionCommand::doApply()|, since we found a pattern to hit this
assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

[yosin_UTC9, 2016-09-09 09:30:16]

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

[yosin_UTC9, 2016-09-09 09:30:17]

PTAL

[commit-bot: I haz the power, 2016-09-09 09:59:53]

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

[commit-bot: I haz the power, 2016-09-09 09:59:54]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2016-09-12 04:58:56]

https://codereview.chromium.org/2322853004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
(left):

https://codereview.chromium.org/2322853004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:1151:
DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot);
What's the reason the assertion was introduced? Is there any buggy behavior that
it's trying to detect?

[yosin_UTC9, 2016-09-12 05:16:58]

Description was changed from

==========
Get rid of a wrong assertion in ReplaceSelectionCommand::doApply()

This patch gets rid of a wrong assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
in |ReplaceSelectionCommand::doApply()|, since we found a pattern to hit this
assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

to

==========
Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

[yosin_UTC9, 2016-09-12 05:18:03]

Description was changed from

==========
Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

to

==========
Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

Note: This assertion is introduced by
https://bugs.webkit.org/show_bug.cgi?id=8592

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

[yosin_UTC9, 2016-09-12 05:21:20]

Description was changed from

==========
Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

Note: This assertion is introduced by
https://bugs.webkit.org/show_bug.cgi?id=8592

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

to

==========
Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

This patch also changes a variable name reference |insertionPos| to
|enclosingBlockOfInsertionPos| to match with current code.


Note: This assertion is introduced by
https://bugs.webkit.org/show_bug.cgi?id=8592

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

[yosin_UTC9, 2016-09-12 05:21:32]

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

[yosin_UTC9, 2016-09-12 05:21:38]

PTAL

https://codereview.chromium.org/2322853004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
(left):

https://codereview.chromium.org/2322853004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:1151:
DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot);
On 2016/09/12 at 04:58:56, Xiaocheng wrote:
> What's the reason the assertion was introduced? Is there any buggy behavior
that it's trying to detect?

This assertion is introduced by https://bugs.webkit.org/show_bug.cgi?id=8592
It seems this assertion is correct, we should not adjust
|enclosingBlockOfInsertionPos| when |enclosingBlockOfInsertionPos ==
currentRoot|.

[commit-bot: I haz the power, 2016-09-12 05:21:45]

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-09-12 07:02:34]

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

[commit-bot: I haz the power, 2016-09-12 07:02:34]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2016-09-12 07:19:49]

lgtm with a nit.

https://codereview.chromium.org/2322853004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
(right):

https://codereview.chromium.org/2322853004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:1148:
// Adjust |enclosingBlockOfInsertionPos| to prevent nesting.
nit: The body of this |if| is still adjusting |insertionPos|. I don't think we
need to change the comments.

[yosin_UTC9, 2016-09-12 07:36:01]

https://codereview.chromium.org/2322853004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
(right):

https://codereview.chromium.org/2322853004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:1148:
// Adjust |enclosingBlockOfInsertionPos| to prevent nesting.
On 2016/09/12 at 07:19:49, Xiaocheng wrote:
> nit: The body of this |if| is still adjusting |insertionPos|. I don't think we
need to change the comments.

I just change variable name |insertPos| to |enclosingBlockOfInsertionPos| to
match current code.

[yosin_UTC9, 2016-09-12 07:36:43]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-09-12 07:36:55]

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-09-12 07:41:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-12 07:44:48]

Description was changed from

==========
Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

This patch also changes a variable name reference |insertionPos| to
|enclosingBlockOfInsertionPos| to match with current code.


Note: This assertion is introduced by
https://bugs.webkit.org/show_bug.cgi?id=8592

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html
==========

to

==========
Convert an assertion to if-condition in ReplaceSelectionCommand::doApply()

This patch converts a following assertion
  DCHECK_NE(enclosingBlockOfInsertionPos, currentRoot)
to if-condition in |ReplaceSelectionCommand::doApply()|, since we found a
pattern to hit this assertion in "paste-text.html".

New test case is the result of converting "paste-text-009.html" to use w3c
test harness. The difference between "paste-test.html" and "paste-text-009.html"
is root editable:
  "past-text.html":
    |currentRoot| is "<div>one"
    |enclosingBlockOfInsertionPos| is "<div>one"

  "paste-text-009.html":
    |currentRoot| is BODY
    |enclosingBlockOfInsertionPos| is "<div>Omitted".

This patch also changes a variable name reference |insertionPos| to
|enclosingBlockOfInsertionPos| to match with current code.

Note: This assertion is introduced by
https://bugs.webkit.org/show_bug.cgi?id=8592

BUG=644147
TEST=LayoutTests/editing/pasteboard/paste_text.html

Committed: https://crrev.com/8f313902e23e2589ddd9d3d2bfed8ee61e979f17
Cr-Commit-Position: refs/heads/master@{#417889}
==========

[commit-bot: I haz the power, 2016-09-12 07:44:49]

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