Fix the crash bug about selecting a marquee element.

Fix the crash bug about selecting a marquee element.

This assertion is in CompositeEditCommand::moveParagraphs, L1223.
The assertion error happens because VisibleSelection(destination, originalIsDirectional) returns default VisibleSelection value If desitination is a marquee element.
Inside the constructor,  VisiblePosition(position, _).deepPosition() returns default VisiblePosition value if position is a marquee element.
Because VisiblePosition::canonicalPosition(position) returns default Position value if position is a marquee element.
This is caused by that position.upstream() returns parent Node if position is a marquee element. This is wrong.
When a marquee element is moving, its upstream should be itself.

To fix this issue, this cl change endsOfNodeAreVisuallyDistinctPositions() called enclosingVisualBoundary() which is called from upstream().
This cl make endsOfNodeAreVisuallyDistinctPositions(node) return true if node is a marquee element.

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

[leviw_travelin_and_unemployed, 2014-02-14 04:57:45]

Nearly there. A couple nits.

https://codereview.chromium.org/165933002/diff/1/LayoutTests/editing/execComm...
File LayoutTests/editing/execCommand/selectAll-including-marquee-crash.html
(right):

https://codereview.chromium.org/165933002/diff/1/LayoutTests/editing/execComm...
LayoutTests/editing/execCommand/selectAll-including-marquee-crash.html:1:
<style>
Let's clean this up a bit more with a doctype and html tag.

https://codereview.chromium.org/165933002/diff/1/LayoutTests/editing/execComm...
LayoutTests/editing/execCommand/selectAll-including-marquee-crash.html:6: 
And a body :)

https://codereview.chromium.org/165933002/diff/1/LayoutTests/editing/execComm...
LayoutTests/editing/execCommand/selectAll-including-marquee-crash.html:8: aa
I suspect you could simply put "Test passes if it does not crash." here instead
of in an innerHTML call.

https://codereview.chromium.org/165933002/diff/1/Source/core/dom/Position.cpp
File Source/core/dom/Position.cpp (right):

https://codereview.chromium.org/165933002/diff/1/Source/core/dom/Position.cpp...
Source/core/dom/Position.cpp:550: // A Marquee element are movings so we should
assume the element's position
I'd change this to something like "Marquee elements are moving so we should
assume their ends are always visibly distinct."

[leviw_travelin_and_unemployed, 2014-02-14 05:18:38]

Also I'd add some more logic in your description. Tell us what was wrong and how
this fixes it.

[yoichio, 2014-02-19 08:00:30]

https://codereview.chromium.org/165933002/diff/1/LayoutTests/editing/execComm...
File LayoutTests/editing/execCommand/selectAll-including-marquee-crash.html
(right):

https://codereview.chromium.org/165933002/diff/1/LayoutTests/editing/execComm...
LayoutTests/editing/execCommand/selectAll-including-marquee-crash.html:1:
<style>
On 2014/02/14 04:57:46, Levi wrote:
> Let's clean this up a bit more with a doctype and html tag.

Done.

https://codereview.chromium.org/165933002/diff/1/LayoutTests/editing/execComm...
LayoutTests/editing/execCommand/selectAll-including-marquee-crash.html:6: 
On 2014/02/14 04:57:46, Levi wrote:
> And a body :)

Done.

https://codereview.chromium.org/165933002/diff/1/LayoutTests/editing/execComm...
LayoutTests/editing/execCommand/selectAll-including-marquee-crash.html:8: aa
On 2014/02/14 04:57:46, Levi wrote:
> I suspect you could simply put "Test passes if it does not crash." here
instead
> of in an innerHTML call.
This test case executes "SelectAll" and "InsertHTML". 
It replaces the existing content.

https://codereview.chromium.org/165933002/diff/1/Source/core/dom/Position.cpp
File Source/core/dom/Position.cpp (right):

https://codereview.chromium.org/165933002/diff/1/Source/core/dom/Position.cpp...
Source/core/dom/Position.cpp:550: // A Marquee element are movings so we should
assume the element's position
On 2014/02/14 04:57:46, Levi wrote:
> I'd change this to something like "Marquee elements are moving so we should
> assume their ends are always visibly distinct."

Done.

[leviw_travelin_and_unemployed, 2014-02-19 18:17:25]

>But marquee elements are moving so VisiblePosition of the node should not be
same to the parent. 

This is definitely right, and I think the fix is good. Can you tell me what
causes the crash as well?

[yoichio, 2014-02-20 07:06:17]

Update the CL description.

[leviw_travelin_and_unemployed, 2014-02-21 22:06:53]

lgtm

[leviw_travelin_and_unemployed, 2014-02-21 22:06:58]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2014-02-21 22:08:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoichio@chromium.org/165933002/80001

[commit-bot: I haz the power, 2014-02-21 23:44:44]

Change committed as 167617